[wz-gsa]

Summary

Fixes #569. Also addresses the pre-existing process races tracked in #685.

Git exports repository-local variables into hooks; changing directory alone does not clear them. Fixture Git commands could therefore mutate the invoking repository. This PR clears that state in the current shell, with sentinel and negative-control coverage.

Current implementation

• Fetch through private staging and publish metadata-free kit payloads, restoring repeated symbolic fetch and failed-fetch recovery that the earlier destination guard broke.
• Refuse empty/root/ambiguous-newline destinations and existing worktrees, bare repositories, Git metadata and nested repositories. Bare protection remains effective with safe.bareRepository=explicit.
• Preserve previous payloads when fetch fails and roll them back when publication fails.
• Clear hook-exported Git-local state before fixture operations.
• Bound captured output to a shared final byte cutoff, preserve foreground stdin, and fail on capture IO errors instead of claiming success.
• Install startup-shim termination handling before spawning its child, forwarding pending termination and reaping the child.

Re-review handoff

@mogul: please re-review bf9473408b9224960fbf838239d3d5ac7fa82509. The outstanding changes-requested review targets 522456fb48352f51e099b8ec51aa4422ab57bfcc; its legitimate re-fetch/recovery regression has been repaired rather than weakening repository guards. Independent fixed-head QA and security re-reviews found no remaining blocking findings after follow-up fixes for newline paths and capture IO.

Validation at this head

• All five hosted checks pass, including the real Bash 3.2 build and offline suite: https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/42102246278 .
• Hosted pre-commit passes: https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/42102246275 .
• Both implementation commits passed actual global commit hooks in an isolated, non-root, init-enabled Docker environment; pre-push hooks also passed. No hooks skipped.
• Final focused regression run: 49 tests passed. Startup-shim suite after the spawn-race fix: 27 tests passed. Full suite runs are included in the successful commit hooks; earlier development-only counts are not substituted for final-head evidence.
• Source-to-commit byte bindings and clean review checkouts were corroborated by the primary orchestrator.

No live backend provisioning was performed. Hosted success does not establish live SBX/MSB behavior. Earlier local gate failures led to explicit baseline process-race fixes, with deterministic injected-failure tests.

Scope and attribution

Eight PR-owned files changed against the integrated main baseline. Two new commits integrate main and implement the repaired fetch/capture behavior; detailed history remains available in this PR. Clearing environment centrally in an external global hook remains out of scope: repository tests must be safe independently.

AI-assisted implementation, investigation and independent QA/security review using OpenCode. Agent review evidence is not a substitute for the required human re-review. No secrets or live backend data included.

Current cache-upgrade re-review

Please review af753798b7db773970269dc6498e9a7104973094. This supersedes the earlier bf94734 handoff above and addresses the current-head review’s legacy-cache upgrade blocker.

The MSB caller now materializes into ${ACQ_MSB_KIT_CACHE}/payload-v2/<slug> instead of the legacy repository-bearing <slug> directory. Legacy caches are not adopted or deleted. Existing namespace symlinks and repository obstructions are refused rather than followed or used as fallback. The shared direct-fetch repository protections remain unchanged.

Eight new caller-level tests seed real historical Git caches and prove pinned upgrade, matching symbolic-ref refresh, failed upgrade/retry, configuration non-execution, symlink refusal, and namespace obstruction preservation. Assertions cover old HEAD, refs, config, bytes, modes and payload symlinks/executable bits. Initial focused run passed 47 tests; final corrected upgrade suite passed all eight cases. Both new commits passed actual full commit hooks in the isolated validation container; normal pre-push hooks passed. Final-head hosted checks must be read from this PR, not inferred from the earlier runs linked above.

Independent QA/security re-review found no remaining actionable defect after correcting the symbolic-ref fixture. Nexus supplied an adversarial excerpt review; its input was corroborated against source and tests, not treated as full-source approval. Lossy slug aliases and cooperative cache ownership remain inherited assumptions; this does not claim protection against hostile concurrent writers.

File-size exception

acq.backends/msb.sh, acq.backends/kit-translate.sh and scripts/verify-backends already exceed the repository guideline. This repair retains the existing adapter/helper boundaries and makes a small caller change rather than combining a large refactor with an upgrade fix. The new upgrade suite is a separate 211-line file. A broader module split would require its own behavior-preserving design and review; it is not silently performed here. Duplicated incident narratives in the fixture helpers were shortened to the required operational constraint.

AI-assisted implementation and review (OpenCode); current source revision and validation limits stated above. No Quickstart release was cut.

[mogul]

Changes requested.

1. High - bare Git repositories bypass the new destructive-destination guard (acq.backends/kit-translate.sh:118-123). git rev-parse --show-toplevel emits no working-tree path for a bare repository, so the guard permits that destination. The subsequent fetch path runs git init and git remote add in the bare repository, converting/mutating it before the fetch fails. I reproduced this with an isolated git init --bare fixture: after kit_translate_fetch returned nonzero, the directory was no longer bare and had remote.origin.url=https://example.invalid/x.git. Detect bare repositories as well as work-tree repositories before any destructive operation, and add a regression test proving a bare repository remains untouched.

The Bash 3.2 required workflow is currently failing at 30-dispatch-routing.bats:318; its failure appears to arrive from the merged main changes rather than the isolation tests (which pass under Bash 3.2), but it must be green before merge.

[wz-gsa]

Local follow-up: fixed bare-repository bypass using --absolute-git-dir; added populated-bare non-mutation and symlinked-metadata tests. Both reproduced failure before fix;27focused tests now pass. Independent review approves guard change. Refreshed from current main with no conflict. Publication blocked: normal test-acq pre-commit hook did not complete within900s; no hooks skipped and no commit/push occurred. Secret scanning and ShellCheck pass. Identity override t@example.com found and removed with user authorization, restoring verified human identity. Two narrowly file+literal-scoped Gitleaks allowlists handle main startup-probe PLACEHOLDER fixture; both secret-scan layers now pass. Work remains staged locally pending diagnosis of stalled full-suite hook.

[wz-gsa]

Hook diagnosis now has concrete evidence: test-acq defaults to serial on this macOS host (nproc absent), with833tests and buffered hook output. Visible runs continued advancing, so the15min timeout alone did not establish one frozen test. Tests invoke ambient python3 after moving HOME into each synthetic fixture: observed mise downloading/installing Python3.13.16 during the supposedly offline95-kit-ref-persistence test. Parallel snapshot also showed real git-remote-https children. Provider-input no-follow path checks reject macOS /var→/private/var;110-volumes fails3cases with normal TMPDIR but passes12/12 using its physical path. Diagnostic BATS_TEST_TIMEOUT cannot be trusted because test helper exports sleep(){ :; }, making its watchdog fire immediately. One101grammar output-pipeline timeout was not repeatable (five isolated reruns completed); not treating it as root cause. Next fix should make test runtime/path selection deterministic and enforce offline boundaries rather than increase commit timeout or bypass hooks.

[wz-gsa]

Published follow-up44597aec4e1ab799b9185111d27fc7411e9aa409. @mogul please re-review. Bare repository blocker fixed with --absolute-git-dir and populated-bare/symlink regressions. Expanded scope is explicit: deterministic installed Python before HOME isolation; writable fixture wrapper (never symlink host executable); physical temp roots; file-only Git transport; restored offline projection boundary with real147tests opting in; automatic bounded4worker execution and progress; corrected readiness/capability fixtures; Bash3.2 create/remove EXIT-trap lock retention fix in both adapters; probe cleanup continues on macOS group-check EPERM. Two narrow file+literal Gitleaks exceptions cover the existing PLACEHOLDER fixture. Full normal commit hooks passed without skipping: Bats825passed/16skipped (841total), startup-probe26passed, secret scanning, ShellCheck, hygiene and executable checks. Push hooks passed. Required remote CI newly triggered and unmeasured. Real pinned Patterns contract cases remain CI-only. During development an unsafe test interpreter symlink allowed a fixture to overwrite the mise executable; corrected to disposable wrapper, added a host-byte-preservation regression, and restored Python3.13.16 with user authorization. Host identity and runtime verified after tests. No live backend provisioning claims.

[mogul]

Verdict: REQUEST CHANGES

AI-assisted adversarial re-review by OpenCode (GPT 6.1), including an independent review sub-agent, requested by @mogul, of #596 at 522456fb48352f51e099b8ec51aa4422ab57bfcc.

Blocking finding

P1 — The destination guard rejects legitimate re-fetches and failed-fetch recovery (acq.backends/kit-translate.sh:179–182). A successful direct fetch leaves $destdir/.git; a failed fetch also leaves initialized Git metadata. Unconditionally rejecting any existing repository prevents subsequent calls from refreshing or recovering these acq-created scratch repositories.

Both required CI jobs reproduce this regression:

• test/bats/102-kit-fetch-reuse.bats:102: the second symbolic HEAD fetch into the same scratch destination fails with kit-translate: refusing to fetch into a git repository: …/a.
• test/bats/102-kit-fetch-reuse.bats:120: after an intentionally failed fetch, the next attempt is rejected by the guard instead of reaching revision verification; the later successful recovery cannot proceed either.

This affects production callers: _acq_msb_fetch_kit reuses stable cache destinations, while symbolic/abbreviated refs bypass invocation reuse. Failed invocation-source fetches also leave a repository without acq-verified, which the next attempt cannot refresh.

Distinguish demonstrably owned disposable fetch state from arbitrary repositories, or fetch through fresh owned staging directories. Preserve the work-tree/bare-repository protections and the existing refresh/recovery assertions; do not remove the guard or weaken the tests to obtain green CI.

Previous blocker: addressed

The replacement with git rev-parse --absolute-git-dir correctly covers bare repositories. Both CI jobs pass the new populated-bare preservation and symlinked-metadata refusal tests. They also pass the work-tree/root/empty-destination checks and clean-scratch check. The Git-local environment clear runs in the current shell before fixture Git operations; the isolation tests and negative control pass under both ordinary Bash and Bash 3.2.

Verification and limits

Reviewed the entire three-file diff, exact-head source/callers, prior formal review and actual logs for https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/40766247762 and https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/40766247732 . Both required jobs fail on the two fetch-reuse tests above; these are not the earlier dispatch failure. All 10 added tests pass in both jobs. The broader follow-up changes described in PR comments are absent from this head and were not credited as implemented.

Local reproduction was BLOCKED because the review sub-agent could not create its approved /tmp/opencode fixture location (permission denied); no reviewed code was executed locally. No source/branch changes, dependencies installed or reproduction network requests. Working tree remains clean. Required CI must pass after the regression is fixed.

This is an AI-assisted review posted using the requesting account, not a substitute for independent human review or production authorization.

[mogul]

Verdict: REQUEST CHANGES

AI-assisted adversarial re-review by OpenCode (GPT 6.1), requested by @mogul, of #596 at bf9473408b9224960fbf838239d3d5ac7fa82509.

Blocker

P1 — Existing MSB kit caches cannot transition to the metadata-free fetch layout.

The repository refusal at acq.backends/kit-translate.sh:130–135 correctly protects arbitrary repositories, but _acq_msb_fetch_kit still uses the same stable ${ACQ_MSB_KIT_CACHE}/${slug} destination and explicitly passes destinations containing .git back to kit_translate_fetch (acq.backends/msb.sh:1295–1303). The previous implementation created .git in those cache directories. After upgrading, any unchanged kit ref with a populated legacy cache now fails before transport; the MSB provisioning loop treats that failure as fatal (acq.backends/msb.sh:3540–3544). Invocation-source reuse does not help because destination validation runs first.

Local offline reproduction: I sourced the working checkout's pre-change fetch implementation, routed its fetch to a real local Git fixture, and fetched a pinned kit into the same slug-shaped cache destination used by MSB. It succeeded and left .git. Sourcing this PR's exact-head implementation and fetching the identical ref into that destination produced:

OLD_FETCH_RC=0
OLD_PAYLOAD=kit payload
LEGACY_METADATA=present
NEW_FETCH_RC=1
NEW_OUTPUT=kit-translate: refusing to fetch into a git repository: …/cache/git_https___example.test_kits.git_ref_<pin>_dir_kits_one
PRESERVED_PAYLOAD=kit payload

The old cache remains intact, which is good, but existing installations lose the ability to apply those kits. The new regression tests refresh destinations first produced by the new implementation; they do not test upgrading from the old repository-bearing layout.

Required change: introduce a safe cache-layout transition in the owning caller—for example, a distinct versioned metadata-free cache namespace that leaves legacy repositories untouched—or another demonstrably safe ownership-aware migration. Do not simply remove the repository guard, delete arbitrary .git directories, or trust a forgeable marker. Add an upgrade regression that seeds a legacy cache using the previous layout, exercises the MSB caller, proves the same ref can be materialized after upgrade, and proves legacy repository contents/config/refs are preserved. Keep the direct/reuse repository-preservation and refresh/recovery tests.

Nitpicks / non-blocking maintainability observations

• test/bats/helper.bash:68–86 and the introductory comments in test/bats/155-git-fixture-isolation.bats repeat a lengthy incident narrative. Keep the actionable constraint—git -C does not override inherited repository-local variables, and the unset loop must run in the current shell—but trim duplicated history per Coding Practices §13.6.
• The changed production files already substantially exceed the §13.3 file-size guideline (kit-translate.sh: 1,736 lines; verify-backends: 1,677). This PR expands them further. Add the required written exception rationale to the PR description; I am not requesting an unrelated broad refactor as a condition of this fix.

Prior findings and verification

The earlier bare-repository blocker and fresh-destination re-fetch/recovery blocker are repaired: private source staging, metadata-free consumer publication, pinned revision checks, destination rechecks, and rollback coverage are present. I reviewed the eight-file diff, production callers, prior formal reviews, and actual hosted job logs rather than relying solely on the PR description.

• Local PASS: python3 scripts/test-probe-msb-startup.py — 27 tests, OK.
• Local PASS: bash scripts/run-shellcheck acq.backends/kit-translate.sh scripts/verify-backends test/bats/102-kit-fetch-reuse.bats test/bats/145-verify-capture.bats test/bats/155-git-fixture-isolation.bats test/bats/helper.bash — exit 0, no diagnostics.
• Hosted PASS: pre-commit at https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/42102246275 — lint, secrets scan, full Bats suite (1,187 planned cases, with reported skips), and 27 startup-probe tests. The fetch/reuse/isolation/capture cases reviewed here pass.
• Hosted PASS: actual Bash 3.2 build and offline suite at https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/42102246278, including the relevant regression cases.
• Local full Bats suite NOT RUN: vendored Bats submodules are uninitialized; no dependencies were installed. /tmp/opencode is not writable, so local inspection and the offline reproduction used a private directory inside the current workspace instead.
• No live SBX/MSB provisioning performed; offline and hosted results do not establish live backend behavior. No source/branch changes or credentials included. The exact PR head was rechecked before submitting this review.

This is an AI-assisted review posted using the requesting account, not a substitute for independent human review or production authorization.

[mogul]

Verdict: REQUEST CHANGES

AI-assisted adversarial re-review by OpenCode (GPT 6.1), including an independent full-diff review sub-agent, requested by @mogul, of #596 at af753798b7db773970269dc6498e9a7104973094.

Blocking finding

P2 — The metadata-free cache transition misses persistent provider-kit discovery caches. The changed repository guard (acq.backends/kit-translate.sh:130–135) protects legacy repositories correctly, but acq_kit_source_provider_kits (acq.backends/common.sh:2928–2936) still fetches into ${XDG_CACHE_HOME}/acq/kit-sources/<seed>, without a versioned namespace transition.

Accepted symbolic/abbreviated source refs can leave repository-bearing discovery caches under the old implementation. After upgrade, the new guard rejects those caches. Discovery suppresses the fetch diagnostic and returns success with no kits. acq kit-source ls consequently omits discovered providers, and acq_provider_kit_candidates (common.sh:2809–2824) omits them from configuration choices. Retrying hits the same legacy repository and cannot recover.

The independent reviewer reproduced this offline using exact base/head fetch implementations, the exact-head discovery function, real local Git objects and a wrapper routing fetch exclusively to a local fixture. A valid symbolic HEAD source produced:

OLD_DISCOVERY=[usai-provider example.test/kits HEAD kits]
LEGACY_METADATA=present
NEW_DISCOVERY_RC=0 NEW_DISCOVERY=[]
NEW_DIRECT_RC=1
LEGACY_HEAD_PRESERVED=yes
kit-translate: refusing to fetch into a git repository: …/cache/acq/kit-sources/2558038037

I independently corroborated the exact-head caller's unchanged destination, suppressed failure/empty-success branch, candidate consumer and repository guard. This is a demonstrated upgrade regression, not a speculative hostile concurrent-writer issue.

Required change: apply a safe metadata-free namespace transition to this persistent discovery caller too, leaving legacy repositories untouched. Add caller-level upgrade coverage using a historical repository-bearing cache, proving provider discovery succeeds and legacy contents/config/refs remain preserved. Keep the shared repository guard and existing MSB upgrade/re-fetch protections.

Prior blockers: addressed

The MSB caller now correctly uses payload-v2 (msb.sh:1295–1308), leaves legacy directories untouched and refuses namespace symlinks/repository obstructions. Bare/worktree/nested-repository protections remain. Private staging and metadata-free publication preserve fresh-destination refresh/recovery and rollback behavior. Capture review covered foreground stdin, final shared cutoff, reader reaping and explicit I/O failure propagation. Startup-shim handlers precede child spawn, with pending-termination forwarding and child-reaping coverage. No new credential-exposure or security vulnerability was verified.

Verification and limits

Read the full ten-file diff, exact-head source/callers/tests, latest blocking reviews, repository rules and full Coding Practices. Independent reviewer inspected actual current-head logs:

• Pre-commit: https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/42336638094/job/128294928944 — successful; 1,195 planned Bats cases, including eight MSB upgrade tests and relevant fetch/isolation/capture cases; startup probe 27 tests, OK. Secrets scanning, ShellCheck, hygiene and executable-bit checks pass.
• Bash 3.2 workflow: https://github.com/GSA-TTS/agentic-coding-quickstart/actions/runs/42336638081 — all three actual test shards pass, with relevant cases present in their logs. The interpreter cache was used; this is not a claim it was newly compiled in this run.
• All nine reported current-head check runs succeed. These checks do not cover the missing discovery-cache upgrade case.

Independent exact-head shell/Python syntax checks passed; the local offline discovery reproduction demonstrated the blocker. Full local Bats, live SBX/MSB provisioning and independent SCA were not run. No dependencies installed or tracked source files modified. Local reproduction was reported under approved temporary storage; the existing untracked workspace scratch directory was left unchanged. Required branch-protection configuration is inaccessible to this token (HTTP 403); only reported checks were independently verified.

Existing oversized production modules have a written exception rationale in the PR description; no unrelated split is requested. Lossy aliases/cooperative cache ownership are inherited assumptions, not additional blockers.

This AI-assisted review is posted using the requesting account, not independent human review or merge/production authorization. No credential values included.