Repository navigation
test(agent-status): inventory legacy pane identity surfaces - #16575
Conversation
…nsumer PR 1 of the identity migration. It changes no displayed or routed identity — it only measures. Why measure first: the hierarchy shipped in #16148/#16157 has zero consumers, while ~31 sites still derive identity independently. Every migration decision after this is currently a guess, including the one that matters most — how often a real pane has no evidence at all. A live P0 reports "No Claude status shown", and this design trades toward showing nothing when uncertain, so the blank rate has to be a number before any surface moves. - `pane-agent-identity-evidence.ts` — one assembler that gathers a pane's evidence, so consumers stop each inventing their own ladder. - `pane-agent-identity-census.ts` — shadow-only counters keyed by host kind (native / wsl-host / wsl-distro / ssh / relay) and launch mode (typed / orca-launch / resume). Records a bitmask of which sources were present and whether the resolver returned null or ambiguous. No titles, prompts, paths, handles, or agent text. - `pane-agent-identity-inventory.test.ts` — a ratchet that fails when a legacy identity helper gains a new production caller, so the surface cannot grow while the migration runs. Three review findings are encoded rather than deferred: launch stays above run-key-less completed hooks (promoting the hook lets a stale record hijack a pane); OMP/Pi evidence is owner-normalized before assembly, since OMP emits Pi-compatible frames and a wrapper's hook would otherwise be read as the agent it wraps; and Windows-side `wsl.exe` is rejected as process evidence, because the host observes the distro wrapper rather than the agent inside it. The census cannot be completed from a worktree. It needs representative native, SSH, WSL and relay cohorts collected from real use, and that review is the gate on PR 3 — not this PR.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a Vitest inventory ratchet that scans pane-agent identity helpers and direct identity/action surfaces across Merge Risk: 🔵 Low · up to The PR adds inventory and regression coverage without changing runtime behavior. It is mergeable with owner awareness that the inventory could undercount legacy identity surfaces when markers appear in string literals or when resolver results are unavailable. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 375b80b4-4de4-4f4e-96a1-af695951e681
📒 Files selected for processing (7)
src/renderer/src/lib/pane-agent-evidence.tssrc/shared/pane-agent-identity-census.test.tssrc/shared/pane-agent-identity-census.tssrc/shared/pane-agent-identity-evidence.test.tssrc/shared/pane-agent-identity-evidence.tssrc/shared/pane-agent-identity-inventory.test.tssrc/shared/pane-agent-identity-resolver.test.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/lib/pane-agent-evidence.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| row.noEvidence += sources.length === 0 ? 1 : 0 | ||
| row.titleOnly += sources.length > 0 && sources.every((source) => source === 'title') ? 1 : 0 | ||
| row.noNonTitleEvidence += sources.every((source) => source === 'title') ? 1 : 0 | ||
| row.ambiguousTopRank += resolution.ambiguousAt === undefined ? 0 : 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Count non-ambiguous null resolutions. Line 90 records only ambiguity. With sibling-only evidence, the resolver returns agent: null, but this row records neither noEvidence nor ambiguity. Add a null-resolution counter based on resolution.agent === null.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 15d451e0-32ca-4df3-985c-c77194db9907
📒 Files selected for processing (3)
src/renderer/src/lib/pane-agent-evidence.tssrc/shared/pane-agent-identity-inventory.test.tssrc/shared/pane-agent-identity-resolver.test.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/lib/pane-agent-evidence.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/shared/pane-agent-identity-resolver.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
|
|
||
| it('pins direct single-source identity and action branches outside named helpers', () => { | ||
| for (const site of DIRECT_SINGLE_SOURCE_SURFACES) { | ||
| const source = stripComments(readFileSync(join(process.cwd(), site.path), 'utf8')) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Blank string literals before checking direct markers.
stripComments retains strings. A marker in a message or template literal can keep this ratchet passing after the direct branch is removed. Apply blankStringContents after comment removal, with the same desynchronization check used above.
Proposed fix
- const source = stripComments(readFileSync(join(process.cwd(), site.path), 'utf8'))
+ const decommentedSource = stripComments(readFileSync(join(process.cwd(), site.path), 'utf8'))
+ if (blankStringContentsDesynced(decommentedSource)) {
+ throw new Error(`String scanner desynchronized while inventorying ${site.path}`)
+ }
+ const source = blankStringContents(decommentedSource)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const source = stripComments(readFileSync(join(process.cwd(), site.path), 'utf8')) | |
| const decommentedSource = stripComments(readFileSync(join(process.cwd(), site.path), 'utf8')) | |
| if (blankStringContentsDesynced(decommentedSource)) { | |
| throw new Error(`String scanner desynchronized while inventorying ${site.path}`) | |
| } | |
| const source = blankStringContents(decommentedSource) |
|
Status: LAND Reviewed exact head
Residual follow-up: real host-authoritative usage measurement across native, WSL, SSH, and relay remains separate instrumentation work and is intentionally outside this PR. |
…#16575) * feat(agent-status): measure identity evidence before migrating any consumer PR 1 of the identity migration. It changes no displayed or routed identity — it only measures. Why measure first: the hierarchy shipped in stablyai#16148/stablyai#16157 has zero consumers, while ~31 sites still derive identity independently. Every migration decision after this is currently a guess, including the one that matters most — how often a real pane has no evidence at all. A live P0 reports "No Claude status shown", and this design trades toward showing nothing when uncertain, so the blank rate has to be a number before any surface moves. - `pane-agent-identity-evidence.ts` — one assembler that gathers a pane's evidence, so consumers stop each inventing their own ladder. - `pane-agent-identity-census.ts` — shadow-only counters keyed by host kind (native / wsl-host / wsl-distro / ssh / relay) and launch mode (typed / orca-launch / resume). Records a bitmask of which sources were present and whether the resolver returned null or ambiguous. No titles, prompts, paths, handles, or agent text. - `pane-agent-identity-inventory.test.ts` — a ratchet that fails when a legacy identity helper gains a new production caller, so the surface cannot grow while the migration runs. Three review findings are encoded rather than deferred: launch stays above run-key-less completed hooks (promoting the hook lets a stale record hijack a pane); OMP/Pi evidence is owner-normalized before assembly, since OMP emits Pi-compatible frames and a wrapper's hook would otherwise be read as the agent it wraps; and Windows-side `wsl.exe` is rejected as process evidence, because the host observes the distro wrapper rather than the agent inside it. The census cannot be completed from a worktree. It needs representative native, SSH, WSL and relay cohorts collected from real use, and that review is the gate on PR 3 — not this PR. * test(agent-status): keep identity migration inventory-only * test(agent-status): reuse reliable source scanner * test(agent-status): bound inventory scan work * test(agent-status): avoid inventory path false negatives * test(agent-status): refresh identity inventory after base repair * test(agent-status): correct inventory classifications * test(agent-status): correct action boundary inventory * test(agent-status): pin inventory occurrence counts * test(agent-status): fail closed on scanner desync
…#16575) * feat(agent-status): measure identity evidence before migrating any consumer PR 1 of the identity migration. It changes no displayed or routed identity — it only measures. Why measure first: the hierarchy shipped in stablyai#16148/stablyai#16157 has zero consumers, while ~31 sites still derive identity independently. Every migration decision after this is currently a guess, including the one that matters most — how often a real pane has no evidence at all. A live P0 reports "No Claude status shown", and this design trades toward showing nothing when uncertain, so the blank rate has to be a number before any surface moves. - `pane-agent-identity-evidence.ts` — one assembler that gathers a pane's evidence, so consumers stop each inventing their own ladder. - `pane-agent-identity-census.ts` — shadow-only counters keyed by host kind (native / wsl-host / wsl-distro / ssh / relay) and launch mode (typed / orca-launch / resume). Records a bitmask of which sources were present and whether the resolver returned null or ambiguous. No titles, prompts, paths, handles, or agent text. - `pane-agent-identity-inventory.test.ts` — a ratchet that fails when a legacy identity helper gains a new production caller, so the surface cannot grow while the migration runs. Three review findings are encoded rather than deferred: launch stays above run-key-less completed hooks (promoting the hook lets a stale record hijack a pane); OMP/Pi evidence is owner-normalized before assembly, since OMP emits Pi-compatible frames and a wrapper's hook would otherwise be read as the agent it wraps; and Windows-side `wsl.exe` is rejected as process evidence, because the host observes the distro wrapper rather than the agent inside it. The census cannot be completed from a worktree. It needs representative native, SSH, WSL and relay cohorts collected from real use, and that review is the gate on PR 3 — not this PR. * test(agent-status): keep identity migration inventory-only * test(agent-status): reuse reliable source scanner * test(agent-status): bound inventory scan work * test(agent-status): avoid inventory path false negatives * test(agent-status): refresh identity inventory after base repair * test(agent-status): correct inventory classifications * test(agent-status): correct action boundary inventory * test(agent-status): pin inventory occurrence counts * test(agent-status): fail closed on scanner desync
…#16575) * feat(agent-status): measure identity evidence before migrating any consumer PR 1 of the identity migration. It changes no displayed or routed identity — it only measures. Why measure first: the hierarchy shipped in stablyai#16148/stablyai#16157 has zero consumers, while ~31 sites still derive identity independently. Every migration decision after this is currently a guess, including the one that matters most — how often a real pane has no evidence at all. A live P0 reports "No Claude status shown", and this design trades toward showing nothing when uncertain, so the blank rate has to be a number before any surface moves. - `pane-agent-identity-evidence.ts` — one assembler that gathers a pane's evidence, so consumers stop each inventing their own ladder. - `pane-agent-identity-census.ts` — shadow-only counters keyed by host kind (native / wsl-host / wsl-distro / ssh / relay) and launch mode (typed / orca-launch / resume). Records a bitmask of which sources were present and whether the resolver returned null or ambiguous. No titles, prompts, paths, handles, or agent text. - `pane-agent-identity-inventory.test.ts` — a ratchet that fails when a legacy identity helper gains a new production caller, so the surface cannot grow while the migration runs. Three review findings are encoded rather than deferred: launch stays above run-key-less completed hooks (promoting the hook lets a stale record hijack a pane); OMP/Pi evidence is owner-normalized before assembly, since OMP emits Pi-compatible frames and a wrapper's hook would otherwise be read as the agent it wraps; and Windows-side `wsl.exe` is rejected as process evidence, because the host observes the distro wrapper rather than the agent inside it. The census cannot be completed from a worktree. It needs representative native, SSH, WSL and relay cohorts collected from real use, and that review is the gate on PR 3 — not this PR. * test(agent-status): keep identity migration inventory-only * test(agent-status): reuse reliable source scanner * test(agent-status): bound inventory scan work * test(agent-status): avoid inventory path false negatives * test(agent-status): refresh identity inventory after base repair * test(agent-status): correct inventory classifications * test(agent-status): correct action boundary inventory * test(agent-status): pin inventory occurrence counts * test(agent-status): fail closed on scanner desync (cherry picked from commit a624e7c)
…#16575) * feat(agent-status): measure identity evidence before migrating any consumer PR 1 of the identity migration. It changes no displayed or routed identity — it only measures. Why measure first: the hierarchy shipped in stablyai#16148/stablyai#16157 has zero consumers, while ~31 sites still derive identity independently. Every migration decision after this is currently a guess, including the one that matters most — how often a real pane has no evidence at all. A live P0 reports "No Claude status shown", and this design trades toward showing nothing when uncertain, so the blank rate has to be a number before any surface moves. - `pane-agent-identity-evidence.ts` — one assembler that gathers a pane's evidence, so consumers stop each inventing their own ladder. - `pane-agent-identity-census.ts` — shadow-only counters keyed by host kind (native / wsl-host / wsl-distro / ssh / relay) and launch mode (typed / orca-launch / resume). Records a bitmask of which sources were present and whether the resolver returned null or ambiguous. No titles, prompts, paths, handles, or agent text. - `pane-agent-identity-inventory.test.ts` — a ratchet that fails when a legacy identity helper gains a new production caller, so the surface cannot grow while the migration runs. Three review findings are encoded rather than deferred: launch stays above run-key-less completed hooks (promoting the hook lets a stale record hijack a pane); OMP/Pi evidence is owner-normalized before assembly, since OMP emits Pi-compatible frames and a wrapper's hook would otherwise be read as the agent it wraps; and Windows-side `wsl.exe` is rejected as process evidence, because the host observes the distro wrapper rather than the agent inside it. The census cannot be completed from a worktree. It needs representative native, SSH, WSL and relay cohorts collected from real use, and that review is the gate on PR 3 — not this PR. * test(agent-status): keep identity migration inventory-only * test(agent-status): reuse reliable source scanner * test(agent-status): bound inventory scan work * test(agent-status): avoid inventory path false negatives * test(agent-status): refresh identity inventory after base repair * test(agent-status): correct inventory classifications * test(agent-status): correct action boundary inventory * test(agent-status): pin inventory occurrence counts * test(agent-status): fail closed on scanner desync
ELI5
Orca currently has many places that decide which agent owns a terminal pane. Before migrating those decisions, this PR records every production occurrence of nine named legacy identity helpers plus a curated set of known direct branches. The test ratchet prevents those named-helper occurrences from changing unnoticed and keeps the recorded direct sites visible for review.
This PR does not change any identity decision and does not collect real-use measurements.
What changed
Scope and non-goals
Linked issue
Inventory foundation for #9236. This PR does not resolve that issue or migrate any consumer.
Testing
Visual proof
N/A — test-only inventory and regression coverage; no rendered behavior changes.
Notes