Skip to content

fix(daemon): explicit abort message on uncommitted repair close#1383

Merged
thymikee merged 4 commits into
callstack:mainfrom
jibin7jose:fix-issue-1380
Jul 24, 2026
Merged

fix(daemon): explicit abort message on uncommitted repair close#1383
thymikee merged 4 commits into
callstack:mainfrom
jibin7jose:fix-issue-1380

Conversation

@jibin7jose

Copy link
Copy Markdown
Contributor

Fixes #1380

Problem

In the repair study, several agent failures were caused by the agent running close --save-script mid-repair (without first committing via replay --from). Because this resulted in a silent no-op ({ written: false } with no error), the agents didn't realize the script wasn't written and retried in a loop, wasting their budget.

Solution

  • Loud Abort: Modified commitRepairBeforeClose in session-close.ts to return an explicit { kind: 'aborted' } when saveScriptComplete is false.
  • Clear Guidance: Updated the close handler to check for repair.aborted alongside --save-script. If triggered, it appends a loud, explicit warning describing exactly how to recover: "The repair was aborted and no script was written. Recovery: replay --from <n> --plan-digest <digest> before closing.".
  • Docs Update: Added the abort-on-close rule explicitly to the agent-device help workflow documentation (in src/cli/parser/cli-help.ts) right next to the resume step.

@thymikee

Copy link
Copy Markdown
Member

Love it that you've dug it from the benchmark, thank you!

@thymikee

Copy link
Copy Markdown
Member

Review finding:

  • [P1] Aborted repair still exits successfully. session-close.ts only appends warning text, while both response paths still return ok: true; the CLI therefore exits 0 when close --save-script writes no artifact. That misses close --save-script on an uncommitted repair declines silently — make the abort loud #1380's explicit acceptance that the exit reflect that nothing was written. After teardown, return a typed/non-success abort result that preserves the abort and recovery guidance. Add a production-route regression proving the nonzero/error outcome and no artifact, plus coverage that plain close and committed-repair close remain unchanged.

CI also needs owner action: Lint & Format fails because src/daemon/handlers/session-close.ts does not pass pnpm format:check; all other current checks pass.

@thymikee

Copy link
Copy Markdown
Member

Re-review at 72067d47: the prior exit-status finding is fixed. The aborted close --save-script now tears down per ADR 0012 and returns ok: false with the requested abort/recovery guidance. The new provider-route test is non-vacuous and covers aborted, plain, and committed close paths.

The head is still not ready for validation:

  • armRepair(daemon: any, ...) never uses daemon; with noUnusedParameters, typecheck will fail.
  • let selection in the test and let text in session-close.ts are never reassigned, violating prefer-const.
  • the test has trailing whitespace at lines 68, 83, and 85, so format will fail.
  • all six exact-head workflows are action_required; an owner must authorize them after these static failures are fixed.

No ready label yet. Do not apply labels.

@thymikee

Copy link
Copy Markdown
Member

Re-review at a06a9120: the prior static blockers are fixed. The unused helper parameter is gone, the two immutable bindings now use const, and the patch is whitespace-clean. The functional abort result and its provider-route coverage remain sound; no new code finding on this delta.

Validation is still owner-blocked: CI, Linux, macOS, Android, iOS, and Size are all action_required, with no checks reported on this exact head. Please authorize the workflows. No ready-for-human label until those owner-action failures are cleared.

Do not apply labels.

@thymikee
thymikee merged commit 968db8b into callstack:main Jul 24, 2026
19 checks passed
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.

close --save-script on an uncommitted repair declines silently — make the abort loud

2 participants