factory: port the fresh-push-credential runner fix and raise the wall clock (toon-meta#248) - #61
Merged
Merged
Conversation
…n-meta#248) Closes #59. Ports connector#463's fix — App installation tokens expire ONE HOUR after mint; a runner that mints once at job start and pushes only at the end of a long cycle dies at the final push after every expensive agent iteration has already been spent (connector#462). Fresh-mint and publish-early land first; the clock moves last, per the issue's ordering. Key decisions: - New `mint-app-token.ts` (forge-core, duplicated standalone in `.sandcastle/` — mirrors the existing sandbox-secrets.ts split between the live path and the stage-0 runner) signs the App JWT and exchanges it for an installation token via RS256 (`node:crypto`, no jsonwebtoken dep). `createTokenMinter` returns `undefined` when APP_ID/APP_PRIVATE_KEY are absent so local runs / ambient-GH_TOKEN runs are unchanged. - `sandcastle-runners.ts`'s new `pushBranch` mints fresh immediately before every push, delivers the token via STDIN into a mode-600 file read by a one-shot `credential.helper`, and deletes it after. The leading `-c credential.helper=` reset is load-bearing (multi-valued config key — verified directly by the "resets the container-global helper" test, which asserts the empty reset precedes the one-shot helper in the pushed command) and is what stops the stale container-global helper from `gh auth setup-git` (SANDBOX_READY_HOOKS) from winning. Also refreshes the host's own `process.env.GH_TOKEN` for the `gh pr list`/`gh pr create` calls that follow. - New `pushEarly` runner + `PushEarly` cycle.ts injection point: publishes best-effort right after the implement phase, before the pre-review gate and review, so a kill mid-review still leaves recoverable work on the remote. The primitive fails loud; `runCycle` is what makes it best-effort (catches and logs). - `run.ts` wires `createTokenMinter()` into `createSandcastleRunners` and wraps it with `withMasking` (`::add-mask::`) as defence-in-depth alongside the existing redact-then-upload log artifact step. - Workflow + template: job timeout 60->180, new step timeout 170 (was 50), APP_ID/APP_PRIVATE_KEY added to the HOST-only implement step env (never forwarded into the sandbox — sandboxSecrets/PASSTHROUGH_KEYS omit both). Template additionally gained the redact+upload steps it never had. - `.sandcastle/agent-implement-issue.ts`: PR-mode push now goes through the same pushBranch, plus a new best-effort early-publish call after the implement phase. Files changed: packages/forge-core/src/{mint-app-token.ts, mint-app-token.test.ts} (new), sandcastle-runners.{ts,test.ts}, cycle.{ts, test.ts}, index.ts; packages/forge-cli/src/run.{ts,test.ts}; .github/workflows/agent-implement.yml; templates/workflows/agent-implement.yml; .sandcastle/{agent-implement-issue.ts, mint-app-token.ts (new)}. Gate green: format, lint, typecheck, 189 tests (+34), build. Notes for next iteration: `.sandcastle/` and `templates/` are excluded from eslint/vitest/tsc by design (confirmed in eslint.config.js/vitest.config.ts), so the two hand-authored .sandcastle/*.ts changes were syntax-checked with esbuild (no type errors caught) rather than run through the typed gate — matches the precedent set by #57's review-verdict.ts port. Per `templates/sandcastle`'s locked six-file inventory (templates.test.ts:22-24), runner .ts scripts are not currently part of the stamped bundle at all, so "the template stamps agent-implement-issue.ts" in the issue is aspirational for a future forge-new wiring, not current stamp.ts behavior — fixed this repo's own real .sandcastle/agent-implement-issue.ts regardless, since it is what actually runs today. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The live forge-core/forge-cli push path wraps mintToken with withMasking (::add-mask::) as defence-in-depth alongside the redact-then-upload log step, but the mirrored stage-0 .sandcastle/agent-implement-issue.ts path (what a stamped repo runs) didn't get the same treatment. Bring it in line for consistency on a PUBLIC repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 10, 2026
Closed
ALLiDoizCode
added a commit
that referenced
this pull request
Aug 10, 2026
…s on it (#63) Closes #62 `pushBranch`'s fresh-mint script opened with `set -euo pipefail`, but `sandbox.exec` runs every command through `sh -c`, and `/bin/sh` in the sandbox image is dash. dash has no `-o pipefail`, so it aborted on line 1 with exit 2 and `git push` never ran. With APP_ID/APP_PRIVATE_KEY present — i.e. every CI run — no agent PR could be opened at all. Forge's factory has been fully down since #61 merged. Reproduced and fixed under the exact shell: OLD $ dash -c 'set -euo pipefail; echo reached' dash: 1: set: Illegal option -o pipefail exit 2 NEW $ dash -c 'set -eu; ...' LINE 7 REACHED — push would run; mode=600 bytes=11 exit 0 `-o pipefail` was never load-bearing here: the script contains no pipeline. `set -eu` keeps the fail-fast behaviour that matters, and the mode-600 token file, the EXIT trap and the empty `credential.helper=` reset are all untouched — verified still present in the dash run above. Fixed in both copies that go through `sandbox.exec`: - packages/forge-core/src/sandcastle-runners.ts - .sandcastle/agent-implement-issue.ts Deliberately NOT changed: packages/forge-cli/src/stamp.ts's generated verify stub, which carries a `#!/usr/bin/env bash` shebang and is invoked as `bash verify/<id>.sh` — pipefail is valid and wanted there. Not propagating: connector's runner has no wrapper script, so no other factory repo carries this shape. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Produced by the sandcastle
agent:implementrunner; awaiting human review.Closes #59
🤖 Generated with Claude Code