fix(harness): preserve regenerated session configuration during resume

[BYWallace]

Problem and motivation

After a session ends, Studio can still send updates that say “ended.” Each update can start another deletion. An old deletion can then remove new configuration files that Resume just wrote.

Summary and scope

Delete once per run, and wait for that deletion to finish before Resume writes new files. Repeated status updates no longer start extra deletions. This applies to Claude, Codex, and other harnesses.

If resume setup fails after writing files but before the agent starts, normal exit cleanup removes those files too.

Also repairs a missing credential-path mock and removes fixed timing assumptions from two CI tests.

Before

flowchart LR
    B1["Repeated ended updates<br/>start extra deletions"] --> B2["Resume writes new files<br/>before deletion finishes"]
    B2 --> B3["Old deletion removes<br/>the new files"]
    style B3 fill:#fff1f2,stroke:#be123c,color:#881337

Loading

With this fix

flowchart LR
    F1["Delete once<br/>when the run ends"] --> F2["Wait for deletion<br/>to finish"]
    F2 --> F3["Resume writes<br/>new files"]
    style F2 fill:#ecfdf5,stroke:#047857,color:#064e3b
    style F3 fill:#ecfdf5,stroke:#047857,color:#064e3b

Loading

Related work

Independent cleanup fix. The Codex MCP connection fix (#810) and its tests (#879) are already merged.

Validation

• All 58 cleanup, authentication, Codex reader, and child-session tests pass.
• Both timing failures reproduce with delayed file setup before the fixes and pass after them.
• Previous full harness run: 4,043 passed, 2 skipped. Two local file-watcher failures also occur on unchanged main.
• All 10 performance tests, server build, type check, and lint passed.

Diff, checklist, and release details

Changed area	What changes	Additions
Server + session manager	Documented cleanup guard and resume failure cleanup	28
Tests	Cleanup regressions and CI test setup fixes	165
Changeset	Release note	5
Total		198

Primary change type

• Bug fix
• Documentation
• Feature
• Tests
• Dependency update
• Maintenance or refactor

Tests and documentation

Extended the generated-configuration retention tests. README update is N/A: this restores existing cleanup behavior; the changeset records the fix.

Compatibility and release impact

• Breaking or externally visible changes: resumed sessions retain their regenerated configuration. Failed preparation clears the prior exit code and tail while preserving the last activity timestamp. No migration required.
• Changeset: added an @sapiom/harness patch changeset.

Security

• I have not included secrets, credentials, private data, or unsanitized logs.
• This pull request does not publicly disclose a suspected vulnerability.

AI assistance

• I did not use AI assistance for this change.
• I used AI assistance and have described it below.

Codex implemented the cleanup guard and regressions, then isolated this change from #810. Verified with diff review, lifecycle tests, build, typecheck, and lint on this branch.

Checklist

• I read CONTRIBUTING.md and followed the contribution policy.
• This pull request addresses one focused problem.
• I added or updated tests.
• I ran the relevant build, typecheck, lint, and test commands as described above.
• I updated documentation, or marked it N/A above.
• I added a Changeset.
• I can explain and maintain every submitted change, including any AI-assisted work.

Summary by CodeRabbit

• Bug Fixes
  • Fixed session resume so regenerated configuration is no longer removed prematurely by repeated exit-status updates.
  • Improved resume behavior for sessions restored after restart or imported from history.
  • Failed resume preparation now returns sessions to an exited state while cleaning up generated configuration correctly.
  • Subsequent resume attempts can regenerate configuration and complete cleanup reliably.

[github-actions]

Review — PR #812 (fix(harness): preserve regenerated session configuration during resume)

Confidentiality: the changeset and all new comments are generic ("sessions restored after
restart or imported from history"); no company names, no internal hosts, no business terms.
Patch level is right for a bug fix, no dependency/package.json/exports/frontend changes.
One correctness finding.

1. A resume that fails after buildLaunchOpts leaves the guard set forever, so the regenerated dir (API key + Agent Map bearer token) is never cleaned at exit

packages/harness/src/server/index.ts:1143 adds the session id to
generatedCleanupScheduled whenever buildLaunchOpts runs on an exited session. The set
is only ever cleared on a transition to starting/running (:1558). Between those two
points, resume() can throw without any status change:

• packages/harness/src/server/index.ts:1150 — throw new Error("Agent Map MCP endpoint is not bound"), thrown by the wrapper itself after the add.
• session-manager.ts — spec = adapter.resume(session.agentSessionId, opts) is wrapped in
  a try/catch that rethrows, and it runs after buildLaunchOpts and before
  session.status = "starting". (Every failure after that line goes through
  transitionExited, so those are fine — the guard was already cleared by starting.)

Failure scenario: user clicks Resume on an exited session, adapter.resume() throws (bad
spawn spec, unbound Agent Map endpoint). buildLaunchOpts has already rewritten
generated/<id>/mcp-config.json, which carries x-api-key and, on the resume path, a
freshly rotated Agent Map Bearer token (core/inject/mcp-config.ts:93,124). The session
stays exited, so no further transition ever occurs — and every later exited metadata
broadcast now hits if (generatedCleanupScheduled.has(session.id)) return at :1607 and
does nothing. Before this PR one of those repeated broadcasts was exactly what removed the
dir; now the only remaining reaper is the age-gated boot sweep
(GENERATED_SWEEP_MAX_AGE_MS, 7 days), so live credentials sit on disk for at least a week.
The PR's own test comment ("rather than retaining credentials indefinitely") names the risk
but only covers the happy path.

Suggested fix — same behaviour, no stuck state: track the previous status per session
(Map<string, SessionStatus> updated at the top of onStatusChange) and schedule removal
only on an actual !== "exited" → "exited" transition. That kills the repeated-broadcast
race at its root, covers restored/imported history without the buildLaunchOpts write-back
at :1143, and cannot be wedged by a resume that never starts. If you keep the current
shape instead, clear the id from the set when the resume throws (wrap the add so a
downstream failure releases it) and add a test for adapter.resume() throwing.

Verdict

Request changes — the guard fixes the reported race but introduces a narrower path where a
failed resume permanently suppresses cleanup of a credential-bearing directory.

[github-actions]

Follow-up review — PR #812 (delta since afe904d1)

One commit moved since the last review: c9b535bb.

Earlier findings

1. Failed resume leaves the cleanup guard set forever → credential-bearing
generated/<id> survives — fixed. The fix takes the root-cause route rather than the
suggested one: buildLaunchOpts + adapter.resume moved inside the try after
session.status = "starting" and emitStatus (core/session-manager.ts:850-861), and the
speculative generatedCleanupScheduled.add() in the buildLaunchOpts wrapper is gone
(server/index.ts:1138). So the "starting" broadcast clears the guard before any file is
written, repeated exited broadcasts during generation no longer match (session.status is
starting), and every failure — including the wrapper's own "Agent Map MCP endpoint is not
bound" throw and a throwing adapter.resume — now lands in the catch → transitionExited
→ a real removal. Verified against the surviving paths: canResume/resolveAgentMapIdentity
throw before the flip and write nothing; task ids never hit the removed get()?.status
branch; SpawnSpec is still imported for create(). The dropped explicit
onAgentMapSessionExit call is not a loss — transitionExited makes the same call
(core/session-manager.ts:2014). The new it.each covers both failure sites plus retry and
next-lifetime cleanup.

Confidentiality clean: the appended changeset sentence ("Failed resume preparation also
cleans up regenerated configuration.") names no company, host, or business arrangement.
Still patch-level, still no API-surface, dependency, or frontend change.

New

Nit — transitionExited clears exitTail/exitCode, so a buildLaunchOpts or
adapter.resume failure now wipes the previous crash's exit tail from the dead pane where
before the push it was preserved; consistent with the other pre-spawn failures, but it is an
externally visible delta the PR body does not mention.

Note: I could not execute vitest in this environment, so the new tests are reviewed, not run.

Verdict

Approve — the guard hole is closed at the root; nothing new blocking.

[BYWallace]

The failed-resume finding was valid. Preparation now follows the existing exit path, so a builder or adapter failure removes generated configuration.

Simplified the guard in 9f7ecee: the removal promise itself prevents duplicate cleanup and is cleared after the next configuration build awaits it. The extra Set and reset callbacks are gone. All 202 lifecycle and retention tests pass, including preparation failure, retry, and metadata broadcasts after deletion completes.

The description also notes the visible change from the follow-up review: failed preparation clears the old exit code and tail while preserving the previous activity timestamp.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9a669e78-6b3d-4643-ae9b-814d030633b3

📥 Commits

Reviewing files that changed from the base of the PR and between fd9fe9d and ebe263d.

📒 Files selected for processing (5)

• packages/harness/src/core/collector/codex-tailer.test.ts
• packages/harness/src/core/session-manager.ts
• packages/harness/src/server/definition-list-enrichment.test.ts
• packages/harness/src/server/generated-retention.test.ts
• packages/harness/src/server/index.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/harness/src/core/session-manager.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The harness now persists the starting session state before resume preparation, tracks generated-directory cleanup across exit notifications, protects regenerated configuration during resume, and cleans up after failed preparation. Tests cover restart, history adoption, repeated broadcasts, and retry behavior.

Changes

Session configuration retention

Layer / File(s)	Summary
Resume lifecycle state packages/harness/src/core/session-manager.ts, packages/harness/src/core/session-manager.test.ts	resume() persists and emits the starting state before preparation. Preparation failures now transition through the exited-state cleanup path. The test expects cleared exit data and preserved session activity.
Generated-directory cleanup packages/harness/src/server/index.ts	Cleanup promises remain tracked for each session lifetime. Repeated exited notifications do not schedule duplicate removal. Resume preparation awaits and clears prior cleanup before generating configuration.
Retention behavior validation packages/harness/src/server/generated-retention.test.ts, packages/harness/src/core/collector/codex-tailer.test.ts, packages/harness/src/server/definition-list-enrichment.test.ts, .changeset/session-config-cleanup.md	Tests cover repeated broadcasts, restart and adopted-history resumes, failed preparation, successful retries, deterministic tailer events, and credentials-file setup. A patch release changeset documents the fix.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to ebe26

Resume now coordinates generated-configuration cleanup with regeneration and cleans up failed preparation attempts. The covered lifecycle paths show no remaining merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant SessionManager
  participant Server
  participant GeneratedDirectory
  SessionManager->>Server: emit exited status
  Server->>GeneratedDirectory: schedule removal once per lifetime
  SessionManager->>Server: resume session
  Server->>GeneratedDirectory: await removal before rebuilding configuration
  Server->>SessionManager: return prepared launch options

Loading

Suggested reviewers: gwitwer, evtran0209

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the primary bug fix: preserving regenerated session configuration during resume.
Description check	✅ Passed	The description is complete, focused, and follows the required structure. It explains the problem, scope, related work, validation, release impact, security, AI assistance, and checklist status. The v…

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch brettwallace/session-config-cleanup

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit hops where sessions start,
And clears old trails with careful art.
Exit bells ring, but files stay bright,
Resume paths tidy them just right.
Config blooms after cleanup’s tune,
Then ears perk up beneath the moon.}

_{Comment @coderabbitai help to get the list of available commands.}

[ynadge]

Good catch!