Repository navigation
Conversation
…t change needs workflow scope)
basiliskus
left a comment
There was a problem hiding this comment.
Verdict: request changes — a714d671001bcb5dd9f2f107f990ae95db9d176b
-
major —
scripts/verify-host-authority-live:622: Establish stale provenance before testing refresh.acq kit updateexits successfully without applying anything when provenance is current (acq:1060–1063). Fresh creation records current provenance, and changingbuiltin-kitsdoes not change that currency signal. Consequently this step can label a healthy backend FAIL without exercising refresh. Confirmed by source tracing and an extracted-function offline probe. Make only this throwaway sandbox stale and confirm all expected kits were attempted. -
major —
scripts/verify-host-authority-live:623(also:612,:531): Do not infer PASS from partial output while ignoring command failure. One successful kit's “refreshed” line makes the entire refresh PASS even if another kit fails and the command exits 1. The install-gate step likewise passes when the proof counter advances but restart times out (124). Confirmed with offline injected-command probes. Require successful execution and complete per-step evidence; classify timeout/probe failures as BLOCKED rather than a product result. Add runtime regression tests, not just dry-run assertions. -
major —
scripts/verify-host-authority-live:172–188: Use uniquely owned manual fixtures and cleanup. MANUAL 5 reuses$HOME/hal37, overwrites its sentinel, then instructsrm -rf "$D"; the fixed sandbox names are also force-removed. These commands can damage files/sandboxes the check did not create. Confirmed by tracing the printed commands. Use a checked unique scratch directory and collision-safe names; clean only resources created by that run. -
minor —
scripts/verify-host-authority-live:414: Bound all backend probes.proof_linesinvokesmsb execwithout timeout; version/doctor preflights are also unbounded. A hung backend can bypass the advertised timeout and prevent reaching cleanup. Confirmed by source tracing. Route these through bounded capture with explicit status handling.
Not verified: No live backend execution; local Bats unavailable (uninitialized submodules). Saved CI logs show the 14 new parsing/dry-run tests ran under bash 3.2; they do not exercise these runtime paths.
Assisted-by: OpenCode [gpt-6.1-sol-latest-guardrails-defaultv2] skill:change-review
…es own provenance stale
…vidence; collision-safe names; bounded probes; runtime tests
…ailures are BLOCKED; null active_config is BLOCKED
|
🤖: AI agent (First Mate), posting on behalf of @mogul. Human review required before merge. Addressed the review in one push (head 479aca0, merged origin/main with plain
Also: Tests: new |
…pipes timeout | tee waited for every descendant holding the pipe, so a TERM-ignoring background helper hung the whole run past --create-timeout. Commands now write to files, run in the background and are polled against a wall-clock deadline; on expiry the process tree is TERMed then KILLed. Adds per-step budget, hang diagnostics (default on), bounded cleanup, and a summary that always prints, including after SIGINT. A timed-out step is BLOCKED. Co-authored-by: OpenCode Agent <user@gsa.gov>
|
🤖: AI agent (First Mate), posting on behalf of @mogul. Human review required before merge. Fix pushed for the unbounded wait in Cause (reproduced offline): Changes:
This fixes the unbounded wait. It is NOT live proof, and the ORIGINAL stuck process on the maintainer's macOS run is still unidentified until it is rerun with this revision (the transcript will now show what was stuck). |
🤖: AI agent (First Mate), AI-assisted change (OpenCode), posting on behalf of @mogul. Human review required before merge.
Summary
Prepares the human-run live validation for #639. This sandbox has no
sbxormsb, so nothing here was run live. The script was only syntax-checked (bash -n), shellchecked, and dry-run tested through offline bats. No live result is claimed or recorded; every row in the new results template isBLOCKED/unrun.scripts/verify-host-authority-live: walks the "Still unverified on a real host" checklist. Prints PASS / FAIL / BLOCKED per step with the acq revision and backend version, usestimeout --foreground, tees live output to a transcript, has a per-step timeout, an INT/TERM/EXIT trap that removes created sandboxes, lowercasehal-<ddhhmmss>-<x>names,ACQ_NO_PROMPT=1, closed stdin, and never prints secrets. Output defaults to<repo>/tmp/verify-host-authority-live-<stamp>/(--out DIRoverrides):transcript.log,results.tsv.--dry-runprints the plan and MANUAL instructions and touches nothing.docs/VERIFY_PROVIDER_ADOPTION.md: new "Host Authority Live Validation Record" template (revision, backend version, date, result, transcript path), allBLOCKED/unrun.test/bats/161-verify-host-authority-live.bats: 14 offline cases for argument parsing and--dry-runonly.test/bats/162-verify-host-authority-live-runtime.bats: 21 RUNTIME regression tests of the verdict logic against stubbed acq/sbx/msb (seamsHAL_ACQ_BIN,HAL_RUN_ID); stub results are not live proof.Automated steps (12): msb:
msb-ro-kit-create(readonly fixture kit file end to end on create; unblocked by the Patterns v1.19.0 pin),msb-inspect-shape(.active_config.mountsand.config.mounts),msb-exit-code,msb-ro-kit-heal(restart),msb-forged-markers(exec sessions),msb-install-gate(delete hostinstall-*keys),msb-ro-oci(shippedoci-enginereadonly files). sbx:sbx-baseline,sbx-forged-heal,sbx-record-change,sbx-stale-refresh,sbx-legacy-reseed.Manual (7, printed with exact commands and what to observe): msb interactive forged markers and agent key; forged ssh socket with a live agent; reboot persistence; Linux / other msb versions; ADR-0037 steps 1-4 and 6; steps 5 and 8; step 7 (Windows).
Not repeated (already validated live per the issue): the msb 0.7.7
qualify-msb-ro-mountcanary, the default-sandboxmsb inspectmount shape, the provider-adoption gate.Issues
Refs #639 (not closing it: the live runs are still to be done by a human). Depends on #649 (Patterns pin, merged).
Before / After
Before: the checklist in #639 was prose only. After: one host command produces per-step PASS / FAIL / BLOCKED rows plus a transcript, and a results table exists to record them.
Verification
Run in this sandbox (offline only; no sbx/msb available):
bash -n scripts/verify-host-authority-live: okshellcheck --severity=warning(viascripts/run-shellcheck, script and new bats): clean./scripts/test-acq-batson bats 161, 162, 158, 150: 50 ok, 0 not ok (head 479aca0)scripts/test-acq-bash32 --syntaxcould not run here (no bash 3.2 available); CIbash32-compatis the real check./acq kit validateon the generated fixture kit: OKbash32-compat)Not verified: every live code path (the 12 automated steps' real behavior against msb and sbx). Expect the first live run to find script bugs; treat a script error as BLOCKED, not as a product failure.
Review round 1 (basiliskus)
Addressed in 479aca0: stale-refresh now forces only its own sandbox's provenance stale, verifies
acq kit checksays stale, and requires exit 0, no warnings and all four kit lines; every step requires exit status 0 plus complete evidence, and timeouts and probe or inspect failures are BLOCKED, not FAIL; the install gate cannot PASS on a timed-out restart; unique run-id sandbox names with a name-collision refusal, cleanup only of sandboxes this run created, scratch dirs via checkedmktemp -dunder the output dir, MANUAL 5 uses checked scratch dirs with run-id names; all backend probes go through the bounded runner.Merge Guard
Draft. Do not merge until a human has reviewed. Follow-up for a human with workflow scope: add
scripts/verify-host-authority-liveto the relevance list in.github/workflows/bash32-compat.yml(the push token lacksworkflowscope, so I listed it underNOT_INPUTSintest/bats/158-bash32-relevance-paths.bats; the script is only executed in--dry-runby the suite). That move is a one-line change in both files.Rollback
Revert this PR; it adds one script, one bats file, one docs section, and one
.pre-commit-config.yamlregex entry. No product code changes.Security Impact
None to acq. The script runs only on a host a human chooses, creates throwaway lowercase-named sandboxes, and removes them on exit or interrupt. It uses existing acq credentials but never prints, seeds, or writes secret values;
msb inspectJSON (placeholders only) is saved under the gitignoredtmp/. It deletes only hostinstall-*keys and edits the hostbuiltin-kitsrecord of its own throwaway sandbox. It does not push or modify CI.Co-authored-by: OpenCode Agent user@gsa.gov