docs+test(ps1-exit): T1 contract freeze + PS1-vs-Rust parity harness - #57
Conversation
…s1 contract (T1) Mechanically inventory every externally observable behavior of the two PS1 monoliths ahead of the T2-T5 port: full param() blocks, -Mode and Skip*/Allow*/Require* semantics, env vars, exit codes, artifacts, and side effects, each cited to a real file:line. Cross-references the 51 scripts/tests/*.ps1 suites to separate covered behavior from gaps. Corrects the ticket's file attribution for Invoke-SentruxAgentTool.ps1: shim management and pro activation actually live in tools/sentrux-shim/*.ps1 (given a light pass here), and Sentrux failure normalization lives in run-code-intel.ps1 itself, not the sentrux agent tool. Confirms -SkipGitHubResearch and Test-GitHubSolutionResearchRequired are dead code (kill-candidate). Identifies that 8 of run-code-intel.ps1's early-exit blocks are already thin forwarders to the Rust CLI (shim, not port). Addresses #46.
scripts/tests/test-ps1-rust-parity.ps1 runs the same repo snapshot through run-code-intel.ps1 and `code-intel run execute` (mirroring ci.yml's authoritative self-scan invocation exactly), then diffs the run-manifest node set + verdicts, hospital-report.json status/diagnosis, and sentrux structural-evidence metrics after normalizing volatile fields (timestamps, temp-root paths, content hashes) using the ConvertTo-NormalizedCanonicalValue pattern already established in test-parity-baseline.ps1. This is observational, not a gate: divergence is expected at T1 and the harness reports it precisely (JSON verdict with an explicit diverged[] list) rather than hiding it. Run twice this session against a release build; both runs produced byte-identical verdicts. Addresses #46.
New, separate workflow (does not touch ci.yml/release.yml/skill-check.yml, which have open PRs against them) that runs the T1 parity harness on pull requests touching run-code-intel.ps1, Invoke-SentruxAgentTool.ps1, the harness itself, or crates/**, plus workflow_dispatch. Single windows-latest job, continue-on-error at both job and harness-step level so a divergence report can never block a PR, uploads the JSON verdict as a build artifact. Addresses #46.
…erence A background sub-agent independently re-swept scripts/tests/*.ps1 plus a sibling tests/ directory the primary pass had missed. Re-verified every disagreement against actual file content before writing it in: - tests/*.ps1 (5 files, not previously searched) covers -ProactiveSkillSuggestions/-AutomaticPullRequests via test-follow-up-automation.ps1. - -RepowiseProvider/-RepowiseModel/-RepowiseReasoning were wrongly marked covered: the original grep matched unrelated function-name substrings (Get-RepowiseProviderArgs, Normalize-RepowiseProvider), not the actual flags. Corrected to a documented gap. - -RepowiseScopePaths/-RepowiseRootFiles are covered via config-resolution (camelCase repo.json keys), split out from -RepowiseWorkspaceRoot (weak/hollow fixture match) and -RepowiseShadowRoot (genuine gap). - -SurvivalScanArtifactRoot is covered; -SurvivalScanRequest is not (previously reported as one undifferentiated gap). - Invoke-SentruxAgentTool.ps1's -Tool operations are invoked positionally, not as -Tool "value" flags -- explains why naive grep found zero per-operation coverage. Re-derived precisely: 3 of 11 operations (dsm, health, git_stats) are actually covered. Row/classification tallies in Section 6 and the coverage summary in Section 4 recounted programmatically after these corrections (87 rows, was 83; 60 covered / 23 gaps / 4 partial).
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The first real CI run on this branch surfaced a genuine gap: a fresh
windows-latest runner has no rg on PATH, so both the Rust CLI ("cannot
launch rg.exe: program not found") and run-code-intel.ps1 ("Missing
required tool: rg", :3499) failed before producing any artifacts to
compare. ci.yml already has a matching "Install ripgrep" step for its
own jobs -- this workflow needs the same, just missed initially.
The harness itself handled the missing-artifacts case correctly (no
crash, verdict still emitted and uploaded with rustArtifactsFound /
ps1ArtifactsFound all false) -- this fix is about making the comparison
meaningful, not about a harness bug.
…p gap Section 5 now documents the third, independent confirmation of the harness's degrade-gracefully behavior: the first parity-observe.yml run on a genuinely fresh windows-latest runner hit a missing-rg environment difference neither local run could have exercised, and the harness handled it correctly (coherent degenerate verdict, no crash).
Summary
T1 of the PS1-monolith retirement campaign (#55). Pure analysis + test
infrastructure — zero production behavior changes. Three deliverables:
docs/ps1-exit/contract-inventory.md— mechanical behaviorinventory of
run-code-intel.ps1(4723 lines) andInvoke-SentruxAgentTool.ps1(3125 lines), every row cited to a realfile:line. Plus a light pass ontools/sentrux-shim/*.ps1(906lines) — see "scope correction" below.
scripts/tests/test-ps1-rust-parity.ps1— one command that runsthe same repo snapshot through both the PS1 pipeline and
code-intel run execute, diffs the comparable artifacts, and emits aJSON verdict. Observational, not a gate.
.github/workflows/parity-observe.yml— new, non-blockingworkflow (does not touch
ci.yml/release.yml/skill-check.yml,which have open PRs fix(ci): harden workflow supply-chain and injection surface #43/fix(ai-safety): prompt injection boundary, hardened model delegate, audit regression fixtures #44/fix(audit): ai-safety-003 validate wiring; verify v0.5.1 self-dogfood slice holds #45/fix(security): resolve tool paths absolutely, close digest evidence gaps #56 against them) that runs the
harness on PRs touching the two monoliths or
crates/**,continue-on-errorat both job and step level, uploads the verdictas a build artifact.
Inventory counts
run-code-intel.ps1Invoke-SentruxAgentTool.ps1tools/sentrux-shim/*(bonus light pass)Grand total: 87 behaviors documented, 60 covered by existing tests, 23
gaps (+4 partial) — full per-behavior detail and a proposed minimal test
for each gap is in the doc's §4.2 (gaps are recorded, not fixed here, per
the ticket). These numbers reflect a self-correction: a supplementary
automated cross-reference caught a false-positive in the primary manual
sweep (see "coverage self-correction" below) and a sibling
tests/directory the manual sweep hadn't searched; both are folded in.
Environment variables: 13 read/written directly in
run-code-intel.ps1(5 console-encoding writes, 8 reads — DSM provider preference, rust-CLI
override, repowise provider/model/reasoning x2 names each), 3 in
Invoke-SentruxAgentTool.ps1(writes only, subset of the same 5), plusSENTRUX_CORE_EXE(new, found in the sentrux-shim light pass) and 2transitive reads (
CODE_INTEL_ARTIFACT_ROOT,CODE_INTEL_SHADOW_ROOTviatools/code-intel-platform.psm1).Classification tallies (counted programmatically from the doc's own
tables, not estimated): port 64, shim 18, kill-candidate 1 (documents
two independently-dead symbols:
-SkipGitHubResearchandTest-GitHubSolutionResearchRequired, both confirmed zero-effect/zero-call-sites),uncertain (
port?/shim?) 4.Scope correction (documented in §0 of the inventory)
The ticket described
Invoke-SentruxAgentTool.ps1's "distinctivesurfaces" as shim management, pro activation, and failure normalization.
Verified by grep against the live file: zero matches for any of those
concepts. Traced the real locations instead: shim management + pro
activation live in
tools/sentrux-shim/sentrux-shim.ps1(not named in the ticket — given a light pass here anyway), and Sentrux
failure normalization lives in
run-code-intel.ps1'sNew-CodeIntelSentruxFailures— i.e. the other named monolith.Invoke-SentruxAgentTool.ps1is, verifiably, an 11-operation DSM /git-churn / complexity-metrics engine. Documented as what it actually is
rather than forced to match a premise the code contradicts.
Also confirmed, with evidence: 8 of
run-code-intel.ps1's early-exitcode paths (
:113-247) are already thin forwarders to the Rust CLI —they resolve
$defaultRustCli, invoke it, andexit $LASTEXITCODE. Theseclassify as
shim, notport: the retirement work for that slice is"stop routing through run-code-intel.ps1", not "port logic to Rust" (the
logic is already there).
Coverage self-correction (documented in §4 of the inventory)
A background sub-agent independently re-swept test coverage and disagreed
with the primary manual pass on several rows. Every disagreement was
re-verified directly against actual file content (not trusted from either
side blindly) before landing:
tests/directory (5 more PS1 files, separate fromscripts/tests/, not named in the ticket) covers-ProactiveSkillSuggestions/-AutomaticPullRequestsviatests/test-follow-up-automation.ps1— the manual pass had onlysearched
scripts/tests/.-RepowiseProvider/-RepowiseModel/-RepowiseReasoningwere wronglymarked covered in an earlier draft: the manual grep matched unrelated
function-name substrings (
Get-RepowiseProviderArgs,Normalize-RepowiseProvider), not the actual CLI flags. Corrected to adocumented gap — left in as an explicit self-correction rather than
silently fixed.
Invoke-SentruxAgentTool.ps1's-Tooloperations are invokedpositionally (
& $tool dsm $path), never as-Tool "value"— whichis why a naive quoted-string grep found zero per-operation coverage.
Re-derived precisely: 3 of 11 operations (
dsm,health,git_stats)are actually covered.
Current parity divergence (harness run twice this session, byte-identical verdicts)
{ "ok": false, "compared": 17, "matched": 4, "rustExitCode": 0, "ps1ExitCode": 0 }Both paths completed successfully; the divergence is entirely in artifact
content, not process failure:
pairing this harness can currently assert with real evidence
(
evidence.sentrux<->sentrux check) matched. Every other RustDAG node and PS1 step has no asserted counterpart yet (expected at T1 —
that mapping is T2-T5 work).
post_op(clean,domainVerdict: pass), PS1 lands indiagnose(
primary_diagnosis: "architecture graph missing",overall_score: 54).Root cause: this worktree has no
knowledge-graph.json, and PS1'shospital-decision logic treats that as diagnostically significant while
Rust's evidence provider does not. A genuine, concrete divergence, not
a harness artifact.
the two
code-intel-hospital.v1producers: PS1 nests its verdict undertriage, Rust nests it underdiagnosis+ a top-leveldomainVerdict.Same schema string, incompatible shape — the single highest-value
finding for whoever picks up the hospital-report port ticket.
ok: falseis the expected, correct result right now — exactly why theCI workflow is
continue-on-error.?rows (could not determine with full confidence)-Language's effect scope (only traced one advisory-string consumer)dsmtool operation's port-vs-delete framing (partially ported —rust-primary/PS-fallback already exists, but the fallback is 900+ lines
of real logic)
sentrux-lite-core.ps1's own classification (out of budget for a fullread; flagged as reasonable follow-up scope, not assumed)
Full reasoning for each is inline in the doc next to its
?marker.Verification
pwsh -NoProfile -Command "$null = [scriptblock]::Create(...)"— harness syntax OKpython3 -c "import yaml; yaml.safe_load(...)"— workflow YAML OKcargo build --release --lockedbinary;
diffon the two verdict JSON files reported no differencesAddresses #46.
Update: real CI run caught a genuine gap this session's local runs couldn't
The first run of
parity-observe.ymlon this PR (freshwindows-latestrunner) had no
rgon PATH -- an environment difference this dev boxnever exercises. Both paths failed early as a result, but the harness
did not crash: it produced a coherent, correctly-degenerate verdict
(all artifact-found flags
false,compared: 6,matched: 5) and theworkflow uploaded it -- validating the tolerant-of-missing-artifacts
design under a real failure mode. Root cause was
parity-observe.ymlmissing the same "Install ripgrep" step
ci.ymlalready has -- fixed ina follow-up commit (
215dae1), documented in the inventory's §5.