Update documentation with 9 changed files (#4522)

[rysweet]

Summary

Concise workflow-generated PR for documentation.

Issue

Closes #4522

Changed files

• docs/howto/converge-a-stuck-red-canary-self-deploy.md
• docs/index.md
• docs/reference/canary-gate-convergence.md
• docs/reference/canary-unit-test-gate-diagnostics.md
• mkdocs.yml
• src/operator_cli/mod.rs
• src/self_relaunch/gates.rs
• src/self_relaunch/types.rs
• src/self_relaunch_semaphore/handoff.rs

Diff stat

 .../converge-a-stuck-red-canary-self-deploy.md     |   5 +
 docs/index.md                                      |   1 +
 docs/reference/canary-gate-convergence.md          |   5 +
 .../reference/canary-unit-test-gate-diagnostics.md | 321 ++++++++++++++++
 mkdocs.yml                                         |   1 +
 src/operator_cli/mod.rs                            |   2 +-
 src/self_relaunch/gates.rs                         | 406 ++++++++++++++++++++-
 src/self_relaunch/types.rs                         |  89 +++++
 src/self_relaunch_semaphore/handoff.rs             |   2 +-
 9 files changed, 814 insertions(+), 18 deletions(-)

Behavior

Implemented behavior through these branch commits:
587f7d2 fix(self-deploy): converge stuck red-canary self-deploy with diagnosable unit-test gate (#4522)

Validation

ℹ NODE_OPTIONS=--max-old-space-size=32768 (saved preference). To change: /home/azureuser/.amplihack/config
I'll start by inspecting the current state of the work — branch, changed files, and the specific changes to gates.rs.

● Get branch, diff stat, status (shell)
│ cd /home/azureuser/src/Simard-deploy-4049/worktrees/feat/issue-4522-simard-self-deploy-is-stuck-
│ in-a-red-canary-crash && git branch --show-current && echo "---DIFF STAT---" && git --no-pager
│ diff main --stat 2>/dev/null | tail -30 && echo "---STATUS---" && git --no-pager status --short
└ 27 lines…

Working tree is clean. Let me examine the key changed file (gates.rs) and understand the changes relevant to this task.

/ Search (grep)

Risk

No high-risk subsystem pattern detected from changed paths.

Checklist

• Branch has 1 commit(s) ahead of main
• Code review completed
• Philosophy check passed

---

This PR was created as a draft for review before merging.

---

Step 16b: Outside-In Testing Results

Tested from the PR branch (feat/issue-4522-simard-self-deploy-is-stuck-in-a-red-canary-crash) as a consumer of the self-deploy canary gate would exercise it.

Detected toolchains: Rust / Cargo workspace (Cargo.toml at root, simard crate, edition 2024, cargo 1.95.0). No Node/Python/Go manifests. Per the qa-team skill's repo-type detection, Rust CLI repos substitute cargo test for the gadugi harness.

Chosen strategy: Native cargo test against the changed module src/self_relaunch/gates.rs. The outside-in boundary is the canary gate's observable verdict (pass/fail + diagnosable detail) — tests spawn real fake candidate binaries and a real fixture crate via cargo, never asserting on private internals.

#	Scenario	Command	Result	Key output
Build	Gate crate compiles cleanly	cargo test --lib self_relaunch::gates --no-run	✅ Pass	Finished test profile in ~64s, executable built
Simple	Unit/diagnosability/convergence gate behavior (real failure surfaces failing test name + test result: FAILED summary; deny-by-default env scrub; hijack-class vars never re-injected; allow-list re-injection)	cargo test --lib self_relaunch::gates:: -- --nocapture	✅ Pass	unit_test_failure_surfaces_failing_test_name_and_summary ... ok
Edge / integration	Healthy candidate PASSES the unit-test gate under --lib scope — real cargo spawn of a fixture crate whose tests/ target would redden the scrubbed all-targets scope	(same run, scope_tests)	✅ Pass	unit_test_gate_passes_for_healthy_candidate_under_lib_scope ... ok

Aggregate: test result: ok. 28 passed; 0 failed; 0 ignored.

This directly validates both #4522 defects the PR fixes: (1) a real failure is now diagnosable (failing test name + summary surfaced from stdout, which the prior truncate_output(&stderr, 200) discarded); (2) the scope/env mismatch is eliminated — the gate is aligned to the proven-green --lib unit scope so a genuinely-healthy candidate goes GREEN, while the LD_PRELOAD/hijack-class deny-by-default defense is proven intact (hijack_class_name_in_canary_env_is_never_reinjected ... ok).

Fix count during outside-in testing: 0 — all scenarios passed on the first run; no code changes required.

[rysweet]

Step 17b — Comprehensive Code Review (#4528 / issue #4522)

Verdict: ✅ Approve. The change correctly addresses both root causes of the stuck red-canary crash-loop (test-scope mismatch + undiagnosable failure) with security properly threaded. All 53 self_relaunch lib tests pass locally (cargo test --lib self_relaunch:: → 53 passed; 0 failed), including the 3 new test modules. No blocking issues.

Checklist

• Code quality & standards — clean, idiomatic Rust; thorough doc-comments tying each change to the #4522 root cause; tracing/OTel only, zero print!/println!/eprint! on the gate path.
• Test coverage adequate — new diagnosability_tests, scope_tests, and redacted_display tests cover the scope fix, both-stream capture, char-boundary safety, DoS bounding, and credential redaction. scope_tests is a real hermetic cargo spawn (fails loudly if toolchain absent — no silent skip).
• No TODOs, stubs, or swallowed exceptions — none. The Err(e) spawn arm is now a loud tracing::error! + fail-closed RED.
• No unimplemented functions — all helpers (parse_unit_test_failure, truncate_output_tail, bound_gate_detail, redacted_display) are complete and tested.
• Logic correctness — --lib matches the proven-green unit scope; parse_unit_test_failure captures stdout+stderr (fixing the prior stdout-discard), surfaces failing-test names + test result: summary + a tail for the no-summary abort shape. Bounds: 20 markers / 8 KiB tail / 16 KiB detail / 512 B emission.
• Edge case handling — multi-byte UTF-8 boundary safety in both truncate_output and truncate_output_tail; empty streams; abort-with-no-summary; oversized flood (DoS guard).
• Security — redacted_display() + bound_gate_detail() scrub URL-embedded credentials (SEC-D2) at every emitting sink; deny-by-default env scrub with is_hijack_class_env (SEC-D3) is preserved and never weakened; LD_PRELOAD-class never allow-listed.

Non-blocking observations

1. Redaction is URL-scheme-scoped. redact_credentials only redacts scheme://userinfo@host authorities. A token appearing outside that shape — an scp-like user@host:path remote, or a GITHUB_TOKEN=ghs_… echoed in a panic/compile message — would pass through into detail. This is acceptable here (the env scrub prevents most token exposure and D4 explicitly scoped out secret-scanning regex), but it's a residual risk worth a code comment so a future reader doesn't assume detail is unconditionally secret-safe.
2. Minor DRY: bound_gate_detail (gates.rs) and redacted_display (types.rs) both implement "redact_credentials + bound to 512 B" with near-identical char-boundary back-off. Consider extracting one shared helper to keep the bound/redaction policy in a single place.
3. TMPDIR omitted from the BASE env floor (the D2 design listed it). cargo/rustc fall back to /tmp on Linux so this is fine today; flagging for portability if the gate ever runs where /tmp isn't the default temp dir.
4. scope_tests spawns a real cargo build inside the unit suite. Fast in practice (ran in 0.17 s, cached) and correctly fails loudly rather than skipping — just noting it adds a toolchain dependency to cargo test --lib.
5. PR title nit: "Update documentation with 9 changed files (#4522)" undersells the change — the substantive work is the canary unit-test gate scope + diagnosability fix, not docs. Consider retitling to match the commit (fix(self-deploy): …diagnosable unit-test gate (#4522)).

None of the above block merge. The fix is correct, well-bounded, fail-closed, and well-tested.

[rysweet]

Step 17c — Security Review (#4528 / issue #4522)

Verdict: ✅ No high-confidence, exploitable security vulnerabilities found. SEC-D2 (secret redaction) and SEC-D3 (deny-by-default env scrub) are correctly implemented and consistently applied at every emitting sink. Verified by tracing all gate-detail sinks, redaction/truncation ordering, the env-scrub allowlist and its call sites, and the handoff file write.

Checklist

• Verify all security requirements met — SEC-D2 & SEC-D3 controls present and wired
• Check for new vulnerabilities — none high-confidence/exploitable
• Confirm sensitive data handling — redaction applied before every emission
• Authn/authz — N/A (no auth surface changed)
• Injection vulnerabilities — none (argv arrays, no shell; env is deny-by-default)

Controls verified

SEC-D2 — redaction applied at every sink, in safe order (redact → truncate):

• verify_canary per-gate tracing: bound_gate_detail = truncate_output(redact_credentials(detail), 512) (gates.rs:153,165–170).
• run_unit_test_gate failure warn! uses bound_gate_detail (gates.rs:258).
• operator_cli/mod.rs:475: raw {r} → r.redacted_display().
• handoff.rs:96: g.to_string() → g.redacted_display(); RpcCallFailed.reason built from already-redacted strings.
• GateResult::redacted_display() redacts then char-boundary-safe truncates to 512 (types.rs:82–99).
• Confirmed the only two production consumers of verify_canary results (operator_cli, handoff.rs) both use redacted_display(). No raw {r}/.to_string()/.detail emission of a GateResult remains in production (raw Display survives only in unit tests, by design).

SEC-D3 — env scrub is genuinely deny-by-default:

• scrub_gate_env calls env_clear(), then re-injects a fixed BASE floor + allowlist (gates.rs:64–103). Dangerous ambient vars not in BASE (RUSTFLAGS, CARGO_BUILD_RUSTFLAGS, CARGO_TARGET_*_RUNNER, GIT_CONFIG_*, LD_PRELOAD, GIT_SSH_COMMAND) are dropped and never re-added.
• is_hijack_class_env (case-insensitive LD_*/DYLD_*/GIT_SSH* + BASH_ENV/ENV/SHELLOPTS/BASHOPTS/IFS) guards the re-injection loop (gates.rs:115–120).
• config.canary_env is populated only from the fixed 3-name canary_gate_env_allowlist() (SIMARD_HOME, SIMARD_PROMPT_ASSETS_DIR, SIMARD_STATE_ROOT) at both call sites (source_prep.rs:677, operator_cli/mod.rs:457) — not attacker-controlled.

Command/argument injection — none. Gates spawn via Command::new + .arg() argv arrays; no /bin/sh -c. Cargo args are literals; only interpolated values are local paths and a numeric timeout (gates.rs:234–241,309–312). No untrusted branch/issue-title/git-URL is interpolated.

Truncation — panic-safe & leak-safe. truncate_output, truncate_output_tail, and inline truncation walk to the next is_char_boundary (no multibyte split panic). Truncation always applied after redaction — no partial-secret leak.

Handoff file — no secret exposure. ready_signal_path writes only {"pid":…,"status":"ready"} (handoff.rs:58–59,164–170) — no gate detail or credentials.

Residual gaps (assessed — NOT high-confidence exploitable)

1. scp-like URLs (git@host:path) and bare tokens (ghp_/ghs_/github_pat_) are not matched by redact_credentials (handles scheme://user:token@host only, source_prep.rs:153–178). But the only data feeding gate detail is cargo test --lib output / candidate stderr; SEC-D3 strips token-bearing env before those subprocesses run, and --lib tests do no authenticated git ops — no realistic path for a live token to reach detail. Real coverage gap, below exploitability threshold.
2. redact_credentials edge cases (#/? in userinfo ends authority scan early) don't apply to real GitHub tokens (alphanumeric + _).
3. is_hijack_class_env denylist omits RUSTFLAGS/CARGO_TARGET_*_RUNNER/GIT_CONFIG_*/*PATH* — matters only if canary_env were ever sourced from untrusted input (currently it is not). Defense-in-depth only.

Non-blocking hardening suggestions

1. Extend redact_credentials with a token-shaped pass (ghp_/ghs_/gho_/ghr_/github_pat_) to close gap 1 as defense-in-depth before any future caller widens detail's data sources.
2. Convert is_hijack_class_env to an allowlist-of-safe (or add RUSTFLAGS/CARGO_TARGET_*_RUNNER/GIT_CONFIG*/*PATH* to the denylist) so the guarantee holds if canary_env is ever sourced from operator/remote input.

Neither is required to merge — current wiring is fail-closed and safe.

[rysweet]

Step 17d — Philosophy Guardian Review (#4528 / issue #4522)

Verdict: ✅ Compliant. The change adheres to the project's ruthless-simplicity and bricks-&-studs philosophy. No blocking issues.

Compliance checklist

• Ruthless simplicity achieved — Each new function does exactly one thing: truncate_output (head-bounded), truncate_output_tail (tail-bounded), parse_unit_test_failure (marker extraction + bounded tail), redacted_display (redact + bound). No speculative abstraction, no config knobs added "just in case." The scrub allowlist (canary_gate_env_allowlist) is a plain Vec<String>, not a framework.
• Bricks & studs pattern followed — Clean contract seams: redacted_display() is the single public "stud" every emitting sink calls; redact_credentials is reused from self_deploy::source_prep rather than reimplemented; gates.rs (execution) and types.rs (data + safe rendering) keep distinct responsibilities.
• Zero-BS implementation — No stubs, no faked APIs, no swallowed exceptions. The unit-test gate is fail-closed (a red candidate stays red) and surfaces real cargo output. 53 lib tests pass, including the new diagnosability_tests, scope_tests, and redacted_display modules.
• No over-engineering — Redaction (SEC-D2) and the 16 KiB / 512 B DoS bounds are proportional to concrete, documented threats, not gold-plating. Marker de-dup + MAX_MARKER_LINES cap is the minimum needed to prevent log-flood.
• No unimplemented functions — All new fns are complete and exercised by tests.
• Clean module boundaries — self_relaunch owns gate types/execution; the only cross-module reach is the deliberate reuse of source_prep::redact_credentials, which is the correct DRY choice.

Non-blocking philosophy notes (advisory, do not block merge)

1. Minor duplication (DRY) — the char-boundary back-off loop appears inline in redacted_display and again in truncate_output/truncate_output_tail. A small shared floor_to_char_boundary(s, n) helper would remove the triplication. Low priority; current form is correct.
2. Doc-comment density — several new fns carry long rationale comments. These are load-bearing (they encode why tail-preservation matters for cargo's trailing summary), so they earn their place — but watch that future edits keep them accurate rather than letting them drift.

The implementation is simple, fail-closed, well-bounded, and modular. Philosophy-compliant — merge-ready.

[rysweet]

Ready for Final Review

Workflow steps completed: requirements, design, implementation, tests, code review, philosophy compliance, cleanup, and quality audit.

Ready for merge approval.

[github-actions]

📊 Coverage Summary

Generated by cargo llvm-cov --workspace --summary-only (nightly, excluding test files)

Module	Lines	Covered	Coverage
Total	196640	165382	84.1%

_{Coverage data from CI run. Test files matching tests?/ are excluded from line counts.}