Close Hospital and Repowise trust boundaries - #8
Conversation
|
Warning Review limit reached
Next review available in: 41 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 Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds scoped Repowise egress manifests and payload freezing, strengthens hospital evidence gating and scoring, resolves repository configuration by physical path, updates control-plane and artifact contracts, and adds PowerShell/Python regression coverage wired into CI. ChangesScoped Repowise egress
Hospital fail-closed contract
Repository configuration resolution
Local-first control-plane documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![gemini-code-assist[bot]](https://avatars.github.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a fail-closed egress and trust contract for scoped Repowise document generation and hospital-mode state tracking. Key changes include the addition of egress manifest validation, path-traversal and reparse-point security checks, and updated state-machine guards to handle provider quota exhaustion and incomplete structural evidence. Review feedback highlights several critical runtime issues: PowerShell scripts should use the fully qualified [System.StringComparer] type to prevent TypeNotFound exceptions, the hospital state machine should route to the 'govern' protocol upon Sentrux gate failures, and the Python boundary requires path normalization for Windows compatibility and verification of the get_repo_structure signature to avoid a potential TypeError.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| [string]$WorkingTreePolicy | ||
| ) | ||
|
|
||
| $inventoryByPath = [System.Collections.Generic.Dictionary[string, object]]::new([StringComparer]::Ordinal) |
There was a problem hiding this comment.
In PowerShell, [StringComparer] is not a default type accelerator. Using it without the full namespace will throw a TypeNotFound exception at runtime. Use [System.StringComparer] instead.
$inventoryByPath = [System.Collections.Generic.Dictionary[string, object]]::new([System.StringComparer]::Ordinal)
| } | ||
|
|
||
| $orderedPaths = [string[]]@($inventoryByPath.Keys) | ||
| [Array]::Sort($orderedPaths, [StringComparer]::Ordinal) |
There was a problem hiding this comment.
In PowerShell, [StringComparer] is not a default type accelerator. Use [System.StringComparer] instead to avoid a runtime TypeNotFound exception.
[Array]::Sort($orderedPaths, [System.StringComparer]::Ordinal)
| if ($providerQuotaCount -gt 0) { return "triage" } | ||
| if ($null -ne $GitHubResearch -and [bool]$GitHubResearch.required) { return "github_solution_research" } | ||
| if ($FailureCounts.graphMissing -gt 0) { return "diagnose" } | ||
| if (-not $RulesExists) { return "govern" } |
There was a problem hiding this comment.
If a Sentrux check or gate fails ($FailureCounts.sentruxFail -gt 0), the next protocol should be "govern" to address the architectural violation. Currently, the function falls through to "surgery_plan" or "post_op", which bypasses the governance phase. Update the guard to route to "govern" when rules are missing or when there is an active gate failure.
if (-not $RulesExists -or $FailureCounts.sentruxFail -gt 0) { return "govern" }
| relative = entry.get("path") | ||
| expected_hash = entry.get("sha256") | ||
| if not isinstance(relative, str) or not isinstance(expected_hash, str): | ||
| raise RuntimeError(f"egress manifest {label} entry is incomplete") |
There was a problem hiding this comment.
To ensure robust cross-platform path handling, normalize any backslashes in relative to forward slashes. On Windows, paths in the manifest might contain backslashes, which PurePosixPath does not treat as directory separators, leading to single-segment paths containing literal backslashes.
relative = entry.get("path")
expected_hash = entry.get("sha256")
if not isinstance(relative, str) or not isinstance(expected_hash, str):
raise RuntimeError(f"egress manifest {label} entry is incomplete")
relative = relative.replace("\\", "/")| file_infos = list(traverser.traverse()) | ||
| repo_structure = traverser.get_repo_structure() | ||
| source_map = freeze_provider_payload(repo_path, egress_manifest, file_infos, manifest) | ||
| repo_structure = traverser.get_repo_structure(file_infos) |
There was a problem hiding this comment.
If the underlying repowise library's get_repo_structure method does not accept any arguments (as in standard versions), passing file_infos here will raise a TypeError at runtime. Since the test suite only mocks this method or skips calling it on the real class, this signature mismatch would not be caught during verification.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test-repo-config-resolution.ps1 (1)
13-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the repo-config helpers across scripts
Find-RepoConfigByPathand the repo resolver have different contracts inrun-code-intel.ps1andcheck-code-intel-tools.ps1(Resolve-RepovsResolve-RepoPath, and repos-only vs full-config input). Rename or normalize them so the shared names don’t imply interchangeable behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test-repo-config-resolution.ps1` around lines 13 - 18, Normalize the extracted helper contracts across run-code-intel.ps1 and check-code-intel-tools.ps1: align the resolver naming (Resolve-Repo versus Resolve-RepoPath) and make Find-RepoConfigByPath consistently accept the same repos-only or full-config input in both scripts. Update all corresponding callers and the functionNames extraction list in test-repo-config-resolution.ps1 so the shared names accurately represent interchangeable behavior.run-code-intel.ps1 (1)
1896-1919: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard reads for
localToolError/graphMissing/sentruxFailare not as defensive asproviderQuota.
providerQuotais read via an explicitIDictionary.Contains/PSObject.Propertiesguard (1899-1904), buttoolsOk/graphOk/sentruxOkstill use direct member access (e.g.[int]$FailureCounts.localToolError). If a caller ever supplies aFailureCountsobject missing one of those keys,[int]$nullcollapses to0, i.e. the guard silently passes (fail-open) for the one area this contract most wants fail-closed. All current callers populate every key, so this is defensive-only, but routing it throughGet-FailureCountwould keep the seam uniformly fail-closed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@run-code-intel.ps1` around lines 1896 - 1919, The guard logic around the state-machine checks should read localToolError, graphMissing, and sentruxFail defensively like providerQuota, failing closed when keys are absent. Reuse the existing Get-FailureCount helper for these three values, then base toolsOk, graphOk, and sentruxOk on its results while preserving the current zero-count behavior for present keys.Run-ScopedRepowiseDocs.py (1)
306-326: 🚀 Performance & Scalability | 🔵 TrivialPer-file HEAD-blob validation runs twice and spawns 2 git processes per file.
For the
head-tracked-onlypolicy,_validate_file_entriesshells out togit rev-parse HEAD:<path>andgit hash-objectfor every inventory entry, and this full validation runs once here (Line 316) and again insidefreeze_provider_payload(Line 292). For small scoped worktrees this is negligible, but if a scope ever spans many files the process-spawn count grows to roughly4 * N. If that becomes a hot path, consider batching with a singlegit ls-tree -r HEADplusgit hash-object --stdin-paths(orgit status --porcelain) to compare blobs in one pass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Run-ScopedRepowiseDocs.py` around lines 306 - 326, Remove the redundant egress-manifest validation from generate_docs after freeze_provider_payload already validates it, while preserving the manifest value needed by the function. Ensure head-tracked-only file validation occurs only once rather than being repeated by generate_docs and freeze_provider_payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@run-code-intel.ps1`:
- Around line 1896-1919: The guard logic around the state-machine checks should
read localToolError, graphMissing, and sentruxFail defensively like
providerQuota, failing closed when keys are absent. Reuse the existing
Get-FailureCount helper for these three values, then base toolsOk, graphOk, and
sentruxOk on its results while preserving the current zero-count behavior for
present keys.
In `@Run-ScopedRepowiseDocs.py`:
- Around line 306-326: Remove the redundant egress-manifest validation from
generate_docs after freeze_provider_payload already validates it, while
preserving the manifest value needed by the function. Ensure head-tracked-only
file validation occurs only once rather than being repeated by generate_docs and
freeze_provider_payload.
In `@test-repo-config-resolution.ps1`:
- Around line 13-18: Normalize the extracted helper contracts across
run-code-intel.ps1 and check-code-intel-tools.ps1: align the resolver naming
(Resolve-Repo versus Resolve-RepoPath) and make Find-RepoConfigByPath
consistently accept the same repos-only or full-config input in both scripts.
Update all corresponding callers and the functionNames extraction list in
test-repo-config-resolution.ps1 so the shared names accurately represent
interchangeable behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4654f86e-7daf-4092-90c4-90c6c020025d
📒 Files selected for processing (17)
.github/workflows/ci.ymlInvoke-ScopedRepowise.ps1Run-ScopedRepowiseDocs.pycheck-code-intel-tools.ps1docs/adr/0008-local-first-project-control-plane.mddocs/agents/issue-tracker.mddocs/artifact-data-contract.mddocs/hospital-mode.mddocs/project-management-support.mdpipeline.config.jsonrun-code-intel.ps1test-doctor-repo-config-resolution.ps1test-hospital-trust-contract.ps1test-repo-config-resolution.ps1test-scoped-repowise-security.ps1test-scoped-repowise-validator.pytest-scoped-repowise-worktree.ps1
1 participant
Summary
Verification
No real remote provider was invoked during tests.