fix(review): name the check that refused an intended-untracked selection

[L4XB]

Refs #941. This does not fix the rejection the reporter hit — it makes the rejection say which check failed, which is what the report is actually stuck on. Details and evidence below, including what I could rule out.

What #941 is stuck on

select-intended-untracked decides eight conditions in one boolean and answers every one of them with the same string:

const rejected = input === undefined || canonicalReviewCaptureBinding(input) !== canonicalBinding
  || exactCollectArgument(input, "target_identity") !== status.targetIdentity
  || exactCollectArgument(input, "projection") !== status.projection.projection
  || exactCollectArgument(input, "base_tree") !== status.projection.baseTree
  || exactCollectArgument(input, "candidate_tree") !== status.projection.currentCandidateTree
  || !Array.isArray(eligible) || selected.reason !== undefined
  || selected.intendedUntracked!.some((path) => !eligible.includes(path));
if (rejected) return { …, outcome: "intended-untracked-selection-binding-rejected", … };

A caller holding provider-issued bytes cannot tell a status that moved under it (retry after a fresh inspect) from a path outside the eligible list (fix the selection) from an unreadable eligible_paths_json (a provider problem). The report shows the consequence: five attempts of the same call with machine-verified bytes, and no forward route.

After this change each condition names itself, in the same order, with the same outcome kept for compatibility:

reason	fired by	field
selection-input-absent	the status carries no matching collect input	—
binding-mismatch	the submitted bytes are not this input's canonical binding	—
status-field-mismatch	the live status no longer matches the bound value	the field that moved
eligible-paths-unreadable	eligible_paths_json absent, duplicated, or not JSON	—
untracked-selection-invalid	validateNativeStartUntrackedSelection refused the pair	—
path-not-eligible	a selected path is not in the eligible list	the refused path

nativeStartRejection has carried reason/field for START since it was introduced, so this is the shape the file already uses.

On the one judgement call worth a reviewer's eye: path-not-eligible echoes back a path the caller itself just submitted, in the response to that same call — no information the caller did not already have, and with a 400-entry inventory it is the only way to tell which entry was refused. Dropping the field there is a one-line change if you would rather it never appear.

What I could rule out

The empty-selection path itself is sound. I reproduced the reporter's shape in the existing harness — candidate touching only tracked files, an unrelated untracked inventory (including a path with spaces), intendedUntracked: [] — and it starts, taking the exclude scope with intended_untracked: []:

starts[0].intendedUntrackedSelection = { argumentTokens: […],
  value: '{"schema":"gentle-ai.review-intended-untracked-selection/v1","untracked_scope":"exclude","expected_untracked_inventory":"sha256:aaa…","intended_untracked":[]}' }

That is now a test, so the case cannot regress. It also means the rejection in #941 comes from one of the identity/binding/eligible-list terms against the native provider's payload, which I cannot reproduce from here — hence the reason field rather than a guess at a fix. Once this lands, @jmasenjowork-glitch's next attempt reports which one, and that is a one-line answer instead of a five-attempt investigation.

Things I checked and found innocent: validateNativeStartUntrackedSelection accepts {scope: exclude, inventory, intendedUntracked: []} (extensions/gentle-ai.ts:5009 only rejects a non-empty selection under exclude); exactCollectArgument does no tokenizing, so a path containing spaces is not truncated; isCanonicalProcessString imposes no length bound, so a 426-entry eligible_paths_json is not rejected for size anywhere in the TypeScript layer.

Verification

• node --experimental-strip-types --test tests/review-controller-native-routing.test.ts → 68 pass, 0 fail (was 65; three new cases).
• Negative control — reverting only extensions/gentle-ai.ts and keeping the tests → 4 fail: both existing pre-lineage intended-untracked selection … cases (the per-case reason assertions) and both moved-status cases. The empty-selection test passes in both states, which is what makes it a regression guard rather than a restatement of the change.
• node scripts/check-types.mjs → 203 recorded diagnostic(s), no regressions. The gate also reports one pre-existing improvement in tests/review-candidate-view.test.ts TS2540, which is already true on a clean origin/main (verified by stashing), so scripts/types-baseline.json is deliberately left untouched here.

Summary by CodeRabbit

• Bug Fixes

  • Improved validation for intended untracked-file selections, preventing selections from proceeding when their status, bindings, or eligible paths no longer match.
  • Rejected selections now include a specific reason and, when applicable, the affected field.
  • Added safeguards to prevent work from starting when live status data has changed.

• Improvements

  • Empty selections are now accepted when no untracked files are involved, allowing the operation to proceed with an exclusion submission.

[coderabbitai]

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: 7df5235e-d258-48dd-8835-37a1874bf3e8

📥 Commits

Reviewing files that changed from the base of the PR and between 6e4478c and c49f78d.

📒 Files selected for processing (2)

• extensions/gentle-ai.ts
• tests/review-controller-native-routing.test.ts

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

---

📝 Walkthrough

Walkthrough

The change centralizes intended-untracked selection validation, reports structured rejection reasons and fields, and adds tests for status drift, invalid selections, and valid empty selections.

Changes

Intended-untracked selection validation

Layer / File(s)	Summary
Structured selection validation and routing extensions/gentle-ai.ts	The new validator checks selection bindings and returns detailed rejection reasons with an optional failing field. SELECT_INTENDED_UNTRACKED exposes these details in blocked responses.
Selection validation regression coverage tests/review-controller-native-routing.test.ts	Tests assert rejection reasons, detect drift in target_identity and candidate_tree, confirm rejected selections do not start, and verify that valid empty selections start with an EXCLUDE submission.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ReviewController
  participant SelectionValidator
  participant ReviewStart
  ReviewController->>SelectionValidator: validate intended-untracked selection
  SelectionValidator-->>ReviewController: return reason and optional field
  ReviewController->>ReviewStart: start when validation succeeds

Loading

Suggested reviewers: alan-thegentleman

Merge Risk: ⚪ Minimal · up to c49f7

The structured rejection reporting preserves rejection behavior and adds coverage for the new reason, field, status-drift, and empty-selection cases.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: reporting which validation check rejected an intended-untracked selection.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[LaurenteEber]

There is a concrete committed-range variant of this rejection that can inform the follow-up fix. This is not evidence that #941 has the same cause, and it does not require expanding this diagnostic-only PR.

Observed on published gentle-pi 2.5.0 (macOS)

The tracked worktree was clean, with an existing committed candidate and an unchanged unrelated untracked inventory. An ordinary START with an explicit base commit and committedOnly: true returned the expected non-empty base-diff and an intended-untracked selection binding. Immediately submitting that exact returned binding with intendedUntracked: [] and the same explicit worktree returned intended-untracked-selection-binding-rejected, with mutation_performed: false and mutation_outcome: none. The default workspace inspection instead saw an empty tracked diff. No code changes, authority reset/recovery, or binding rewriting were used between those calls.

Installed-source diagnosis

In extensions/gentle-ai.ts from 2.5.0:

• START resolves the base to a canonical commit and passes baseRef plus committedOnly: true to the initial status negotiation (approximately lines 6015–6044).
• SELECT_INTENDED_UNTRACKED negotiates status again with cwd and optional signal only (5978–5981). It does not retain the committed-range selectors from the START that emitted the binding.
• The exact-binding/identity checks then compare the original range binding against the default workspace status (5984–5988). The aggregate rejection does not identify which individual condition fired, but the selector omission is directly visible in the installed source.
• There is a second loss at the recursive START (5990–5991): its input carries only ordinary mode and the untracked selection fields. It also drops baseRef, committedOnly, policyPath, and focus. Fixing only the status recheck would therefore leave the continuation incomplete. Policy/focus loss is a static finding, not the cause demonstrated by this runtime occurrence.

Suggested follow-up boundary

Keep the rejection checks. Retain the validated pre-lineage route in the host, bound to the exact provider-issued binding, session, and canonical worktree. Preserve the already-resolved base commit, committed-only semantics, and original policy/focus through both the fresh status check and the subsequent START. Do not make the caller reconstruct selectors or relax provider binding validation. Unknown/stale routes and conflicting contexts for the same binding should fail closed.

The regression worth adding is the composition: clean tracked workspace + committed-range START + unrelated untracked inventory + selection [], asserting that both the recheck and final START retain the canonical base and committed-only flag. Existing workspace-selection tests around tests/review-controller-native-routing.test.ts:913–951 do not exercise that combination. Also preserve negative controls for inventory/candidate drift, conflicting roots, and replay, and test policy/focus transport separately.

Evidence boundary: the facade rejection was observed in the live Pi host; the causal path was then checked statically in the installed package. No new regression test, patched build, or end-to-end fix verification has been executed. This comment was prepared with AI assistance and is posted with the user's authorization; private repository details and candidate identifiers are omitted.

[L4XB]

Thanks — this is the kind of report that makes a diagnostic worth having, and both source claims hold on current main, not only in the published 2.5.0:

• extensions/gentle-ai.ts:7356 — the SELECT re-negotiation passes { cwd: defaultCwd, ...signal } and nothing else. The baseRef / committedOnly selectors the START used to issue the binding are not carried.
• extensions/gentle-ai.ts:7368 — the recursive START's input is { mode: ORDINARY, untrackedScope, expectedUntrackedInventory, intendedUntracked }. baseRef, committedOnly, policyPath and focus are all dropped, so fixing only the status recheck would leave the continuation reading a different range than the one the caller asked for.

I agree this does not belong in this PR: it is a behaviour change, and this one only names the check that already refused.

What it does do for your case is turn the aggregate into a discriminator. The order in intendedUntrackedSelectionRejection is deliberate, so the reason you get back tells you which of two things happened:

• selection-input-absent — the default-workspace status emits no intended-untracked collect input at all. That is the reading where the empty tracked diff means there was nothing to select against, and the binding never got as far as being compared.
• status-field-mismatch with field: "base_tree" (or candidate_tree) — the input is there, and the named field is the one that moved. That is the reading where the committed-range binding is being compared against a default-workspace projection, which is the selector loss at 7356 stated in one word.

binding-mismatch would mean something else again: the input is present and its own capture binding differs, which points at the binding rather than at the range.

I cannot reproduce your exact state here (it needs the committed candidate and the untracked inventory you had), so I am not claiming which of the two fires for you. But re-running the same two calls against this branch answers it in one line, and whichever it is narrows the follow-up: selection-input-absent says the fix has to reach the status emission, status-field-mismatch says carrying the selectors at 7356 is enough for the rejection and 7368 is the separate half.

On the boundary you suggest: keeping the rejection checks and retaining the validated pre-lineage route bound to the exact provider-issued binding sounds right to me, and the policy/focus loss at 7368 is worth its own line in that follow-up even though, as you say, this occurrence does not demonstrate it.