From 571088f49271382a3590eb86df6cca8b89ca91df Mon Sep 17 00:00:00 2001 From: Sam Heutmaker Date: Sat, 26 Sep 2026 17:15:04 -0700 Subject: [PATCH 1/4] agent: run the final-message check on a capped answer When a turn hits the tool-call cap, finalAnswer returned the model's tools-disabled reply without ev.CheckFinal, so a definition with an output contract could end a capped turn "succeeded" with no output: the daemon's correction round is what usually pushes a reviewer to the cap, and the answer it forced skipped the very check that asked for it. The capped answer now goes through the same check the loop runs on a no-tools reply, with the one correction round the check may ask for. A second miss fails the turn, as it does in the loop. Co-Authored-By: Claude Fable 5.1 --- internal/agent/agent.go | 26 ++++++- internal/agent/final_answer_test.go | 117 ++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 internal/agent/final_answer_test.go diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 30e63d8a0..5e864fdcb 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1235,8 +1235,32 @@ func (a *Agent) ManualCompact(ctx context.Context, ev Events) error { // finalAnswer makes one last completion with tools disabled, so a run that hit // the tool-turn cap still returns the model's best answer instead of an error. -// A system nudge tells the model to stop calling tools and answer now. +// The capped answer goes through the same final-message check the loop runs +// on a no-tools reply: a definition's output contract is a property of the +// turn, not of how it ended. One correction round is allowed, as in the loop; +// a second miss fails the turn. func (a *Agent) finalAnswer(ctx context.Context, ev Events) (string, error) { + text, err := a.cappedAnswer(ctx, ev) + if err != nil || ev.CheckFinal == nil { + return text, err + } + retry, err := ev.CheckFinal(text) + if err != nil || !retry { + return text, err + } + if text, err = a.cappedAnswer(ctx, ev); err != nil { + return text, err + } + if _, err := ev.CheckFinal(text); err != nil { + return "", err + } + return text, nil +} + +// cappedAnswer is one tools-disabled completion. A system nudge tells the +// model to stop calling tools and answer now; the turn's ephemeral notices +// (an output-contract correction among them) ride along as usual. +func (a *Agent) cappedAnswer(ctx context.Context, ev Events) (string, error) { msgs := append(slices.Clone(withEphemeralSystem(a.Messages, ev.ephemeral())), llm.Message{Role: "system", Content: "You have reached the tool-call limit. Do NOT request any more tools. Give your final answer now using only what you have already gathered."}) client := *a.Client diff --git a/internal/agent/final_answer_test.go b/internal/agent/final_answer_test.go new file mode 100644 index 000000000..12f79a5e2 --- /dev/null +++ b/internal/agent/final_answer_test.go @@ -0,0 +1,117 @@ +package agent + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/context-labs/whip/internal/llm" + "github.com/context-labs/whip/internal/tools" +) + +// A turn that hits the cap ends with a tools-disabled answer. That answer must +// pass the same final-message check a no-tools reply does, with the one +// correction round the check may ask for. +func TestFinalAnswerRunsCheckFinalWithOneCorrection(t *testing.T) { + t.Parallel() + var finals []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request llm.Request + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Error(err) + http.Error(w, "bad request", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "text/event-stream") + switch { + case len(request.Tools) > 0: + fmt.Fprint(w, `data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"c1","type":"function","function":{"name":"echo","arguments":"{\"s\":\"work\"}"}}]}}]}`+"\n\n") + default: + // The correction notice rides as an ephemeral system message + // ahead of the tools-disabled nudge. + var system []string + for _, message := range request.Messages { + if message.Role == "system" { + system = append(system, message.Content) + } + } + joined := strings.Join(system, "\n") + finals = append(finals, joined) + answer := "not json" + if strings.Contains(joined, "correct") { + answer = `{"ok":true}` + } + fmt.Fprintf(w, `data: {"choices":[{"delta":{"content":%q},"finish_reason":"stop"}],"usage":{"prompt_tokens":5,"completion_tokens":2}}`+"\n\n", answer) + } + fmt.Fprint(w, "data: [DONE]\n\n") + })) + defer server.Close() + ag := NewRuntime(llm.New(server.URL, "key"), "model", 1024, "system", tools.NewServices()) + defer ag.Services.Close() + ag.Tools = []tools.Tool{echoTool()} + ag.MaxTurns = 1 + var notice string + checks := 0 + ev := Events{ + EphemeralNotices: func() string { return notice }, + CheckFinal: func(text string) (bool, error) { + checks++ + if text == `{"ok":true}` { + return false, nil + } + if checks > 1 { + return false, fmt.Errorf("output_invalid: %q", text) + } + notice = "please correct: reply with one JSON value" + return true, nil + }, + } + final, err := ag.Turn(t.Context(), "go", ev) + if err != nil || final != `{"ok":true}` { + t.Fatalf("capped turn = %q, %v", final, err) + } + if checks != 2 || len(finals) != 2 { + t.Fatalf("checks=%d final calls=%d", checks, len(finals)) + } + if strings.Contains(finals[0], "correct") || !strings.Contains(finals[1], "correct") { + t.Fatalf("correction notice not delivered on the second final call: %q", finals) + } +} + +func TestFinalAnswerFailsTheTurnOnASecondMiss(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request llm.Request + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Error(err) + http.Error(w, "bad request", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "text/event-stream") + if len(request.Tools) > 0 { + fmt.Fprint(w, `data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"c1","type":"function","function":{"name":"echo","arguments":"{\"s\":\"work\"}"}}]}}]}`+"\n\n") + } else { + fmt.Fprint(w, `data: {"choices":[{"delta":{"content":"still not json"},"finish_reason":"stop"}],"usage":{"prompt_tokens":5,"completion_tokens":2}}`+"\n\n") + } + fmt.Fprint(w, "data: [DONE]\n\n") + })) + defer server.Close() + ag := NewRuntime(llm.New(server.URL, "key"), "model", 1024, "system", tools.NewServices()) + defer ag.Services.Close() + ag.Tools = []tools.Tool{echoTool()} + ag.MaxTurns = 1 + checks := 0 + _, err := ag.Turn(t.Context(), "go", Events{CheckFinal: func(string) (bool, error) { + checks++ + if checks > 1 { + return false, fmt.Errorf("output_invalid") + } + return true, nil + }}) + if err == nil || !strings.Contains(err.Error(), "output_invalid") || checks != 2 { + t.Fatalf("second miss should fail the turn: err=%v checks=%d", err, checks) + } +} From e9923c5e0278a939ad44b2a0b8b8e03328b2e384 Mon Sep 17 00:00:00 2001 From: Sam Heutmaker Date: Sat, 26 Sep 2026 19:36:25 -0700 Subject: [PATCH 2/4] agent: errors.New for a constant test error (golangci-lint perfsprint) Co-Authored-By: Claude Fable 5.1 --- internal/agent/final_answer_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/agent/final_answer_test.go b/internal/agent/final_answer_test.go index 12f79a5e2..0504ad172 100644 --- a/internal/agent/final_answer_test.go +++ b/internal/agent/final_answer_test.go @@ -2,6 +2,7 @@ package agent import ( "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -107,7 +108,7 @@ func TestFinalAnswerFailsTheTurnOnASecondMiss(t *testing.T) { _, err := ag.Turn(t.Context(), "go", Events{CheckFinal: func(string) (bool, error) { checks++ if checks > 1 { - return false, fmt.Errorf("output_invalid") + return false, errors.New("output_invalid") } return true, nil }}) From 210f9755a1138040ad104a437dd94f03f1890e80 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 12 Oct 2026 19:00:56 +0000 Subject: [PATCH 3/4] chore(deps): bump the actions group across 1 directory with 6 updates Bumps the actions group with 6 updates in the / directory: | Package | From | To | | --- | --- | --- | | [actions/setup-node](https://github.com/actions/setup-node) | `7.0.0` | `7.1.0` | | [actions/upload-artifact](https://github.com/actions/upload-artifact) | `7.0.1` | `7.0.2` | | [actions/download-artifact](https://github.com/actions/download-artifact) | `8.0.1` | `8.0.2` | | [github/codeql-action/init](https://github.com/github/codeql-action) | `4.37.9` | `4.38.3` | | [github/codeql-action/autobuild](https://github.com/github/codeql-action) | `4.37.9` | `4.38.3` | | [github/codeql-action/analyze](https://github.com/github/codeql-action) | `4.37.9` | `4.38.3` | Updates `actions/setup-node` from 7.0.0 to 7.1.0 - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](https://github.com/actions/setup-node/compare/v7...v7.1.0) Updates `actions/upload-artifact` from 7.0.1 to 7.0.2 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/043fb46d1a93c77aae656e7c1c64a875d1fc6a0a...cf430e030ddbb5b0abf93d22962f4752f3646cd9) Updates `actions/download-artifact` from 8.0.1 to 8.0.2 - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](https://github.com/actions/download-artifact/compare/3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c...9000827ccba6bdab643e8b6fd33ac0654aef8333) Updates `github/codeql-action/init` from 4.37.9 to 4.38.3 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/cdf488f595d80d6e07e03d4674febd5ab45fa938...24c54180a607b1449ed407dd24f251e4e9147c8d) Updates `github/codeql-action/autobuild` from 4.37.9 to 4.38.3 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/cdf488f595d80d6e07e03d4674febd5ab45fa938...24c54180a607b1449ed407dd24f251e4e9147c8d) Updates `github/codeql-action/analyze` from 4.37.9 to 4.38.3 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/cdf488f595d80d6e07e03d4674febd5ab45fa938...24c54180a607b1449ed407dd24f251e4e9147c8d) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: 7.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions - dependency-name: actions/upload-artifact dependency-version: 7.0.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: actions/download-artifact dependency-version: 8.0.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: github/codeql-action/init dependency-version: 4.38.3 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions - dependency-name: github/codeql-action/autobuild dependency-version: 4.38.3 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions - dependency-name: github/codeql-action/analyze dependency-version: 4.38.3 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yml | 14 +++++++------- .github/workflows/desktop-check.yml | 6 +++--- .github/workflows/desktop-release.yml | 8 ++++---- .github/workflows/docs-deploy.yml | 10 +++++----- .github/workflows/mobile.yml | 2 +- .github/workflows/publish-cli.yml | 28 +++++++++++++-------------- .github/workflows/security.yml | 6 +++--- 7 files changed, 37 insertions(+), 37 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 691c66848..c86c326c5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -192,7 +192,7 @@ jobs: with: go-version-file: go.mod cache: true - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: 24 cache: npm @@ -222,7 +222,7 @@ jobs: - run: node apps/web/scripts/performance.mjs - run: npm run test:example - name: Shared verified renderer for desktop and release consumers - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: ci-renderer if-no-files-found: error @@ -231,7 +231,7 @@ jobs: apps/web/renderer-manifest.json - name: Browser acceptance evidence if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: web-browser-evidence if-no-files-found: ignore @@ -246,7 +246,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: ref: ${{ github.sha }} - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm @@ -261,7 +261,7 @@ jobs: - run: npm run test:docs:browser - run: npm run build:storybook:docs - name: Static docs artifact - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: docs-static if-no-files-found: error @@ -343,14 +343,14 @@ jobs: with: go-version-file: go.mod cache: true - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm - run: npm ci env: ELECTRON_SKIP_BINARY_DOWNLOAD: '1' - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: ci-renderer path: apps/web diff --git a/.github/workflows/desktop-check.yml b/.github/workflows/desktop-check.yml index 5ff360497..f13fb1589 100644 --- a/.github/workflows/desktop-check.yml +++ b/.github/workflows/desktop-check.yml @@ -19,7 +19,7 @@ jobs: with: go-version-file: go.mod cache: true - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm @@ -28,7 +28,7 @@ jobs: ELECTRON_SKIP_BINARY_DOWNLOAD: '1' - run: node node_modules/electron/install.js - run: npx playwright install ffmpeg - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: ${{ inputs.renderer-artifact }} path: apps/web @@ -53,7 +53,7 @@ jobs: run: node apps/desktop/scripts/smoke.mjs - name: Desktop package and lifecycle evidence if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: desktop-ci-evidence if-no-files-found: warn diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index d52bfea2e..3a89ea644 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -45,7 +45,7 @@ jobs: with: go-version-file: go.mod cache: true - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm @@ -64,7 +64,7 @@ jobs: env: ELECTRON_SKIP_BINARY_DOWNLOAD: '1' - run: node node_modules/electron/install.js - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: ${{ inputs.renderer-artifact }} path: apps/web @@ -102,7 +102,7 @@ jobs: if (!(await stat(`${root}/${name}`)).size) throw new Error(`Missing final ${name}`); } NODE - - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: desktop-release-arm64 path: apps/desktop/out/release @@ -111,7 +111,7 @@ jobs: compression-level: 0 - name: Sanitized startup diagnostics (including failures) if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: desktop-startup-diagnostics-arm64 path: apps/desktop/out/diagnostics/startup.json diff --git a/.github/workflows/docs-deploy.yml b/.github/workflows/docs-deploy.yml index 3601f53d5..2f94e747a 100644 --- a/.github/workflows/docs-deploy.yml +++ b/.github/workflows/docs-deploy.yml @@ -25,7 +25,7 @@ jobs: with: ref: ${{ github.sha }} persist-credentials: false - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm @@ -51,7 +51,7 @@ jobs: npm run build:docs (cd apps/docs && node scripts/package-deploy.mjs) node apps/docs/scripts/worker-test.mjs - - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: docs-deploy-${{ github.sha }} path: apps/docs/dist/deploy @@ -72,10 +72,10 @@ jobs: with: ref: ${{ github.sha }} persist-credentials: false - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: docs-deploy-${{ github.sha }} path: apps/docs/dist/deploy @@ -114,7 +114,7 @@ jobs: with: ref: ${{ github.sha }} persist-credentials: false - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm diff --git a/.github/workflows/mobile.yml b/.github/workflows/mobile.yml index 7c48f8282..8976a851d 100644 --- a/.github/workflows/mobile.yml +++ b/.github/workflows/mobile.yml @@ -20,7 +20,7 @@ jobs: - uses: actions/checkout@v7 with: ref: ${{ github.sha }} - - uses: actions/setup-node@v7 + - uses: actions/setup-node@v7.1.0 with: node-version: 24 cache: npm diff --git a/.github/workflows/publish-cli.yml b/.github/workflows/publish-cli.yml index f647c5331..0aede03fa 100644 --- a/.github/workflows/publish-cli.yml +++ b/.github/workflows/publish-cli.yml @@ -122,12 +122,12 @@ jobs: with: go-version-file: go.mod cache: true - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm - run: npm ci - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: ci-renderer path: apps/web @@ -150,7 +150,7 @@ jobs: -o "dist/release/$ASSET" ./cmd/whip - name: Accept the packaged binary from a fresh home with the tested renderer run: python3 scripts/test-distributions.py --binary "dist/release/$ASSET" --version "$RELEASE_TAG" - - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: whipcode-bin-${{ matrix.goos }}-${{ matrix.goarch }} path: dist/release/${{ matrix.asset }} @@ -178,14 +178,14 @@ jobs: with: ref: ${{ needs.metadata.outputs.source }} persist-credentials: false - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: ci-renderer path: apps/web - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: whipcode-bin-linux-amd64 path: linux @@ -193,7 +193,7 @@ jobs: run: | chmod +x linux/whipcode-linux-x64 node apps/desktop/scripts/linux-smoke.mjs linux/whipcode-linux-x64 - - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: release-linux-evidence path: linux/linux-runtime.json @@ -214,19 +214,19 @@ jobs: with: ref: ${{ needs.metadata.outputs.source }} persist-credentials: false - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: pattern: whipcode-bin-* path: candidate merge-multiple: true - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: desktop-release-arm64 path: candidate - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: release-linux-evidence path: candidate @@ -242,7 +242,7 @@ jobs: uses: actions/attest@1e69f48acb82d1966a394da916b4c1698aa569d6 # v4 with: subject-path: candidate/* - - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 with: name: release-candidate path: candidate @@ -278,14 +278,14 @@ jobs: ref: ${{ needs.metadata.outputs.source }} fetch-depth: 0 persist-credentials: false - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7 with: node-version: '24' cache: npm - run: npm ci env: ELECTRON_SKIP_BINARY_DOWNLOAD: '1' - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8 with: name: release-candidate path: candidate diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index bc64804d4..e3bafa1c2 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -56,11 +56,11 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: ref: ${{ github.sha }} - - uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4 + - uses: github/codeql-action/init@24c54180a607b1449ed407dd24f251e4e9147c8d # v4 with: languages: go queries: security-and-quality - - uses: github/codeql-action/autobuild@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4 - - uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4 + - uses: github/codeql-action/autobuild@24c54180a607b1449ed407dd24f251e4e9147c8d # v4 + - uses: github/codeql-action/analyze@24c54180a607b1449ed407dd24f251e4e9147c8d # v4 with: category: "/language:go" From bee34d819d9ecceb64b8421a2c631ba90c7eb4a7 Mon Sep 17 00:00:00 2001 From: Amar Singh Date: Fri, 25 Sep 2026 16:07:56 +0000 Subject: [PATCH 4/4] Preserve Go build settings in managed child processes (cherry picked from commit 8884403a7e23572c95c1d760c44eafa140d89e4c) --- docs/concurrency.md | 7 +++ internal/capability/process.go | 4 ++ internal/capability/process_go_test.go | 65 ++++++++++++++++++++++++++ internal/capability/process_test.go | 37 ++++++++++++++- 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 internal/capability/process_go_test.go diff --git a/docs/concurrency.md b/docs/concurrency.md index e28da4267..04147d271 100644 --- a/docs/concurrency.md +++ b/docs/concurrency.md @@ -248,6 +248,13 @@ Shell and kernel subprocesses run in managed process groups. This is operational containment, not a security sandbox against another hostile process already running as the same OS user. +Managed host processes inherit the Go build settings `GOCACHE`, `GOMODCACHE`, +`GOPATH`, `GOROOT`, `GOTOOLCHAIN`, and `GOFLAGS` captured when the process manager +starts. This preserves preloaded modules and toolchains in offline environments. +Explicit child-process settings take precedence over that snapshot. Other Go +variables, including credential-bearing proxy/auth settings, are not implicitly +inherited; the allowlist is in `internal/capability/process.go`. + ## Gateway process ownership The socket-only daemon owns execution; `internal/webgateway` owns HTTP/WS diff --git a/internal/capability/process.go b/internal/capability/process.go index 7820d8062..7736964af 100644 --- a/internal/capability/process.go +++ b/internal/capability/process.go @@ -372,6 +372,10 @@ func allowedBaseEnvironment(name string) bool { "TMPDIR", "TMP", "TEMP", "LANG", "LANGUAGE", "TERM", "COLORTERM", "NO_COLOR": return true + case "GOCACHE", "GOMODCACHE", "GOPATH", "GOROOT", "GOTOOLCHAIN", "GOFLAGS": + // Keep the host's Go toolchain and preloaded caches usable offline. + // Do not allow GO* wholesale: proxy/auth settings can carry credentials. + return true } return name == buildinfo.Env("HOME") || strings.HasPrefix(name, "LC_") } diff --git a/internal/capability/process_go_test.go b/internal/capability/process_go_test.go new file mode 100644 index 000000000..564906310 --- /dev/null +++ b/internal/capability/process_go_test.go @@ -0,0 +1,65 @@ +//go:build darwin || linux + +package capability + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestProcessGoUsesPreloadedModuleCacheOffline(t *testing.T) { + goBin, err := exec.LookPath("go") + if err != nil { + t.Skip("Go executable is not on PATH") + } + work := t.TempDir() + cache := t.TempDir() + const dependency = "example.com/whip-cache-test" + // Listing the module graph needs cached metadata, so this fixture exercises + // a real Go command without downloads or toolchain compilation. + mod := filepath.Join(cache, "cache", "download", dependency, "@v", "v1.0.0.mod") + if err := os.MkdirAll(filepath.Dir(mod), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(mod, []byte("module "+dependency+"\n\ngo 1.20\n"), 0o644); err != nil { + t.Fatal(err) + } + info := filepath.Join(filepath.Dir(mod), "v1.0.0.info") + if err := os.WriteFile(info, []byte(`{"Version":"v1.0.0","Time":"2020-01-01T00:00:00Z"}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(work, "go.mod"), []byte("module example.com/app\n\ngo 1.20\n\nrequire "+dependency+" v1.0.0\n"), 0o644); err != nil { + t.Fatal(err) + } + t.Setenv("GOMODCACHE", cache) + t.Setenv("GOCACHE", t.TempDir()) + t.Setenv("GOPATH", t.TempDir()) + t.Setenv("GOTOOLCHAIN", "local") + t.Setenv("GOFLAGS", "-mod=mod") + m := NewProcessManager() + t.Cleanup(func() { _ = m.Close() }) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + var stdout, stderr bytes.Buffer + p, err := m.Start(ctx, "root", goBin, []string{"list", "-m", "all"}, ProcessOptions{ + Cwd: work, + // Explicitly prohibit downloads and ignore host-specific Go settings. + Env: map[string]string{"GOPROXY": "off", "GOSUMDB": "off", "GOENV": "off", "GOWORK": "off"}, + Stdin: strings.NewReader(""), Stdout: &stdout, Stderr: &stderr, + }) + if err != nil { + t.Fatal(err) + } + if err := p.Wait(); err != nil { + t.Fatalf("offline module lookup failed: %v\n%s", err, stderr.String()) + } + if !strings.Contains(stdout.String(), dependency+" v1.0.0\n") { + t.Fatalf("cached dependency missing from module graph: %s", stdout.String()) + } +} diff --git a/internal/capability/process_test.go b/internal/capability/process_test.go index 7ff08c9a1..1bfede6aa 100644 --- a/internal/capability/process_test.go +++ b/internal/capability/process_test.go @@ -61,8 +61,23 @@ func TestProcessEnvironmentCwdAndDescriptors(t *testing.T) { t.Setenv("SSH_AUTH_SOCK", "/daemon-agent") t.Setenv("RANDOM_DAEMON_SECRET", "daemon-random") t.Setenv("WHIPCODE_HOME", "/snapshot-whip-home") + goEnvironment := map[string]string{ + "GOCACHE": "/snapshot-go-build-cache", + "GOMODCACHE": "/snapshot-go-module-cache", + "GOPATH": "/snapshot-gopath", + "GOROOT": "/snapshot-goroot", + "GOTOOLCHAIN": "local", + "GOFLAGS": "-mod=mod", + } + for name, value := range goEnvironment { + t.Setenv(name, value) + } + t.Setenv("GOPROXY", "https://user:secret@proxy.invalid") + t.Setenv("GOAUTH", "daemon-auth-command") + t.Setenv("GO_PRIVATE_TOKEN", "daemon-go-secret") m := NewProcessManager() t.Setenv("HOME", "/changed-after-snapshot") + t.Setenv("GOMODCACHE", "/changed-after-snapshot") f, err := os.CreateTemp(t.TempDir(), "descriptor") if err != nil { @@ -112,7 +127,12 @@ func TestProcessEnvironmentCwdAndDescriptors(t *testing.T) { t.Errorf("%s = %q, want %q", name, env[name], want) } } - for _, name := range []string{"PROVIDER_API_KEY", "SSH_AUTH_SOCK", "RANDOM_DAEMON_SECRET"} { + for name, want := range goEnvironment { + if env[name] != want { + t.Errorf("%s = %q, want %q", name, env[name], want) + } + } + for _, name := range []string{"PROVIDER_API_KEY", "SSH_AUTH_SOCK", "RANDOM_DAEMON_SECRET", "GOPROXY", "GOAUTH", "GO_PRIVATE_TOKEN"} { if _, ok := env[name]; ok { t.Errorf("daemon environment leaked %s", name) } @@ -130,6 +150,21 @@ func TestProcessEnvironmentCwdAndDescriptors(t *testing.T) { if built, err := m.environment(nil); err != nil || !slices.IsSorted(built) { t.Errorf("environment not deterministically sorted: %v, %v", built, err) } + // Launchers that own exec.Cmd receive the captured build settings too; + // explicit per-process values still take precedence. + child, err := m.ChildEnvironment(map[string]string{"GOFLAGS": "-mod=readonly"}) + if err != nil { + t.Fatal(err) + } + childEnv := envMap(child) + for name, want := range goEnvironment { + if name == "GOFLAGS" { + want = "-mod=readonly" + } + if childEnv[name] != want { + t.Errorf("ChildEnvironment %s = %q, want %q", name, childEnv[name], want) + } + } if err := m.Close(); err != nil { t.Fatal(err) }