Skip to content

🤖 feat: RLM Mode — kernel-first exclusive PTC posture with persistent kernel, context isolation, and continual-harness features - #3900

Open
ThomasK33 wants to merge 234 commits into
mainfrom
research-qr9r
Open

🤖 feat: RLM Mode — kernel-first exclusive PTC posture with persistent kernel, context isolation, and continual-harness features#3900
ThomasK33 wants to merge 234 commits into
mainfrom
research-qr9r

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

Summary

Adds RLM Mode — an opt-in, kernel-first execution posture for PTC inspired by PrimeIntellect's prime-agent architecture — plus the continual-harness features around it (refinement journal with rollback, /refine trajectory distillation, family messaging, branch summarization, compaction improvements) and a measurement harness (shux rlm-eval) that every major design decision in this PR was validated against.

With the RLM experiment off, behavior is byte-identical to main (pinned by composition tests and replay-verify on live sessions). With it on, code_execution becomes the primary tool backed by a persistent per-workspace QuickJS kernel.

Background

Research into prime-agent (which posted strong vendor-reported eval results) identified two core ideas worth porting: a single persistent code kernel where in-kernel data never transits model context, and a self-modifying harness with journaled, reversible edits. Mux's Track 1 foundation (journal kit, durable events, sandbox host, replay harness — #3865/#3872) provided the substrate; this PR is "Track 2" built on it, implemented via the phased conductor workflow in workflows/track2-rlm-implementation.js (per-phase quality gates, adversarial review, live dogfooding).

Implementation

RLM kernel (phases r1, r4, r5, r12):

  • rlm-mode experiment, nested under PTC; exclusive-only — enabling it forces the kernel-first narrowed toolset (supplement-mode RLM measured ~2x flat cost and was removed)
  • Persistent per-workspace mount: guest vars survives calls/turns/restarts via journaled snapshots
  • Kernel context isolation: nested shux.* results never enter model context (compact {tool, ok, bytes} summaries); the model's channels are its return value (offloaded via handles >16KB), capped console output, and vars
  • shux.load({path, key}): host-side bulk file ingestion straight into vars (record shows {key, bytes, lines, preview} only)
  • shux.task_spawn + shux.events(): fire-and-forget sub-agents with admission handles, asyncify-safe event drain
  • Batching guidance baked into the kernel-first preamble ("write complete programs")

Continual harness (r2, r6, r11):

  • Every memory/skill mutation journals an invertible refinement durable event (blob-backed inverses)
  • Rollback engine with rollbackOf lineage: shux run debug refinements CLI + RLM-gated refinement_rollback tool
  • /refine: bounded trajectory-distillation pass (dream-agent machinery) applying smallest evidence-backed edits, journaled and reversible

Agent ops (r3, r7, r8, r9):

  • Nuclear-family messaging: task_message_parent / task_message_sibling (RLM stamped on task records at spawn; strict same-parent scoping; server-side labels)
  • RLM-gated compaction keep-recent floor + cumulative read-file tracking
  • Branch summarization on fork/edit-resend (background generation, tail-guarded append)
  • scripts/gate_fingerprint.sh verification-loop memoizer

Measurement (scripts/rlm-eval/, make rlm-eval): scenario x config x seed A/B runner extracting mechanical metrics (tokens, cost, wall time, peak context, vars adoption, batch factor, compactions) from session artifacts.

Validation

  • Key measured results (sonnet-5 / opus-5 / gpt-5.6-sol; fable-5 at medium):
    • Context isolation: 504KB file load -> 867 bytes model-visible (0.17%); pre-fix the same task leaked 610KB into context and cost 10x flat tools
    • RLM-exclusive vs flat tools: -30 to -63% cost in 7/8 model x scenario pairs, faster in 6/8, all cells correct; organic vars adoption 15/16
    • Batching preamble (cross-build A/B): sonnet organic batch factor 2.7 -> 3.5 (3/4 seeds fold all 6 loads into one eval, -42% tokens)
  • Every phase passed an independent gate run + adversarial review + live dev-server-sandbox dogfood with replay-verify PASS (evidence in the workflow run reports)
  • Post-rebase onto the Shux rename: full static-check green; kernel suites (code_execution 50, toolBridge/typeGenerator 43, toolAssembly 14, sandboxHost 25) green; kernel surfaces adopt shux-primary naming with the mux.* alias intact

Risks

  • RLM-off regression risk is the headline concern and is heavily defended: composition tests pin byte-identity per flag combination, and replay-verify was run on live RLM-off control sessions at each phase. Highest-traffic shared code touched: toolAssembly, code_execution, compaction paths (RLM-gated), task spawn paths (flag stamping).
  • RLM-on surfaces are experimental by declaration; known rough edges: peak per-request context is higher when shux.load materializes large files (latent pressure on multi-MB corpora), and one sonnet seed still fragments batching.
  • /refine auto-applies edits (no approval UI in v1) — mitigated by journal + rollback + immutable-base guard rails.

Pains

  • The mid-series mux -> shux rename on main required conflict resolution across the kernel commits (namespace, type-generator identifiers, description text).
  • Sub-agent dogfooding infrastructure failures (background-monitor wakes, uncommitted-work timeouts, transient gateway model errors) shaped several workflow-hardening commits.

Generated with mux • Model: anthropic:claude-fable-5 • Thinking: xhigh • Cost: $763.80

@mintlify

mintlify Bot commented Aug 20, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
Mux 🟢 Ready View Preview Aug 20, 2026, 5:42 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

…inement journal, RLM mode experiment)

Conductor for implementing prime-agent-inspired RLM/continual-harness features behind an opt-in RLM sub-experiment of PTC. Mirrors workflows/track1-implementation.js: per-phase implement -> gate+adversarial-review -> fix rounds -> dogfood.
… code_execution

RLM Mode is an opt-in sub-experiment of Programmatic Tool Calling (flat
flag, gated on the PTC parent at call sites, nested under the PTC toggle
in Settings, mirroring the Memory Hot Set precedent). When enabled with
PTC and sandbox context, code_execution runs on the persistent
per-workspace kernel mount: the guest vars namespace survives across
calls/turns and restarts via snapshots, and the tool description
advertises those kernel semantics. MUX_SANDBOX_PERSISTENT_MOUNTS=1
remains a dev/test override with unchanged behavior. With the experiment
off (and env unset) behavior is byte-identical to before: fresh runtime
per call and today's description.

The rlm flag plumbs through the experiments path end to end:
ExperimentsSchema (send options) -> aiService.streamMessage ->
applyToolPolicyAndExperiments.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
Every mutating memory command (create/str_replace/insert/delete/rename) and
every agent_skill_write/agent_skill_delete now appends exactly one
'refinement' durable event to the acting workspace's session journal
(sharedDurableEventJournal), carrying an inverse payload that byte-exactly
restores the prior file state. Prior contents over 4KB are offloaded to the
session blob store (BlobRef), mirroring hook-context. Evidence records
{workspaceId, toolName, toolCallId?, actor?}.

Always-on and purely additive: journaling failures never fail the tool
(log.debug + continue), read-only ops and failed mutations write no rows.
Cross-workspace caveat (v1): memory/skill files are global/project-scoped
while the journal is per-session; rows land in the acting workspace's log.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
Standalone, always-on-by-usage gate memoizer: 'fingerprint' hashes HEAD sha +
'git diff HEAD' + sorted untracked-not-ignored files with content hashes;
'record <gate> <pass|fail>' and 'check <gate>' store/look up results in a JSON
file inside the worktree-local git dir (git rev-parse --git-path), so records
are never committed and never invalidate themselves.

wait_pr_ready.sh integration was skipped intentionally: it has no local
validation step (it only orchestrates remote Codex/review/CI gates), per the
phase brief's conditional.

Tests spawn the real script against hermetic temp git repos and cover
stability, pass/fail round-trip, tracked-edit / untracked-file / staged-change
invalidation, and corrupt-store self-healing.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…-handle events

Under an RLM persistent mount, nested mux.* results and code_execution
return values whose JSON serialization exceeds 16KB stop entering the
model context: the model-visible record becomes {handle, preview, size}
(plus a follow-up hint for return values) while the full value stays in
the guest at vars.__hN (monotonic per scope via vars.__handleSeq, so it
snapshots/restores with vars), in the content-addressed blob store, and
in one result-handle durable event whose preview mirrors the
model-visible string exactly. Handle bytes retained in vars are capped
with oldest-first eviction (never the newest handle); the blob remains
the durable copy. RLM off / ephemeral runtimes are byte-identical to
today.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
…escription)

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…ervice

finalizeAgentTaskReport now invokes sandboxHostService.postTaskTerminalEvent
(fire-and-forget, gated on no foreground waiters) so spawned-task completions
reach the guest host-event queue in production — previously the hook had zero
production callsites and mux.events() always drained empty. Regression tests
cover both the posted-event and waiter-suppression branches.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…h lineage

listRefinements/rollbackRefinement make the r2 journal actionable: rollbacks
apply the recorded inverse (inline or blob-backed) through atomic writes,
journal their own refinement row with rollbackOf (so double inversion works),
refuse already-rolled-back targets, refuse divergence (later overlapping rows,
deleted/recreated files, content drift for rollback rows) unless forced, and
confine every touched path to memory scope roots / skill directories with
lexical + symlink escape checks that force can never override.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…orce

Signed-off-by: Thomas Kosiewski <tk@coder.com>
Assembled in toolAssembly from the sandbox context inside the PTC branch, so
the tool only exists when RLM mode is on (nested under the PTC parent); with
the experiment off the toolset — and thus every provider request — stays
byte-identical. Force stays CLI-only: divergence overrides are a human call.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…refinement_rollback

Signed-off-by: Thomas Kosiewski <tk@coder.com>
… memory to current session

- P1: the later-rows divergence check now nets out rollback lineage: rows
  whose effect was itself rolled back are skipped, and live rollback chains
  conflict only when their parity re-applies an edit or rewinds past the
  target — so LIFO multi-edit unrolling works for model tool calls without
  force, while re-applied edits (rollback-of-rollback) still refuse.
- P2: workspace-scope memory confinement resolves strictly to the current
  session's memory root (<sessionDir>/memory) instead of any session subdir
  under sessionsDir, closing the cross-workspace write leak.

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…tCode on undefined assignment)

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…action)

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…oundary copies, read-file tracking

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…rendering

Signed-off-by: Thomas Kosiewski <tk@coder.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>
…est metadata, optional chain

Signed-off-by: Thomas Kosiewski <tk@coder.com>
…tch + staleness guard)

Signed-off-by: Thomas Kosiewski <tk@coder.com>
Signed-off-by: Thomas Kosiewski <tk@coder.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9b78f66e94

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/refinement/refineService.ts Outdated
Comment thread src/node/services/refinement/refineRunner.ts Outdated
Comment thread src/node/services/ptc/typeGenerator.ts Outdated
…, kernel types

- Memory inserts: staged refine inserts carry target fingerprints (numeric
  line positions have no content anchor), re-verified inside the memory
  mutation lock at apply — generalized from the r55 delete guard
  (fingerprintMutationTarget, expectedTargetFingerprints).
- refineRunner: the post-cancellation pendingToolRuns drain is bounded by
  the shared drain window once the pass signal aborts; wedged runs are
  handed to the usage-write registry for removal's bounded second chance.
- typeGenerator: LoadResult declares hookResult so TypeScript-analyzed
  kernel programs can branch on load annotations.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60fdb76b61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/refinement/refineRunner.ts
Comment thread src/node/services/refinement/targetMutationLocks.ts
Comment thread scripts/rlm-eval/run.ts Outdated
…l eval fixtures

- fileLock: live holders renew the lockfile mtime every lease/4 (stop-and-join
  at release), so lease-based reclaim on birth-less platforms can never
  displace a live-but-slow holder mid-mutation (double entry on the same
  target). Displaced holders never refresh a successor's lease.
- memoryService: mutations accept an abortSignal re-checked INSIDE the target
  mutation lock immediately before the first durable write; consolidation
  tool checks it at entry and threads it via executeMemoryCommand. Wired from
  runMemoryConsolidation (live) and refineRunner (defense in depth) so a
  detached wedged execution cannot commit durable memory or journal into a
  deleted session directory after teardown.
- rlm-eval: one fixture dir per (scenario, config, seed) cell, wiped before
  setup, so model-written intermediate files cannot leak across cells.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Round 59 fixes (e9eebc8):

  1. Detached refine tool writes after removal — memory mutations are now cancellation-aware end to end: createConsolidationMemoryTool accepts the pass's abort signal (checked at execute entry), threads it through executeMemoryCommand, and every MemoryService mutation (create/str_replace/insert/delete/rename) re-checks it INSIDE the target mutation lock immediately before the first durable write. A wedged execution that unblocks after teardown now refuses instead of committing durable memory or appending its refinement journal row into the deleted session directory. Wired from runMemoryConsolidation (the live-mutation flow) and refineRunner (defense in depth — note the refine run pass creates the tool with dryRun: true, so its executions stage in memory only and could not themselves commit durable writes; the live exposure was the consolidation flow sharing the same drain pattern). Behavioral tests: a delete wedged in pre-commit I/O + abort mid-wedge leaves the target intact and never materializes the session directory; a cancelled pass refuses new executions at entry.

  2. Live target-lock holders reclaimable past the leaseacquireProcessFileLock now renews the lease while held: an unref'd timer refreshes the lockfile mtime every lease/4 (token re-verified before each touch, stop-and-joined before release), so on birth-less hosts lease expiry now implies a crashed/frozen owner, never a live slow one — async-I/O stalls keep renewing because the event loop stays alive. The commit-point assertStillOwned doctrine remains for the residual event-loop-freeze window. Tests: a held lock aged 10 minutes past the lease is refreshed by renewal and renewal stops at release; a displaced holder never refreshes the successor's lease.

  3. Shared eval fixture dirs — each (scenario, config, seed) cell now gets its own fixture directory, wiped with rmSync(recursive, force) before setup, so model-written intermediate files (e.g. a generated .jsonl under shard-pipeline/shards) cannot leak into later cells and skew pass rates by execution order.

Validation: make static-check clean; fileLock, memoryService, tools/memory, memoryConsolidation(+Service), refinement, and branchSummary suites all pass (338 tests).

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9eebc8775

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/memoryConsolidation.ts
…olidation

Consolidation runs rode only a hard timeout: removal drained branch-summary
and refine producers but never aborted MemoryConsolidationService.inFlight,
so the r59 entry/pre-commit checks stayed false during removal and a
detached run could mutate global/project memory and append its refinement
row into the deleted session directory, recreating it.

- Each dream/harvest run now registers a per-workspace removal controller;
  its stream signal is AbortSignal.any(timeout, removal).
- cancelInFlightConsolidation aborts controllers and awaits runs bounded by
  USAGE_WRITE_DRAIN_WINDOW_MS; residuals are handed to the shared
  usage-write registry for the branch-summary drain's second chance, and
  anything beyond that refuses pre-commit via the aborted signal.
- Removal flows call it before clearPendingBranchSummary (both the main and
  phantom-metadata paths).
- Harvest inbox writes (workspace-scope saveFile/deletePath) now carry the
  signal into MemoryService's pre-commit check.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Round 60 fix (4659c9c) — workspace teardown now feeds consolidation cancellation:

  • Every dream/harvest run registers a per-workspace removal AbortController in MemoryConsolidationService (trackRunController, disposed when the run settles); its stream signal is now AbortSignal.any([AbortSignal.timeout(...), removalSignal]), so removal both aborts the provider stream and flips the r59 entry/pre-commit checks in the consolidation memory tool and MemoryService.
  • New cancelInFlightConsolidation(workspaceId) aborts the controllers and awaits inFlight + matching harvestInFlight runs bounded by USAGE_WRITE_DRAIN_WINDOW_MS; residual wedged runs are handed to the shared usage-write registry (drained by clearPendingBranchSummary immediately after in the removal flow), and anything that outlives even that refuses pre-commit inside the target mutation lock because its combined signal is aborted — no durable memory write or refinement-journal append can recreate the deleted session directory.
  • WorkspaceService removal calls it before clearPendingBranchSummary on both the main removal path and the phantom-metadata path (interface + setter extended; wired via the existing coreServices registration).
  • Harvest inbox persistence (writeInboxsaveFile, deleteInboxIfPresentdeletePath) now carries the signal too — the inbox is workspace-scope, so a detached write there would have recreated the session directory directly.

Behavioral test: an in-flight run on a hanging provider stream is aborted and drained by cancelInFlightConsolidation (settles as a stream failure instead of holding the in-flight lock until its multi-minute timeout), and the drain is idempotent when nothing is in flight.

Validation: make static-check clean; memoryHarvest, memoryService, tools/memory, memoryConsolidation(+Service) suites pass (189 tests).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4659c9c4f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/memoryConsolidationService.ts
Comment thread src/node/services/memoryConsolidationService.ts
Comment thread src/node/services/memoryService.ts Outdated
@chatgpt-codex-connector

This comment has been minimized.

…-serialized session deletion

- New workspaceRemoval.ts: a durable removal tombstone under <xumHome>/locks/
  published inside the memory target mutation locks immediately before the
  session directory is deleted (removeSessionDirUnderMemoryLocks). Deletion
  serializes with the workspace store root AND the coarse shared-memory key
  (whose mutations journal into this session dir); fail-closed on a wedged
  writer — the caller keeps the directory as a recoverable orphan rather
  than deleting it out from under a live commit.
- MemoryService: pre-commit checks (r59) upgraded to assertMutationCommittable,
  which also refuses when the durable tombstone exists — foreign backends in
  multi-instance mode observe removal at commit time. resolveStore refuses to
  materialize scope roots for removed workspaces so ensureRoot cannot
  recreate the session dir before the in-lock check.
- MemoryConsolidationService: teardown marks the workspace in a local
  cancelled set before aborting; maybeRun/maybeHarvestThenSweep refuse when
  teardown began (locally or via a foreign tombstone), and controllers
  registered afterwards start pre-aborted — post-harvest sweeps and
  retryable-harvest recovery can no longer start fresh un-aborted runs.
- WorkspaceService removal routes session-dir deletion through the locked
  helper.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Round 61 fixes (24a5f39) — explicit durable teardown tombstone + lock-serialized deletion, replacing reliance on the process-local registry:

  1. P1 — follow-on runs escape cancellation: cancelInFlightConsolidation now marks the workspace in a teardown set BEFORE aborting; maybeRun and maybeHarvestThenSweep (the funnels for the post-harvest sweep and retryable-harvest recovery) refuse with "workspace is being removed", and any controller registered after teardown starts pre-aborted. Test: after cancellation, a follow-on maybeRun refuses.

  2. P2 — cross-process teardown: new workspaceRemoval.ts publishes a durable removal tombstone (<xumHome>/locks/workspace-removed-<sha256>.json, atomic write) inside the memory target mutation locks immediately before the session directory is deleted. MemoryService's pre-commit check (now assertMutationCommittable, inside the same target locks at every mutating commit point incl. saveFile) refuses when the tombstone exists — a foreign backend's dream/harvest run observes removal at commit time even though the remover cannot abort it. Consolidation entry points also gate on the durable tombstone, and resolveStore refuses to materialize scope roots for removed workspaces so ensureRoot cannot recreate the session dir ahead of the in-lock check. Tombstones are retained (workspace IDs are never reused). Tests: tombstone alone (no abort signal) refuses create and the harvest-inbox saveFile without materializing the session dir; a foreign tombstone gates maybeRun.

  3. P2 — deletion vs. in-flight write: session-directory deletion now runs through removeSessionDirUnderMemoryLocks, which holds the workspace store root's target lock AND the coarse shared-memory key (global/project mutations journal into this session dir under that key) while publishing the tombstone and deleting. A write stalled inside store.writeFile() therefore either commits before deletion (and is deleted with the directory) or acquires its lock afterwards and refuses on the tombstone — it can no longer resume after deletion and recreate the directory. Lock acquisition stays fail-closed per the target-lock doctrine; on a genuinely wedged writer the removal keeps the directory as a recoverable orphan instead of deleting under a live commit. Test: deletion blocks behind a live writer holding the target lock, then tombstones and deletes after release.

Validation: make static-check clean; workspaceRemoval, memoryService, tools/memory, memoryConsolidation(+Service), memoryHarvest, refinement, fileLock, and branchSummary suites all pass (352 tests).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24a5f39393

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/workspaceRemoval.ts Outdated
Comment thread src/node/services/memoryService.ts Outdated
Comment thread src/node/services/memoryConsolidationService.ts
Comment thread src/node/services/workspaceRemoval.ts Outdated
Comment thread src/node/services/workspaceRemoval.ts Outdated
Comment thread src/node/services/workspaceRemoval.test.ts
@chatgpt-codex-connector

This comment has been minimized.

- removeSessionDirUnderMemoryLocks publishes the tombstone even when target
  lock acquisition fails: the caller deregisters the workspace regardless,
  so the terminal marker must become durable or a foreign backend would keep
  mutating the retained orphan session forever. Safe outside the locks on
  this path because the directory is NOT deleted.
- isWorkspaceRemovalTombstoned fails closed: only ENOENT means not-removed;
  other access errors refuse the commit (property-based error-code check).
- Root materialization moved INSIDE the target mutation lock: create/saveFile
  now run assertMutationCommittable then store.ensureRoot() in-lock, so an
  out-of-lock mkdir can no longer recreate a removed session dir as an empty
  orphan after removal's serialized deletion (resolveStore no longer takes
  createRoot).
- recordHeadlessUsage gates on the removal tombstone before its sidecar
  mkdir/append and ledger write — a foreign run's late usage callback cannot
  recreate the removed session directory.
- If config.removeWorkspace fails AFTER tombstone+session deletion, the
  tombstone is removed so the surviving registered workspace is not
  permanently refused memory mutations (missing session state self-heals);
  removal stays retryable.
- workspaceRemoval lock test uses an entry signal instead of a fixed sleep.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Round 62 fixes (a1ed863) — all six findings addressed:

  1. Tombstone on lock-timeoutremoveSessionDirUnderMemoryLocks now publishes the tombstone even when target-lock acquisition fails (the caller deregisters the workspace regardless, so the terminal marker must become durable). Publishing outside the locks is safe on that path precisely because the directory is NOT deleted: a mid-commit writer lands in the retained orphan, and every later mutation observes the tombstone. Test: a held foreign file lock → removal throws, directory retained, tombstone durable.

  2. In-lock root materializationresolveStore no longer materializes roots at all; create and saveFile run assertMutationCommittable then store.ensureRoot() INSIDE the target mutation lock, so the mkdir serializes with removal's locked deletion and cannot recreate a removed session directory as an empty orphan. The r61 advisory pre-lock check was removed along with the createRoot option.

  3. Usage-write gaterecordHeadlessUsage checks the durable tombstone before its sidecar mkdir/append (headless-usage.jsonl) and ledger write (session-usage.json), returning undefined for removed workspaces. Dropping the spend row is correct: the workspace whose dashboards it would feed no longer exists. Test: tombstone → no write, no session directory materialized.

  4. Fail-closed existence checkisWorkspaceRemovalTombstoned returns false only on provable ENOENT (property-based error-code check, realm-safe); any other access failure reports removal so a writer never commits into a possibly-deleted directory it cannot verify.

  5. Config-failure recovery — if config.removeWorkspace() fails after tombstone + session deletion, the tombstone is deleted before rethrowing, so the surviving registered workspace is not permanently refused memory mutations (its missing session state self-heals on demand per the crash-resilience doctrine) and removal stays retryable. In-process consolidation stays cancelled until restart, matching the documented drained-producers tradeoff.

  6. Test entry signal — the lock-serialization test now awaits an entered promise resolved inside the writer's locked callback before starting removal, instead of relying on a fixed sleep.

Validation: make static-check clean; workspaceRemoval, sessionUsageService, memoryService, tools/memory, memoryConsolidation(+Service), memoryHarvest, refinement, fileLock, and branchSummary suites all pass (380 tests).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1ed863f48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/sessionUsageService.ts Outdated
Comment thread src/node/services/workspaceRemoval.ts Outdated
Comment thread src/node/services/workspaceService.ts Outdated
Comment thread src/node/services/workspaceRemoval.ts Outdated
@chatgpt-codex-connector

This comment has been minimized.

…le tombstones, startup self-heal

- History write lock relocated to <xumHome>/locks/history-<sha>.lock (was
  inside the session dir, so removal deleted it out from under a foreign
  backend's in-flight append). Removal's critical section now also holds
  this lock, and withCrossProcessWriteLock checks the removal tombstone
  IN-LOCK before ensurePrivateDir — a late append fails instead of
  recreating the deleted session directory.
- recordHeadlessUsage runs its tombstone gate AND its sidecar/ledger commits
  inside the session-dir target mutation lock that removal's tombstone+delete
  section also holds, closing the check-then-write window.
- removeSessionDirUnderMemoryLocks throws TombstoneNotDurableError when no
  marker can be published on either attempt (e.g. ENOSPC); WorkspaceService
  aborts the removal in that case (workspace stays registered + retryable)
  instead of deregistering a workspace whose orphan session would become
  writable again.
- Tombstone rollback failure after config deregistration failure is logged
  loudly, and a startup self-heal (healRemovalTombstonesForRegisteredWorkspaces,
  age-gated 10min) reclaims tombstones for still-registered workspaces so
  that failure residue cannot brick a workspace across restarts.
- workspaceRemoval lock test uses an entry signal instead of a fixed sleep.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

Round 63 fixes (a5074c2) — all four findings addressed:

  1. Usage gate serializationrecordHeadlessUsage now runs its tombstone check AND its sidecar mkdir/append + ledger commits inside the session-dir target mutation lock, which removeSessionDirUnderMemoryLocks also holds (new third key in its sorted acquisition). The check can no longer go stale between preflight and write. Usage recorders never hold memory target locks, so the single-key acquisition cannot ABBA with removal's sorted multi-key acquisition.

  2. Abort deregistration on non-durable tombstones — the fail-closed orphan path now distinguishes outcomes: if the out-of-lock publication also fails (e.g. ENOSPC), it throws TombstoneNotDurableError, and WorkspaceService aborts the removal (workspace stays registered and retryable) instead of deregistering a workspace whose retained orphan would become writable again once the transient failure clears. Test: a blocked locks dir → TombstoneNotDurableError, session dir retained.

  3. Non-silent tombstone rollback + self-heal — a failed compensating rollback is now logged at error level, and a startup self-heal (healRemovalTombstonesForRegisteredWorkspaces) reclaims removal tombstones for still-registered workspaces, so the registered-but-tombstoned residue cannot brick a workspace across restarts. Age-gated (10 min) so another backend's healthy in-flight removal — which is momentarily registered+tombstoned between publication and deregistration — is never healed mid-flight. Wired into the WorkspaceService constructor with an explicit catch (startup must never crash). Test: old registered tombstone healed; fresh and deregistered tombstones retained.

  4. History append serialization — the cross-process history write lock moved OUT of the session directory to <xumHome>/locks/history-<sha256>.lock, removal's critical section acquires it alongside the target locks, and withCrossProcessWriteLock checks the removal tombstone in-lock BEFORE ensurePrivateDir (which itself moved inside the lock — it previously ran before acquisition and could recreate the directory during removal's critical section). A foreign backend's late append now fails with a normal Err instead of recreating the deleted session directory. Test: tombstoned workspace → appendToHistory refuses, no session dir materialized; the existing foreign-lock contention test was updated to the new lock path.

Validation: make static-check clean; workspaceRemoval, historyService, sessionUsageService, replay, hookService, memory (all), fileLock, branchSummary, and refinement suites pass (632 tests across the battery).

# Conflicts:
#	src/node/services/workspaceService.ts
The desktop-cleanup removal tests' Partial<Config> stub predates r61-63,
where removal derives lock/tombstone paths from config.rootDir — the locked
deletion threw on the undefined rootDir and the test's session dir survived.
Also assert a non-empty rootDir in removeSessionDirUnderMemoryLocks so a
malformed config crashes clearly instead of surfacing as a path.resolve
TypeError inside the lock-key derivation.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c51aeb2b61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/historyService.ts Outdated
Comment thread src/node/services/refinement/refineService.ts
Comment thread src/node/services/branchSummary.ts
…al, and DevTools writes (Codex r64)

- History truncation recovery now runs INSIDE the cross-process write lock:
  an unlocked recovery could mistake another backend's live truncation for a
  crash and roll it back mid-flight, resurrecting discarded history with
  mismatched archive/chat state. Read paths probe for recovery artifacts
  without the lock and only lock when artifacts exist, so clean reads stay
  lock-free; removal-tombstoned workspaces skip recovery.
- /refine apply now requires the hash of the proposal the CALLER'S renderer
  displayed. The shared transcript alone cannot prove what this user saw:
  a foreign backend's /refine can replace refine-staged.json and append a
  newer proposal row only its own renderer displayed, letting the existing
  staged-file/transcript checks pass against unseen bytes.
- DevTools devtools.jsonl commits (append + clear) run inside the sessionDir
  target mutation lock with an in-lock removal-tombstone recheck, so a
  foreign backend's step finalization cannot recreate a removed session
  directory via mkdir.
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review
Please take another look.

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da47046aa6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

collapsed.map(async (edit) => {
if (edit.tool !== "agent_skill_write") return edit;
const targetContentHash = await this.fingerprintSkillWriteTarget(projectRoot, edit.input);
return targetContentHash === undefined ? edit : { ...edit, targetContentHash };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Drop skill edits when fingerprinting fails

When staging an agent_skill_write against a target that cannot be fingerprinted, such as during a transient EACCES or read error, this branch retains the edit without targetContentHash. If access recovers and the file changes before /refine apply, the staged writer receives no expected hash and overwrites the newer contents. Drop or refuse the skill edit when its fingerprint cannot be produced, matching the fail-closed memory-edit path.

Useful? React with 👍 / 👎.

Comment on lines +230 to +232
if (Date.now() - parsed.removedAt < REMOVAL_TOMBSTONE_HEAL_MIN_AGE_MS) continue;
if (config.findWorkspace(parsed.workspaceId) == null) continue;
await fsPromises.rm(filePath, { force: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve tombstones for removals that are still active

With XUM_ALLOW_MULTIPLE_INSTANCES=1, if removal remains between session deletion and config.removeWorkspace() for over ten minutes—for example while MCPServerManager.stopServers() awaits a wedged instance.close()—a newly started backend sees an old tombstone plus a still-registered workspace and deletes the marker here even though removal is still active. Foreign history or sidecar writers can then pass their durable removal gate and recreate the session directory before deregistration completes. Tombstone age is not proof of a failed removal; distinguish rollback residue explicitly or coordinate healing with the live removal/config transition.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant