Fix local-only picker silently skipping on first-time enroll (issue #281)#283
Conversation
) On a fresh `hyp remote login` enrollment the interactive local-only directory picker (LLP 0069/0080) showed nothing and skipped to its fallback tip. Root cause is ordering: `runBrowserLogin` ran the picker before `enrollCentralSink` installs the daemon and the daemon backfills `ai_gateway_messages`, so at picker time the cache was empty, `listCapturedDirectories` returned [] and the picker took its `no_candidates` branch. The picker only ever rendered on a re-login or a machine with pre-existing history, never on the fresh enroll that is its own primary trigger (LLP 0069 R6 / #trigger). Fix: defer the picker per fork to the point its cache is populated. - Fresh-enroll (auto-daemon) fork: run the picker after `waitForClientAttach`, gated on a new bounded, best-effort `waitForCapturedDirectories` poll. Attach markers land before the multi-minute backfill subprocess (LLP 0044 "ordering vs first ingest"), so waiting on attach alone is not enough; the poll waits for the cache the picker reads to populate. `executeQuerySql` rediscovers on-disk partitions at query time, so the login process sees the daemon's freshly backfilled rows cross-process. - `--no-daemon` fork: run the picker against the as-is cache; no forwarding daemon exists yet, so the list still lands before any export (R6 holds here). - Re-login / re-seed fork: run the picker against the cache a prior daemon already filled (editor semantics), unchanged. This narrows R6's "not forwarded, even once" to "withheld from every export tick after the pick" on the auto-daemon fresh-enroll path (a one-time backfill window may forward before the user picks). Closing that window fully is deferred to the issue #281 follow-up; the LLP 0069/0080 records carry a forward-ref note rather than a rewrite. Invariants preserved: TTY-gated, defaults to excluding nothing, never blocks or aborts enrollment, best-effort (a failed/empty enumeration never breaks the login it refines). Regression tests assert the new fresh-enroll ordering (picker runs after enroll+attach and sees the now-populated candidates) and the --no-daemon ordering; both fail on pre-fix code and pass after. Fixes #281 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…--no-daemon ordering note Two additive review findings on the #281 fresh-enroll picker reordering: - fresh-enroll auto-daemon fork: the bounded waitForCapturedDirectories poll only helps when a client attached this run (attach triggers the post-attach backfill, LLP 0044). With nothing attached there is no source to populate the cache, so the poll just burned its full 30s budget in silence before the picker skipped to the durable hint. Gate the wait on attached.length > 0 and otherwise run the picker directly against the cache as-is. Invariants hold: still bounded, still best-effort, still never blocks enrollment; the #281 regression test (attached > 0) is unchanged and passes. - LLP 0069/0080 forward-ref notes: corrected the imprecise claim that --no-daemon "keeps the pre-provision ordering". The --no-daemon fresh-enroll fork now runs the picker after enrollCentralSink writes the sink config, but installs no forwarding daemon this run, so the list still lands before any export and R6's substantive guarantee holds. Wording now matches the code and the test name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
neutral review round (review rung) - PR #283Reviewed head: The fix is correct and the load-bearing invariants all hold. The picker deferral is placed correctly in each of the three forks and the Invariant check (all hold)
Findings (2, both fixed)1. [major -> fixed] Dead 30s silent wait when no client attached / non-TTY - 2. [major -> fixed] LLP note said R6 trade-off verdict: acceptable and correctly documentedThe narrowing (auto-daemon fresh-enroll only: "not forwarded even once" -> "withheld from every export tick after the pick") is a real relaxation, but it is inherent to the reuse-the-cache design and is a net privacy improvement over the shipped bug: before this PR the fresh-enroll picker enumerated an empty cache and skipped silently, so a first-time user could exclude nothing and everything forwarded forever. After, they can exclude, at the cost of a bounded one-time backfill window. Closing that window fully would need a bigger change (hold forwarding until the pick, or a local pre-daemon backfill to populate candidates before any forwarding sink exists) - genuinely more than a cheap tweak. It is documented as forward-ref notes on LLP 0069 §trigger and 0080 (not a rewrite of settled decisions), with a follow-up tracked on issue #281. Correct call. Test qualityThe two regression tests genuinely pin the ordering: the fresh-enroll test asserts Anchors ( |
…re fork too Codex round-2 finding (a real regression the #281 picker reordering introduced): the pre-281 code ran the local-only picker once pre-provision, covering every enrolling-login fork including the daemon-install-failure early return. The reordering moved the picker to per-fork post-population points but left the `result.daemonCode !== 0` fork with no picker call, silently dropping the enrollment-time picker / durable hint there. Mirror the --no-daemon fork: run refineLocalOnly against the cache as-is before returning the daemon code. No forwarding daemon runs on a failed install, so the list still lands before any forwarding can start once the user finishes 'hyp daemon install' (R6 holds on this fork too). Tests: pin the picker runs on the daemon-fail path (fails without the fix), and pin the round-1 nothing-attached skip of waitForCaptured (asserts the bounded poll is not called and the picker still runs against the cache as-is). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
neutral PR-health review — round 2 (head
|
On the auto-daemon fresh-enroll path, the bounded waitForCapturedDirectories poll (up to CAPTURE_WAIT_DEFAULT_MS) ran whenever a client attached, even on a non-interactive login where the picker it feeds immediately no-ops on a non-TTY stream (LLP 0072 #tty). A scripted enroll (e.g. 'hyp remote login 2>file') could silently stall for a candidate list it would never show. Mirror the picker's exact TTY decision (ctx.stdin ?? process.stdin, ctx.stderr) before the wait: skip the poll when it cannot prompt and run the picker as-is so it takes its own durable-hint path immediately. Interactive logins are unchanged. Make the existing after-attach test an interactive (TTY) ctx and add a non-TTY regression test asserting the wait is skipped with clients attached. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review round 3 -
|
Review round 4 (final) — cleanHead reviewed: Round-3 TTY-gate fix: verified correct and completeThe gate at
The fresh-enroll auto-daemon fork then polls
All forks place the picker correctly; invariants hold
Invariants confirmed: TTY-gated, defaults to zero exclusions, never blocks or aborts enrollment ( Non-blocking observation (not a finding)The Nothing changed this round. Holding for human merge. |
#281 follow-up) (#286) After the #283 reordering the picker ran at the right time but queried through the wrong registry: hyp remote login boots its kernel at dispatch time, before enrollment, and on a first-run box the local config names no plugins. The login process's query-registry snapshot therefore never registers ai_gateway_messages - that registration arrives with @hypaware/ai-gateway, which the org config-control pull enables only after enrollCentralSink installs the daemon. listCapturedDirectories then failed with "unknown dataset" (best-effort null), waitForCapturedDirectories read null as "enumeration cannot run, stop now" (no 30s poll), and the picker took its enumeration_failed durable-hint path on exactly the fresh enroll it exists for. The #283 tests stubbed waitForCaptured/listCandidates, so the gap was never exercised; a re-login on a populated box worked, which is why manual verification passed. Fix: freshenCaptureEnumeration (src/core/cli/remote_commands.js). On the fresh-enroll interactive fork, after waitForClientAttach returns a client, re-boot one fresh kernel (config profile, the same layered resolution any subsequent hyp command runs) and hand its registry's enumeration to the capture wait. One re-boot is sufficient by construction: attach markers are written by the reconciler that runs only after a confirmed config apply, so attach means the pulled central layer is on disk - and the attach handler requires the gateway capability, so that layer enables the plugin. The snapshot-already-has-it case (re-login on a populated box) skips the boot entirely; a failed re-boot or a still-missing dataset falls back to the prior behavior. Best-effort throughout: the refinement never breaks the login it refines (LLP 0072). The handle is disposed (sources.stopAll, mirroring dispatch) after the picker so no boot-started source keeps the login process alive. local_only.js exports CAPTURE_DATASET so the registry check and the enumeration SQL cannot drift. LLP 0069/0080 carry forward-ref notes per the immutable-record convention. Tests: seam-level coverage of the refresh wiring (fresh enumeration is polled, disposed after the picker; null refresh falls back), unit coverage of freshenCaptureEnumeration (snapshot no-op, success, still-missing, boot-throw), and - the #283 lesson - one deliberately unstubbed test that runs the REAL bootKernel against an on-disk central layer naming @hypaware/ai-gateway and asserts enumeration RUNS (empty list, pollable) instead of failing to null. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…l (LLP 0093) (#287) * Fix local-only picker still skipping on a genuinely fresh enroll (issue #281 follow-up) After the #283 reordering the picker ran at the right time but queried through the wrong registry: hyp remote login boots its kernel at dispatch time, before enrollment, and on a first-run box the local config names no plugins. The login process's query-registry snapshot therefore never registers ai_gateway_messages - that registration arrives with @hypaware/ai-gateway, which the org config-control pull enables only after enrollCentralSink installs the daemon. listCapturedDirectories then failed with "unknown dataset" (best-effort null), waitForCapturedDirectories read null as "enumeration cannot run, stop now" (no 30s poll), and the picker took its enumeration_failed durable-hint path on exactly the fresh enroll it exists for. The #283 tests stubbed waitForCaptured/listCandidates, so the gap was never exercised; a re-login on a populated box worked, which is why manual verification passed. Fix: freshenCaptureEnumeration (src/core/cli/remote_commands.js). On the fresh-enroll interactive fork, after waitForClientAttach returns a client, re-boot one fresh kernel (config profile, the same layered resolution any subsequent hyp command runs) and hand its registry's enumeration to the capture wait. One re-boot is sufficient by construction: attach markers are written by the reconciler that runs only after a confirmed config apply, so attach means the pulled central layer is on disk - and the attach handler requires the gateway capability, so that layer enables the plugin. The snapshot-already-has-it case (re-login on a populated box) skips the boot entirely; a failed re-boot or a still-missing dataset falls back to the prior behavior. Best-effort throughout: the refinement never breaks the login it refines (LLP 0072). The handle is disposed (sources.stopAll, mirroring dispatch) after the picker so no boot-started source keeps the login process alive. local_only.js exports CAPTURE_DATASET so the registry check and the enumeration SQL cannot drift. LLP 0069/0080 carry forward-ref notes per the immutable-record convention. Tests: seam-level coverage of the refresh wiring (fresh enumeration is polled, disposed after the picker; null refresh falls back), unit coverage of freshenCaptureEnumeration (snapshot no-op, success, still-missing, boot-throw), and - the #283 lesson - one deliberately unstubbed test that runs the REAL bootKernel against an on-disk central layer naming @hypaware/ai-gateway and asserts enumeration RUNS (empty list, pollable) instead of failing to null. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Hold sink exports behind a bounded pick-pending marker on fresh enroll (LLP 0093) Closes the one-time forwarding window the issue #281 reordering left open: after PR #283 deferred the local-only picker past enrollCentralSink, the daemon that enroll installs could attach, backfill, and start exporting rows before the user finished the pick. Withholding is enforced at export-read time, and the server has no retraction path, so rows from a directory the user was about to withhold forwarded once, permanently - narrowing LLP 0069 R6's "not forwarded, even once" on exactly the fresh-enroll path the picker exists for. Decision (LLP 0093): the enrolling login writes a machine-local pick-pending marker (usage-policy/pick-pending.json, co-located with the local-only list) BEFORE enrollCentralSink, so no daemon tick can beat it onto disk; the kernel sink driver skips whole export ticks while the marker is fresh (held: 'pick_pending' in the tick report); the login clears the marker in one finally covering every exit from the fresh-enroll fork. Freshness is mtime with a 10-minute TTL and stale or unreadable markers read as absent, so a crashed or abandoned login can never stall exports indefinitely - the hold is a bounded refinement, never a kill switch (LLP 0072). Held rows stay in the cache and export on the first tick after the pick; watermarks make that a plain catch-up. Enforcement is driver-wide rather than per-sink: the driver cannot know which sinks leave the machine without a new registration concept, and briefly deferring a local sink is harmless where one missed forward hold is a permanent leak. Rejected alternatives (backfill-before-forward reordering, off-machine-only gating, unbounded hold) are recorded in the decision doc. Re-logins and --no-forward/query-only logins never write the marker. Tests: marker module (fresh/clear/stale-unlink/injected-now/fail-open), driver hold (fresh marker holds the whole tick, cleared or expired marker exports), and login lifecycle (marker lands before enroll, persists through the pick, cleared on return, on enroll error, and on non-TTY; never written on re-login). Stacked on the issue #281 follow-up fix (fresh-enroll registry refresh). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Problem
On a first-time
hyp remote loginenrollment, the interactivelocal-onlydirectory picker (LLP 0069/0080, #270) silently shows nothing and skips to its fallback tip. It only ever renders the multiselect on a re-login or a machine with pre-existing history, never on the fresh enroll that is its own primary trigger (LLP 0069 R6 /#trigger).Root cause (confirmed)
Ordering.
runBrowserLogin(src/core/cli/remote_commands.js) ranrunLocalOnlyPickerbeforeenrollCentralSinkinstalls the daemon and the daemon backfillsai_gateway_messages. At picker time the cache is empty,listCapturedDirectories→SELECT ... FROM ai_gateway_messagesreturns[], and the picker takes itsno_candidatesbranch, printing only the durable-command tip. The multiselect never renders and~/.hyp/hypaware/usage-policy/is never created.LLP 0044 ("ordering vs first ingest") +
action_attach.jsconfirm attach markers land before the multi-minute backfill subprocess, so waiting on attach alone is not enough.executeQuerySqlrediscovers on-disk partitions at query time, so the login process can see the daemon's freshly-backfilled rows cross-process.Fix
Defer the picker per fork to the point its cache is populated:
waitForClientAttach, gated on a new bounded, best-effortwaitForCapturedDirectoriespoll that waits for the backfilled cache to populate, then runs the picker against it.--no-daemon: run the picker against the as-is cache; no forwarding daemon exists yet, so the list still lands before any export (R6 holds here).Invariants preserved (LLP 0069/0072/0080): TTY-gated, defaults to excluding nothing, never blocks or aborts enrollment, best-effort.
R6 trade-off (flagged, not silently changed)
Deferring past
enrollCentralSinknarrows R6's "not forwarded, even once" to "withheld from every export tick after the pick" on the auto-daemon fresh-enroll path only (a one-time backfill window may forward before the user picks). Closing that window fully (hold forwarding until the pick, or backfill locally before the forward daemon starts) is deferred to a follow-up. LLP 0069 §trigger and LLP 0080 §login carry a forward-ref note pointing here rather than a rewrite of their settled decisions.Tests
Two regression tests assert the new ordering (fresh-enroll picker runs after enroll+attach and sees the now-populated candidates;
--no-daemonpicker runs after the sink is provisioned). Both fail on pre-fix code and pass after. Full suite green (2105 pass).🤖 Generated with Claude Code
Fixes #281