glue: salvage path — push branch + warning PR when the build agent crashes after writing files #4

New issue

Open

opened 2026-11-12 21:25:12 +00:00 by hermes_forgeron · 0 comments

hermes_forgeron commented

2026-11-12 21:25:12 +00:00

Collaborator

Part of #1 (finding 3). In the three failed builds of test_factory_ai#3 (runs 895 / 898 / 901), the files were fully written before the crash — but the outer catch in glue/src/main.ts only posts ⚠️ Agent run failed: … and sets process.exitCode = 1. The ephemeral workspace was destroyed with nothing pushed and the human gate never saw usable output.

Proposed change

In main.ts, build phase only: when runAgent() throws (non-zero status OR a signal — glue/src/opencode.ts reports opencode exited with status ${r.status}, where null means killed), attempt a salvage once:

Call the same commitAndPush() the happy path uses (branch agent/issue-${cfg.issue}, force-push).
If it pushed:
- open the PR via forgejo.createPull() with a warning title, e.g. agent: ⚠️ unverified — agent crashed (issue #N), and a body that states the agent crashed before finishing verification, quotes the error message, includes the spec (same structure as the happy path) — and does NOT contain Closes #N;
- post an issue comment: "work salvaged to an unverified PR" + PR link + the original failure note.
If the working tree is clean (commitAndPush returns pushed: false): keep today's behavior (failure note only, no PR).

Required details:

commitAndPush() in glue/src/git.ts hardcodes Closes #${issue} in the commit message. Parameterize it (e.g. add closesIssue?: boolean, default true) so a salvage commit does NOT auto-close the issue when the warning PR gets merged.
The salvage PR body must also omit Closes #N (the happy path adds it today in main.ts).
Plan-mode failures are unaffected — there is nothing to push.
Dry-run (cfg.dryRun) must log what would be salvaged instead of pushing or posting.
Keep posting the original ⚠️ Agent run failed message with crash context; do not swallow the error.

Acceptance criteria

e2e crash case: an issue whose spec requires a runtime smoke test using the known-crashing npm start & pattern → opencode still fails, BUT branch agent/issue-N is pushed and a PR with the ⚠️ warning prefix is opened; its body contains no Closes #N; merging it does NOT close the issue
e2e happy case: a normal successful build → PR exactly as today (same title format, Closes #N present in body and commit message) — no regression
crash with no file changes (e.g. opencode dies during exploration) → behavior unchanged (failure note, no branch, no PR)
dry-run of a crashing build logs the salvage intent and pushes/posts nothing
npm run check green; new logic covered by smoke.ts tests where testable (e.g. the salvage decision given a git status)

Part of #1 (finding 3). In the three failed builds of test_factory_ai#3 (runs 895 / 898 / 901), the files were fully written before the crash — but the outer `catch` in `glue/src/main.ts` only posts `⚠️ Agent run failed: …` and sets `process.exitCode = 1`. The ephemeral workspace was destroyed with nothing pushed and the human gate never saw usable output. ## Proposed change In `main.ts`, build phase only: when `runAgent()` throws (non-zero status OR a signal — `glue/src/opencode.ts` reports `opencode exited with status ${r.status}`, where `null` means killed), attempt a salvage **once**: 1. Call the same `commitAndPush()` the happy path uses (branch `agent/issue-${cfg.issue}`, force-push). 2. If it pushed: - open the PR via `forgejo.createPull()` with a warning title, e.g. `agent: ⚠️ unverified — agent crashed (issue #N)`, and a body that states the agent crashed before finishing verification, quotes the error message, includes the spec (same structure as the happy path) — and **does NOT contain `Closes #N`**; - post an issue comment: "work salvaged to an unverified PR" + PR link + the original failure note. 3. If the working tree is clean (`commitAndPush` returns `pushed: false`): keep today's behavior (failure note only, no PR). Required details: - `commitAndPush()` in `glue/src/git.ts` hardcodes `Closes #${issue}` in the commit message. Parameterize it (e.g. add `closesIssue?: boolean`, default `true`) so a salvage commit does NOT auto-close the issue when the warning PR gets merged. - The salvage PR body must also omit `Closes #N` (the happy path adds it today in `main.ts`). - Plan-mode failures are unaffected — there is nothing to push. - Dry-run (`cfg.dryRun`) must log what would be salvaged instead of pushing or posting. - Keep posting the original `⚠️ Agent run failed` message with crash context; do not swallow the error. ## Acceptance criteria - [ ] e2e crash case: an issue whose spec requires a runtime smoke test using the known-crashing `npm start &` pattern → opencode still fails, BUT branch `agent/issue-N` is pushed and a PR with the ⚠️ warning prefix is opened; its body contains no `Closes #N`; merging it does NOT close the issue - [ ] e2e happy case: a normal successful build → PR exactly as today (same title format, `Closes #N` present in body and commit message) — no regression - [ ] crash with no file changes (e.g. opencode dies during exploration) → behavior unchanged (failure note, no branch, no PR) - [ ] dry-run of a crashing build logs the salvage intent and pushes/posts nothing - [ ] `npm run check` green; new logic covered by smoke.ts tests where testable (e.g. the salvage decision given a git status)

hermes_forgeron referenced this issue

2026-11-12 21:25:13 +00:00

Dogfood findings: reliability improvements for the agent workflow #1