[pelikhan]

Why

Investigating the three-hour AW health window on 2026-10-21 exposed three actionable failures: Gemini removed an explicitly configured environment setting from shell commands, Serena could not start the project's TypeScript/Bash language servers without a mounted Node runtime, and the concurrency agent submitted a ledger record with tool instead of the required analyzed_tool.

Changes

• Preserve explicitly configured, non-sensitive literal Gemini engine.env names through shell environment redaction. Check effective values after sandbox.agent.env overrides, retain credential exclusions and conservative sensitive-name filtering, and emit names only, never values.
• Provision Node.js 24 using a host-only pre-agent setup step in the shared Serena Go import. Preserve existing sandbox egress and URL filtering instead of adding a Node runtime declaration.
• Give the concurrency workflow an exact ledger payload example and clarify that accepted queued calls do not establish successful downstream validation or persistence.
• Add regressions for effective Gemini environment precedence, intentionally conservative name filtering, agent-job-local setup ordering, preserved network policies, and ledger additional-property rejection. Regenerate affected locks and include a patch changeset.

ADR: ADR-70859: Gemini engine.env shell redaction allowlist documents the design and conservative filtering tradeoff; its status remains Draft for maintainer review.

Validation

Passed for the review follow-up:

• make build and make fmt.
• go test ./pkg/workflow -run 'Test.*Gemini|TestConcurrencyAuditLedgerPromptExample|TestSerenaGoRuntimeExposure' -count=1.
• make recompile - 346/346 workflow sources compiled.
• make check-workflow-drift - generated locks match their sources.
• Custom changed-file Go lint, schema freshness, and impacted Go tests invoked by make agent-report-progress.
• golangci-lint run --allow-parallel-runners --new-from-rev=bb4a057ecaa2e2fadd2585dab33b3b7b57273968 ./pkg/workflow - no new findings. Full package lint reported three pre-existing modernize findings in unchanged files.
• Explicit comparison confirmed URL-filter domain policies match the pre-PR baseline across all 25 changed workflow locks.

Limitations:

• The standard aggregate publication target encountered another running golangci-lint process. Lint was rerun using its supported parallel mode, without changing or disabling checks; workflow drift was checked separately.
• Earlier diagnostic dry-run compilation emitted locks and passed shellcheck plus the zizmor/poutine differential gates. Actionlint rejected pre-existing compiler emission of root-level cache-mode: readonly; model-inventory refresh was unavailable in the isolated diagnostic repository because it had no remote. These are not passing dry-run results.
• The initial read-only security review found no confirmed vulnerabilities, but a final full source/generated-lock security gate has not been established.
• No workflows were dispatched or rerun. These changes have not established live recovery.

Scope

The dominant shared incident was GitHub installation API quota exhaustion, affecting 12 AW runs at the same activation boundary. This PR does not bypass daily-AIC accounting, change credentials, or claim to resolve quota exhaustion, secondary throttling, or OTLP authentication failures. The separate health-output reflection produced recommendations only; this PR does not change gh aw health.

All five requested inline review comments were addressed and replied to in commit 31476a8b65.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · 7.78 AIC · ⌖ 5.34 AIC · ⊞ 14.8K · ◷

Generated by Ponytail Reviewer for #70859

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Commit pushed: 510bfc1

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Test Quality Sentinel · sonnet50 · 14.3 AIC · ⌖ 15.1 AIC · ◇ 6.67 AIC · ⊞ 12.7K · ◷

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

🏗️ Design Decision Gate — ADR Required

This PR adds more than 100 new lines in pkg/ (145 lines) but does not link an Architecture Decision Record (ADR).

📄 Draft ADR committed: docs/adr/70859-allowlist-non-sensitive-gemini-engine-env-through-shell-redaction.md. Please review and complete it before merging.

The draft is about the main design change in pkg/workflow/gemini_tools.go. The compiler now writes a security.environmentVariableRedaction.allowed list into the generated Gemini settings.json. The list holds engine.env names only. A name is left out if it is on the AWF exclusion list, matches the sensitive-name regex, or has a ${{ value. Redaction stays enabled. The other options were turning redaction off, allowlisting every engine.env name, or adding an explicit frontmatter allowlist.

🔒 This PR cannot merge until an ADR is linked in the PR body.

📋 What to do next

1. Review the draft ADR on your branch. It was generated from the PR diff.
2. Complete or correct it. In particular, confirm the alternatives and the trade-offs of the sensitive-name heuristic. Note that it can both wrongly exclude harmless names and miss secrets stored under neutral names.
3. Commit the finalized ADR to docs/adr/.
4. Reference it in the PR body, for example:

   ADR: ADR-70859: Allowlist Non-Sensitive Gemini engine.env Names Through Shell Environment Redaction

Once the ADR is linked, this gate will re-run and check that the implementation matches the decision.

📋 Michael Nygard ADR Format Reference

• Context: What is the problem, and what forces are at play?
• Decision: What did you decide, and why?
• Alternatives Considered: What else could have been done?
• Consequences: What are the trade-offs, both positive and negative?

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · opus55 · 26.1 AIC · ⌖ 40.4 AIC · ⊞ 1.9K · ◷
_{Comment /review to run again}

[github-actions]

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)

reviewed_at: 2026-10-21T11:53:22Z
review_event: COMMENT
top_themes: [regression-test validity, generated-step ordering assertions]
files_reviewed: [pkg/workflow/gemini_tools.go, pkg/workflow/serena_go_runtime_test.go, pkg/workflow/workflow_health_regression_test.go, .github/workflows/daily-mcp-concurrency-analysis.md, .github/workflows/shared/mcp/serena-go.md]
comment_count: 2

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 3.78 AIC · ⌖ 0.687 AIC · ⊞ 1.6K · ◷
_{Comment /review to run again}

[github-actions]

Review

Two test-quality issues are worth correcting: the ledger regression does not actually test rejection of the tool alias, and the Serena ordering assertion can match a step from another job instead of jobs.agent. Inline comments contain the details. No blocking runtime defect was confirmed from the capped diff; validation was static only.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 3.78 AIC · ⌖ 0.687 AIC · ⊞ 1.6K
_{Comment /review to run again}

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — this is a focused, well-tested incident fix. Approving with one minor suggestion.

📋 Key Themes & Highlights

Key Themes

• One finding: the new geminiSensitiveEnvName regex (pkg/workflow/gemini_tools.go) does unbounded substring matching, which over-filters benign names (e.g. AUTHOR_NAME, KEYSTONE_REGION). Fail-safe bias is reasonable for a security allowlist, but it should be a documented tradeoff and covered by a false-positive test case, not just the ACCESS_TOKEN true-positive case.

Positive Highlights

• ✅ /diagnosing-bugs: each of the three incident causes (Gemini env redaction, Serena Node runtime, ledger field alias) has a regression test added in workflow_health_regression_test.go and serena_go_runtime_test.go that fails without the fix and passes with it — verified locally (targeted go test run against HEAD, all green).
• ✅ /codebase-design: the computeGeminiShellTools extraction is a clean, behavior-preserving refactor (confirmed against existing computeGeminiToolsCore subtests, all passing unchanged) that makes the allowlist mapping easier to read.
• ✅ /tdd: the new ledger example test parses the literal markdown example against the live JSON schema and asserts the tool-vs-analyzed_tool alias is rejected — directly encodes the root cause as an executable spec, not just prose guidance.
• ✅ Root cause addressed, not just symptom: the Gemini fix preserves explicit literal engine.env names while still excluding secrets/credentials/runtime expressions via the existing exclusion pipeline, rather than special-casing the one conformance variable that broke.

@copilot please address the review comment above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 74.9 AIC · ⌖ 14 AIC · ⊞ 10.2K
_{Comment /matt to run again}

[Copilot]

🟡 Changes recommended

The runtime setup broadens sandbox network access beyond Serena’s needs.

2 open issues

• Avoid widening sandbox network policies via runtimes.node · New
• Honor sandbox env overrides when checking literal-only settings · New

What changed in this PR

Addresses runtime setup and ledger payload failures in gh-aw’s compiler and repository workflows.

Changes:

• Preserve eligible literal Gemini environment settings in shell commands.
• Provision Node.js 24 for Serena and regenerate affected workflow locks.
• Document the concurrency ledger payload, add regressions, and include a patch changeset.

File	Description
pkg/​workflow/​workflow_health_regression_test.go	Tests Gemini allowlisting and ledger schema acceptance.
pkg/​workflow/​serena_go_runtime_test.go	Checks Node version and setup ordering.
pkg/​workflow/​gemini_tools.go	Adds environment allowlisting and extracts shell-tool mapping.
.github/​workflows/​typist.lock.yml	Adds Node setup and updates network policy.
.github/​workflows/​smoke-copilot.lock.yml	Refreshes generated workflow hashes.
.github/​workflows/​smoke-copilot-arm.lock.yml	Adds Node setup and refreshes hashes.
.github/​workflows/​smoke-copilot-aoai-entra.lock.yml	Adds Node setup and refreshes hashes.
.github/​workflows/​smoke-copilot-aoai-apikey.lock.yml	Adds Node setup and refreshes hashes.
.github/​workflows/​smoke-codex.lock.yml	Updates runtime-derived network policy and hashes.
.github/​workflows/​shared/​mcp/​serena-go.md	Declares Node runtime and documents mounted availability.
.github/​workflows/​purelock.lock.yml	Adds Node setup and updates detection network policy.
.github/​workflows/​mcp-inspector.lock.yml	Updates detection network policy and hashes.
.github/​workflows/​glossary-maintainer.lock.yml	Adds Node setup and updates detection network policy.
.github/​workflows/​engine-conformance-gemini.lock.yml	Allows the conformance sentinel environment name.
.github/​workflows/​daily-mcp-concurrency-analysis.md	Specifies ledger fields and queued-call limitations.
.github/​workflows/​daily-mcp-concurrency-analysis.lock.yml	Refreshes prompt-related hashes.
.github/​workflows/​daily-file-diet.lock.yml	Adds Node setup and updates network policy.
.github/​workflows/​daily-compiler-quality.lock.yml	Adds Node setup and updates network policy.
.changeset/​patch-aw-health-runtime-contracts.md	Records the fixes for a patch release.

_{Review effort: Max}

[gh-aw-bot]

@copilot address the following outstanding work in one pass:

1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
2. Fix failing check impacted-go-tests (FAILURE): https://github.com/github/gh-aw/actions/runs/39598876763/job/119218152481.

Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI.

Sous-chef head: 31476a8
Sous-chef work: 42e5d572d9d75b7168fe7d0aeda0717cc07e65ed240f6d299e6fec661214f1ac
Sous-chef state: 6975d83d73ac48981061a4c1a462c4570a27e3c9c272f3cd100faaa54bcb534d

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · 8.89 AIC · ⌖ 8.84 AIC · ⊞ 1.1K · ◷
_{Comment /souschef to run again}

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.93.0