feat: checkpoint-and-resume so the turn cap stops discarding completed work #356

Merged

joshtronic merged 1 commit from feat/checkpoint-resume into master

2026-07-05 22:32:50 +00:00

igor commented

2026-07-05 22:13:47 +00:00

Collaborator

The problem

--max-turns 100 is meant to bound a tick's runtime (no single issue runs for hours and starves the loop). But the ship-safety gate discarded the entire worktree on the nonzero max-turns exit — throwing away real, commit-worthy work and re-running large tasks from zero, only to hit the cap again next tick (igor#351, #329, #334). The cap was forbidding work from ever finishing, not just bounding runtime.

The design: checkpoint-and-resume (tier-1 issue work)

After the claude run, the worktree's disposition is one of three (pure logic in lib/checkpoint.sh):

disposition	when	action
`commit`	exit 0	finalize the PR (existing flow)
`checkpoint`	a clean `max_turns` result event, no unrestored `git stash`	snapshot the WIP + resume next tick
`discard`	a real crash; or a turn cap left with a stash we can't commit over	the existing ship-safety discard

A checkpoint:

commits the in-progress work and publishes it as a draft (WIP:) PR the review + merge loops skip;
keeps the issue claimable (unassigns the bot), and on the next claim resumes — the worktree is carved from the checkpoint branch (not the base) and Claude gets a "continue, don't restart; your WIP is already committed" directive;
on natural completion (exit 0) drops the WIP: prefix and writes a clean title/body → shadow review picks it up.

Guardrails

Ship-safety preserved. Only a clean max_turns result event with no unrestored stash earns a checkpoint; a real crash still discards (the porksicle#114 invariant). A crash mid-resume preserves the prior (clean) checkpoint and re-queues.
Resume budget. CHECKPOINT_MAX=8, tracked as  in the PR body. Past it, the issue is escalated to the human with Status/Blocked ("too big — split it"), so a non-converging task can't monopolize the loop. Both checkpoints and crash-mid-resumes count against it.
Finalize gates unchanged. Scope, security, vacuous-test, off-limits run only on completion — an incomplete snapshot isn't judged. The commit-count cap now excludes the harness's own WIP: … checkpoint commits so a completed multi-checkpoint task isn't blocked on history it didn't choose. The 400-line cap is orthogonal and still applies to the whole diff — a task that's both turn-expensive and >400 net lines will checkpoint through the turns then block at finalize on scope, which is the correct "human, split this" signal.

The cap stays hardcoded at 100 — it's now a per-tick budget, not a hard ceiling on total work.

Touch points

lib/checkpoint.sh (new, pure) — disposition, max_turns stream detection, WIP-title + counter helpers, budget.
bin/tick.sh — resume detection at claim; worktree-from-branch on resume; resume preamble; disposition routing; checkpoint push + draft-PR; WIP-drop finalize; discovery gate resumes a WIP (not skip); review tick skips WIP; scope-cap excludes checkpoint commits.
lib/forgejo.sh — bot_prs_for_issue returns title; new forgejo_edit_pr.
lib/automerge.sh — excludes WIP drafts.
CLAUDE.md — documents the new subsystem.

Testing

bin/test-checkpoint.sh — 46 assertions (disposition matrix incl. the stash guard, max_turns detection, WIP round-trip, body counter, budget, the exact discovery-gate/resume jq filters, forgejo_edit_pr payload). make test + shellcheck (warning+) clean; SC2015 count unchanged from baseline.

Review notes

This edits the harness's own core tick flow and self-deploys from master in ~1 min, so it's a human-review-and-merge change — I built it, I'm not merging it. Suggest a read of the disposition matrix + the resume lifecycle first. The riskiest bits are the exit-handling rewrite (tick.sh disposition block) and the discovery-gate/resume detection; both are covered by the exact jq filters in the test.

## The problem `--max-turns 100` is meant to bound a tick's **runtime** (no single issue runs for hours and starves the loop). But the ship-safety gate discarded the **entire worktree** on the nonzero max-turns exit — throwing away real, commit-worthy work and re-running large tasks from zero, only to hit the cap again next tick (igor#351, #329, #334). The cap was forbidding work from ever *finishing*, not just bounding runtime. ## The design: checkpoint-and-resume (tier-1 issue work) After the claude run, the worktree's disposition is one of three (pure logic in `lib/checkpoint.sh`): | disposition | when | action | |---|---|---| | `commit` | exit 0 | finalize the PR (existing flow) | | `checkpoint` | a **clean** `max_turns` result event, **no** unrestored `git stash` | snapshot the WIP + resume next tick | | `discard` | a real crash; or a turn cap left with a stash we can't commit over | the existing **ship-safety discard** | A `checkpoint`: 1. commits the in-progress work and publishes it as a **draft (`WIP:`) PR** the review + merge loops **skip**; 2. keeps the issue **claimable** (unassigns the bot), and on the next claim **resumes** — the worktree is carved from the checkpoint branch (not the base) and Claude gets a *"continue, don't restart; your WIP is already committed"* directive; 3. on natural completion (`exit 0`) drops the `WIP:` prefix and writes a clean title/body → **shadow review** picks it up. ## Guardrails - **Ship-safety preserved.** Only a clean `max_turns` result event with **no unrestored stash** earns a checkpoint; a real crash still discards (the porksicle#114 invariant). A crash *mid-resume* preserves the prior (clean) checkpoint and re-queues. - **Resume budget.** `CHECKPOINT_MAX=8`, tracked as `` in the PR body. Past it, the issue is escalated to the human with `Status/Blocked` ("too big — split it"), so a non-converging task can't monopolize the loop. Both checkpoints and crash-mid-resumes count against it. - **Finalize gates unchanged.** Scope, security, vacuous-test, off-limits run only on **completion** — an incomplete snapshot isn't judged. The **commit-count** cap now excludes the harness's own `WIP: … checkpoint` commits so a completed multi-checkpoint task isn't blocked on history it didn't choose. The **400-line** cap is orthogonal and still applies to the whole diff — a task that's both turn-expensive *and* >400 net lines will checkpoint through the turns then block at finalize on scope, which is the correct "human, split this" signal. The cap **stays hardcoded at 100** — it's now a per-tick budget, not a hard ceiling on total work. ## Touch points - `lib/checkpoint.sh` (**new**, pure) — disposition, `max_turns` stream detection, WIP-title + counter helpers, budget. - `bin/tick.sh` — resume detection at claim; worktree-from-branch on resume; resume preamble; disposition routing; checkpoint push + draft-PR; WIP-drop finalize; discovery gate resumes a WIP (not skip); review tick skips WIP; scope-cap excludes checkpoint commits. - `lib/forgejo.sh` — `bot_prs_for_issue` returns `title`; new `forgejo_edit_pr`. - `lib/automerge.sh` — excludes WIP drafts. - `CLAUDE.md` — documents the new subsystem. ## Testing `bin/test-checkpoint.sh` — 46 assertions (disposition matrix incl. the stash guard, `max_turns` detection, WIP round-trip, body counter, budget, the exact discovery-gate/resume jq filters, `forgejo_edit_pr` payload). `make test` + `shellcheck` (warning+) clean; SC2015 count unchanged from baseline. ## Review notes This edits the harness's **own** core tick flow and **self-deploys from master in ~1 min**, so it's a human-review-and-merge change — I built it, I'm not merging it. Suggest a read of the disposition matrix + the resume lifecycle first. The riskiest bits are the exit-handling rewrite (`tick.sh` disposition block) and the discovery-gate/resume detection; both are covered by the exact jq filters in the test.

igor added 1 commit

2026-07-05 22:13:47 +00:00

feat: checkpoint-and-resume so the turn cap stops discarding completed work

Lint / check-sync (push) Successful in 5s

Details

Lint / check-sync (pull_request) Successful in 4s

Details

756de410e4

The `--max-turns 100` cap is meant to bound a tick's RUNTIME (no single
issue runs for hours and starves the loop). But the ship-safety gate
discarded the ENTIRE worktree on the nonzero max-turns exit -- throwing
away real, commit-worthy work and re-running large tasks from zero, only
to hit the cap again next tick (igor#351, #329, #334). The cap was
forbidding work from ever finishing, not just bounding runtime.

Now a CLEAN max-turns cut-off CHECKPOINTS instead of discarding:
- snapshots the work-in-progress as a commit and publishes it as a
  draft ("WIP:") PR that the review + merge loops SKIP;
- keeps the issue claimable, and RESUMES from that branch on the next
  claim with a "continue, don't restart" directive to Claude;
- on natural completion (exit 0) drops the WIP prefix + writes a clean
  title/body, marking the PR ready -> shadow review picks it up.

Guardrails: a real crash still discards (the porksicle#114 ship-safety
invariant + stash guard are preserved -- only a clean max_turns result
event with no unrestored stash earns a checkpoint). A resume budget
(CHECKPOINT_MAX=8, tracked in the PR body) caps how many times one issue
may checkpoint before it's escalated to the human with Status/Blocked,
so a non-converging task can't monopolize the loop. The finalize-time
gates (scope, security, vacuous-test, off-limits) run only on
completion; the commit-count cap excludes checkpoint commits so a
completed multi-checkpoint task isn't blocked on history it didn't
choose.

The cap stays hardcoded at 100 -- it's now a per-tick budget, not a
hard ceiling on total work.

New pure/unit-tested core in lib/checkpoint.sh (bin/test-checkpoint.sh,
46 assertions). make test + shellcheck (warning+) clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YUVfbfUWdUwifGJ2TvkP

igor requested review from joshtronic

2026-07-05 22:13:47 +00:00

igor commented

2026-07-05 22:15:37 +00:00

Author

Collaborator

🤖 Review — `COMMENT` (automated)

CI for 756de410: success

Summary: Adds turn-cap checkpoint-and-resume: a clean max_turns cut-off now snapshots WIP as a draft WIP: PR and resumes next tick instead of discarding the worktree. New pure lib/checkpoint.sh + 46-assertion unit test, with the git/forgejo side effects wired into bin/tick.sh. CI is green, the checklist matches the diff, no security issues, and I found no clear logic defect. But this is a >400-line change to the harness's core tick flow that self-deploys from master, and its most load-bearing seams can't be verified from the diff alone — that belongs in front of a human (which the author themselves states). Handing it over rather than approving.

Things a human must confirm before merge:

The stream file path is the crux and I can't verify it. CLAUDE_STREAM="$(dirname "$CLAUDE_LOG")/claude-stream.jsonl" (tick.sh ~line 4085) — but the diff only shows CLAUDE_LOG=".agent/claude-output.log"; the claude invocation that would produce claude-stream.jsonl isn't in the diff. If that file isn't actually written at that path, checkpoint_hit_turn_cap always returns false, every turn cap silently falls back to discard, and the entire feature is a no-op — with no failing test to catch it (the unit tests feed hand-built fixtures, not a real run). Confirm the live claude run emits stream-json to exactly that path.
Integration is unavoidably untested. checkpoint.sh is pure and well-covered, but the risky part — disposition routing, resume worktree carve, push/--force-with-lease, draft-PR open/finalize, budget escalation — lives in tick.sh and has no end-to-end coverage. The jq filters are pinned in tests, the control flow around them is not.
Crash-mid-resume drops that session's committed work (by design, verify it's intended). In the discard && IS_RESUME branch (tick.sh ~line 4110) the code assumes "HEAD == origin/$BRANCH, nothing to push." If Claude made real commits during the resume and then crashed, those commits are on local HEAD, never pushed, and lost when the worktree is pruned — only the prior checkpoint survives. This matches the stated "crash mid-resume preserves the prior checkpoint" guardrail and is bounded by the budget, so it won't loop, but it is a real (intentional) progress loss worth a conscious sign-off.
WIP → ready transition depends on Forgejo re-deriving draft status from the title. Finalize edits the title to strip WIP: via the issues PATCH endpoint (tick.sh ~line 4325). Whether Forgejo re-clears the draft/mergeable state on a title edit through that endpoint (vs. the PR endpoint) is the hinge on which review+merge actually pick the PR back up. Worth a quick confirm against the deployed Forgejo version.

Minor:

Scope is over the ~400-line guideline; it's a cohesive single feature so that's a note, not an objection.
lib/automerge.sh: the explanatory comment sits just above the done < <(...) line rather than near the filter it describes — cosmetic.

No blocking defect found, but too much of the correctness rests on facts not visible in the diff for me to sign off. Recommend a human review-and-merge, as the author intended.

_{Independent review by the harness on claude-opus-4-8 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `COMMENT` _(automated)_ CI for `756de410`: **success** **Summary:** Adds turn-cap checkpoint-and-resume: a clean `max_turns` cut-off now snapshots WIP as a draft `WIP:` PR and resumes next tick instead of discarding the worktree. New pure `lib/checkpoint.sh` + 46-assertion unit test, with the git/forgejo side effects wired into `bin/tick.sh`. CI is green, the checklist matches the diff, no security issues, and I found no clear logic defect. But this is a >400-line change to the harness's core tick flow that self-deploys from master, and its most load-bearing seams can't be verified from the diff alone — that belongs in front of a human (which the author themselves states). Handing it over rather than approving. **Things a human must confirm before merge:** - **The stream file path is the crux and I can't verify it.** `CLAUDE_STREAM="$(dirname "$CLAUDE_LOG")/claude-stream.jsonl"` (tick.sh ~line 4085) — but the diff only shows `CLAUDE_LOG=".agent/claude-output.log"`; the claude invocation that would produce `claude-stream.jsonl` isn't in the diff. If that file isn't actually written at that path, `checkpoint_hit_turn_cap` always returns false, every turn cap silently falls back to `discard`, and the entire feature is a no-op — with no failing test to catch it (the unit tests feed hand-built fixtures, not a real run). Confirm the live claude run emits stream-json to exactly that path. - **Integration is unavoidably untested.** `checkpoint.sh` is pure and well-covered, but the risky part — disposition routing, resume worktree carve, push/`--force-with-lease`, draft-PR open/finalize, budget escalation — lives in `tick.sh` and has no end-to-end coverage. The jq filters are pinned in tests, the control flow around them is not. - **Crash-mid-resume drops that session's committed work (by design, verify it's intended).** In the `discard && IS_RESUME` branch (tick.sh ~line 4110) the code assumes "HEAD == origin/$BRANCH, nothing to push." If Claude made real commits during the resume and *then* crashed, those commits are on local HEAD, never pushed, and lost when the worktree is pruned — only the prior checkpoint survives. This matches the stated "crash mid-resume preserves the prior checkpoint" guardrail and is bounded by the budget, so it won't loop, but it is a real (intentional) progress loss worth a conscious sign-off. - **WIP → ready transition depends on Forgejo re-deriving draft status from the title.** Finalize edits the title to strip `WIP:` via the issues PATCH endpoint (tick.sh ~line 4325). Whether Forgejo re-clears the draft/mergeable state on a title edit through that endpoint (vs. the PR endpoint) is the hinge on which review+merge actually pick the PR back up. Worth a quick confirm against the deployed Forgejo version. **Minor:** - Scope is over the ~400-line guideline; it's a cohesive single feature so that's a note, not an objection. - `lib/automerge.sh`: the explanatory comment sits just above the `done < <(...)` line rather than near the filter it describes — cosmetic. No blocking defect found, but too much of the correctness rests on facts not visible in the diff for me to sign off. Recommend a human review-and-merge, as the author intended. --- <sub>Independent review by the harness on `claude-opus-4-8` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-05 22:15:38 +00:00

1 minute 33 seconds