Skip to content

fix(review): recover capture routes from fresh native status - #1575

Merged
Alan-TheGentleman merged 2 commits into
Gentleman-Programming:mainfrom
decode2:fix/1316-native-status-capture
Sep 30, 2026
Merged

Alan-TheGentleman merged 2 commits into
Gentleman-Programming:mainfrom
decode2:fix/1316-native-status-capture

Conversation

@decode2

@decode2 decode2 commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Linked issue

Closes #1316

PR type

  • Bug fix
  • New feature
  • Documentation only
  • Code refactoring
  • Maintenance/tooling
  • Breaking change

Summary

  • Revalidate single and complete ordered group captures against fresh Pi-bound native STATUS when volatile facade route metadata is missing.
  • Preserve trusted committed/untracked selectors, exact current-target admission, forecast acknowledgement and downstream relay/reconciliation routing.
  • Align published collect bindings with registration eligibility; add regression and negative-control coverage without route persistence, retries, flags or native states.

Review path

Review extensions/gentle-ai.ts first, then the missing-route/forecast and stale/group controls in the two routing test files. Draft #1542 adds diagnostics only; this PR implements the route-recovery correction without importing that telemetry. It does not claim to establish which registry miss caused every production occurrence.

Changes

File Change
extensions/gentle-ai.ts Fresh native admission, trusted selector fallback, validated route propagation and consistent binding publication.
tests/review-controller-native-routing.test.ts Committed recovery, eviction/session negatives, and exact stale-binding STATUS request/zero-capture assertions.
tests/review-host-relay-routing.test.ts Forecast-to-capture route loss, complete group recovery, selector and admission controls.
odd/tasks/issue-1316-native-status-capture.md Scope, TDD evidence, verification boundaries and observed native-review outcome.

Test plan

  • Strict TDD: initial recovery regressions failed on unchanged production; the same cases passed after the fix. The stale STATUS-count assertion was separately observed RED and corrected with exact request context and zero captures.
  • All three affected routing files pass 135/135, with no failures, cancellations or skips, both after updating to main and in an independent committed-unit recheck.
  • pnpm run typecheck exits 0 against 187 accepted baseline diagnostics, with no regressions (not a diagnostic-free TypeScript build).
  • pnpm run check:runtime-modules: eight generated modules match; git diff --check passes.
  • Code work unit 7b0e7ab9 completed native consolidated review-reliability review as approved; its exact acknowledgement completed and burned authority. The following evidence-only documentation commit changes no executable behavior.
  • A genuine Pi SDK loader/AgentSession exercised registered-tool recovery and negative cases with fixture STATUS. This proves runtime wiring, not native-provider E2E reproduction of the defect.
  • Global full-suite green: the original candidate timed out with 18 shared failures plus one candidate-only stale-count assertion. The 18 failures, cancellation/timeout and standalone empty-persona harness failure reproduced with matching signatures on untouched original main. The candidate-only assertion is corrected and independently passing. Later full-suite phases and global health on the updated base are not claimed; CI remains the publication gate.

Contributor checklist

  • Linked an open approved issue; exactly one PR type selected (type:bug).
  • Conventional commits; no AI attribution or Co-Authored-By trailers.
  • Tests and task evidence accompany the behavior change; no unrelated cleanup.
  • Shell scripts unchanged: shellcheck N/A. Skills unchanged: skill-loading validation N/A.
  • Dedicated isolated feature worktree; no main-current mutation.

Rollback: revert the fix and its passive evidence update; no other authority, state or worktree needs resetting. No merge or auto-merge has been authorized.

Summary by CodeRabbit

  • Bug Fixes
    • Single and grouped captures can proceed when a route is missing or only partially registered, provided registration is not required.
    • Capture requests use a trusted committed-range selector when available, and validated routes are registered for downstream processing.
    • Captures are rejected when route identity or grouped bindings conflict with the current workspace, lineage, or base reference, helping prevent invalid relay launches.

@decode2 decode2 added the type:bug Bug fix label Sep 30, 2026
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e11d9fdc-e26c-4bee-9ca1-6b5e3458382a

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6b148 and 338ad69.

📒 Files selected for processing (4)
  • extensions/gentle-ai.ts
  • odd/tasks/issue-1316-native-status-capture.md
  • tests/review-controller-native-routing.test.ts
  • tests/review-host-relay-routing.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Native capture routing now applies stricter eligibility checks and can recover single or grouped capture routes from fresh STATUS results. The changes also add routing tests and an issue work record.

Changes

Native capture routing

Layer / File(s) Summary
Capture eligibility
extensions/gentle-ai.ts
Route retention, public binding offers, and exact capture selection now require current-target applicability, canonical lineage and target identity, and nonterminal authority.
Single and grouped capture recovery
extensions/gentle-ai.ts, tests/review-controller-native-routing.test.ts, tests/review-host-relay-routing.test.ts, odd/tasks/issue-1316-native-status-capture.md
Single and grouped captures use trusted committed-range selectors in fresh STATUS requests and register routes after validation. Tests cover route recovery and rejection cases. The issue work record describes the scope, execution status, and remaining publication steps.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant CaptureClient
  participant NativeCaptureRouting
  participant NativeSTATUS
  participant HostRelay
  CaptureClient->>NativeCaptureRouting: Submit single or grouped capture
  NativeCaptureRouting->>NativeSTATUS: Request fresh STATUS with trusted selector
  NativeSTATUS-->>NativeCaptureRouting: Return current capture binding or group
  NativeCaptureRouting->>NativeCaptureRouting: Validate capture and register route
  NativeCaptureRouting->>HostRelay: Relay validated capture
Loading

Suggested reviewers: alan-thegentleman

Merge Risk: ⚪ Minimal · up to 338ad

The change recovers missing capture routes while preserving fresh-status validation and committed selectors. No concrete merge-blocking risk is identified; merge remains subject to normal CI checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 338ad

Recovery retains exact current-target validation and explicit acknowledgement before reviewer execution. No introduced security defect was established, but interrupted and repeated captures have not been verified end to end.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is existing reviewer execution and review-authority mutation for the selected workspace, lineage and target. Missing-route recovery makes those existing sinks reachable without a cached route, but only after provider-status admission. The evidence does not establish a broader tenant, credential or deployment exposure, nor prove native-side isolation across concurrent sessions.

Trust Boundaries and Controls

  • observed — Caller-supplied bindings do not directly authorize capture. Fresh STATUS must offer exactly one matching collect input with the requested lineage and provider target; targeted validation may use only its provider-issued correction target. Reviewer execution still requires explicit acknowledgement, and correction-line values remain constrained by provider-issued bounds.
  • observed — Committed selector recovery prefers a retained route; only when no route exists may it use a committed candidate projection matching workspace and lineage. Partial-group recovery can use a surviving later route, while present workspace, lineage or selector conflicts remain rejection conditions.

Resilience and Maintainability Implications

  • observed — An unknown capture outcome triggers status reconciliation, not automatic capture replay. The recovered routing snapshot carries its selector into that path. This bounds automatic repetition within one invocation, but does not establish native idempotency for later invocations or concurrent sessions.

Hardening Proposals

  • proposed — Validate the recovery authority contract against the actual native provider under timeout, interruption, repeated invocation and concurrent sessions, including selectorless recovery after session cleanup. Confirm exact-target admission and stale-mutation rejection rather than adding automatic retries. The current fake-provider regressions do not establish these guarantees.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: recovering capture routes by using fresh native STATUS data.
Linked Issues check ✅ Passed The PR addresses the coding requirements in #1316. extensions/gentle-ai.ts recovers single and grouped captures when volatile route registration is absent, requests fresh Pi-bound STATUS with truste…
Out of Scope Changes check ✅ Passed The changed implementation, routing tests, and odd/tasks/issue-1316-native-status-capture.md work record all support the #1316 capture-recovery objective. The test changes validate the new STATUS re…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@decode2
decode2 marked this pull request as ready for review September 30, 2026 05:48
@Alan-TheGentleman
Alan-TheGentleman merged commit ecf23cf into Gentleman-Programming:main Sep 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native reviewer collectBinding rejected during an active review

2 participants