Skip to content

Make Claude Desktop attachable on purpose: reachable picker row, consent gate, and an entrypoint-owned backfill - #422

Merged
philcunliffe merged 9 commits into
masterfrom
claude-desktop-consent-and-entrypoint-gate
Jul 30, 2026
Merged

Make Claude Desktop attachable on purpose: reachable picker row, consent gate, and an entrypoint-owned backfill#422
philcunliffe merged 9 commits into
masterfrom
claude-desktop-consent-and-entrypoint-gate

Conversation

@bgmcmullen

@bgmcmullenbgmcmullen commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude Desktop could not be attached through any supported route, and
meanwhile its conversations were being imported by a path nobody had opted
into. Two decisions, one per commit.

1. The picker row was inert (LLP 0139)

claude-desktop shipped needs_setup + configure_command but no
compose block
- the only one of the eight bundled rows without one.
composePickerConfig skips descriptors that have none, so ticking Claude
Desktop in hyp init wrote a config with none of Desktop's plugins:

picked ["claude-desktop"] -> plugins: local-fs, format-parquet # before

The configure phase then ran claude-desktop install against a config the
command wasn't in (exit 2), and drop-on-failure printed a catch-up hint
that failed identically forever. /Applications/Claude.app satisfies the
detect probe, so the row arrived pre-checked for anyone with Desktop
installed.

Now a needs_setup row composes every plugin its configure_command
needs. PluginPickerCompose gains plugins[]; the Desktop row composes
@hypaware/claude-account beside its adapter, which requires the
credential capability only that plugin provides. Half a dependency set is
worse than none - the adapter fails requireCapability, so its commands
never register and the dispatcher says unknown command instead of naming
the gap. A test asserts the rule across every bundled row.

Consent moves to the point of action, defaulting to no. Desktop is the
one client that cannot present its own credential through a third-party
endpoint (LLP 0116), so attaching it makes this machine hold one. The
prompt says that and names every file it will touch. It lives in the
command, so the wizard inherits it through the existing ctx.commands.run
seam - one implementation, both surfaces - and a decline lands on
drop-on-failure.

Two adjacent fixes found while testing:

  • --print-commands had side effects. It honored the flag only for the
    plist and restart, still running the credential login and helper write
    for real. On a machine that wasn't signed in, the one flag meant to avoid
    unattended side effects dropped into an interactive OAuth flow and hung.
  • client_attach_missing printed a repair that errors. It offered
    hyp attach --client claude-desktop, which answers unknown client -
    the plugin registers no runtime adapter by design. The repair now comes
    from the client's own configure_command.

2. Backfill imported Desktop history with no opt-in (LLP 0140)

Desktop writes its sessions into ~/.claude/projects, the tree the
@hypaware/claude backfill scans, tagged entrypoint: "claude-desktop".
The provider imported everything it found there, filtered only by time
window and usage policy - neither of which knows about clients. So Desktop
history entered the cache regardless of whether Desktop was ever
configured, and landed under client_name: "claude".

The consent gate above does not cover this. It guards the credential and
the plist, not the door history actually arrives through.

A client now declares contributes.client.transcript_entrypoints. The
runner resolves the value-to-owner map and passes it on
BackfillRunContext - the runner rather than the provider, because the
answer needs the full catalog (the claiming plugin is typically not
active, which is exactly the case that closes the gate) plus the effective
plugin list, neither reachable from the activation context. In manifests
rather than a core table because three hardcoded lists here have already
drifted from the manifests they shadow, and this one would drift toward
over-capture.

Owned + unconfigured -> skipped before projection. Owned + configured ->
imported and attributed to the owner, so Desktop rows become queryable
as claude-desktop. One map, both uses.

Unknown entrypoints fail open (imported, attributed to the scanner). A
strict allowlist would silently drop real history the first time a client
ships a new value, and under-importing without saying so is worse than
filing a row under a slightly wrong client. Only a claimed-but-
unconfigured value closes the gate, so it strengthens as clients declare
rather than depending on a list being exhaustive.

Verification

Measured across 390 real transcripts: every file carries an entrypoint and
exactly one distinct value, so it's a clean per-session discriminator.

Dry run on that corpus:

ConfigImportedGated
@hypaware/claude only132 (both Desktop)
+ @hypaware/claude-desktop150
entrypoint_not_configured entrypoint=claude-desktop owner_client=claude-desktop
scan_complete files_seen=390 sessions_projected=13 sessions_gated=2
  • 2773 pass / 0 fail (39 new tests), typecheck clean
  • Smokes green: core_boot_noop, cli_bundled_plugins_activated,
    status_diagnostics, walkthrough_picker_to_first_query,
    gateway_claude_capture, hypignore_capture_drop

An earlier iteration logged entrypoint_unclaimed 388 times because only
Desktop's entrypoints were declared, leaving the gate inert for the common
case. Fixed by claiming cli/sdk-cli on the claude manifest, and
unclaimed values are now aggregated once into scan_complete rather than
logged per session.

Not addressed

  • Whether Desktop honors the managed profile is still unverified. This
    PR makes the attach reachable; it does not prove capture works. No
    plist was installed during development. LLP 0133 leaves the in-app check
    manual by design, and Desktop's agent sessions run in a VM where the host
    loopback may not be reachable at all.
  • Possible double capture if Desktop live capture does work: the same
    conversation could arrive live as claude-desktop-3p and by transcript as
    claude-desktop. For Claude Code these dedupe because identity comes from
    the transcript either way; unverified for Desktop. Noted in LLP 0140.
  • init.js's --source enum still omits claude-desktop, so the
    non-interactive path cannot select it (a fourth instance of the same
    drift).
  • The backfill consent prompt names providers (claude) without mentioning
    that Desktop content can ride along.
  • Rows imported before this change keep client_name: "claude"; nothing
    rewrites them.

🤖 Generated with Claude Code

bgmcmullenand others added 2 commits July 27, 2026 22:12
The `claude-desktop` picker row shipped `needs_setup` and a
`configure_command` but no `compose` block, the only one of the eight
bundled rows without one. `composePickerConfig` skips descriptors that
have none, so ticking Claude Desktop in `hyp init` wrote a config with
none of Desktop's plugins. The configure phase then ran
`claude-desktop install` against a config the command was not in, exited
2, and drop-on-failure printed a catch-up hint that failed identically
forever. `/Applications/Claude.app` satisfies the row's detect probe, so
the row arrived pre-checked for anyone with Desktop installed.
A row now composes every plugin its `configure_command` needs, not just
its own adapter: `PluginPickerCompose` gains `plugins[]`, and the Desktop
row composes `@hypaware/claude-account` beside `@hypaware/claude-desktop`
because the latter requires the credential capability only the former
provides. Composing half a dependency set is worse than none, since the
adapter then fails `requireCapability` and the dispatcher reports
`unknown command` instead of the missing capability. A test asserts the
rule across every bundled row so no future row can ship inert this way.
The credential opt-in moves from config-file friction to an explicit
consent prompt, defaulting to no. Desktop is the one client that cannot
present its own credential through a third-party endpoint (LLP 0116), so
attaching it makes the machine hold one; the prompt says that and names
every file it will touch. It lives in the command, so the wizard inherits
it through the existing `ctx.commands.run` seam with no second
implementation, and a decline lands on drop-on-failure.
Two adjacent fixes found while testing:
- `--print-commands` ran the credential login and helper write for real,
honoring the flag only for the plist and restart. On a machine that was
not signed in, the one flag meant to avoid unattended side effects
dropped into an interactive OAuth flow and hung. All five steps now
print.
- `client_attach_missing` offered `hyp attach --client claude-desktop`,
which answers `unknown client`: the plugin registers no runtime adapter
by design. The repair now comes from the client's own picker row
`configure_command`.
Not addressed: whether Desktop honors the managed profile once installed
is still unverified, and LLP 0133 leaves that in-app check manual.
Design: LLP 0139.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude Desktop writes its sessions into `~/.claude/projects`, the tree the
`@hypaware/claude` backfill scans, tagged `entrypoint: "claude-desktop"`.
The provider imported every session it found there, filtered only by time
window and usage policy, neither of which knows about clients. So Desktop
history entered the cache whether or not `@hypaware/claude-desktop` was
ever configured, attached, or consented to, and landed under
`client_name: "claude"` because the provider hardcoded its own name.
The consent gate added in LLP 0139 does not cover this: it guards the
credential and the plist, not the door history actually arrives through.
A client now declares the entrypoint values it owns via
`contributes.client.transcript_entrypoints`. The `hyp backfill` runner
resolves the value-to-owner map and passes it on `BackfillRunContext`; the
runner does this rather than the provider because the answer needs the full
catalog (the claiming plugin is typically *not* active, which is exactly
the case that closes the gate) plus the effective plugin list, neither
reachable from the activation context. Declared in manifests rather than a
core table because three hardcoded lists in this area have already drifted
from the manifests they shadow, and this one would drift toward
over-capture.
A session owned by an unconfigured client is skipped before projection,
beside the usage-policy drop and for the same reason. When the owner is
configured the session imports and is attributed to the owner, so Desktop
rows become queryable as `claude-desktop`. One map, both uses.
Unknown entrypoints fail open: imported and attributed to the scanning
client. A strict allowlist would silently drop real history the first time
a client ships a new value, and under-importing without saying so is worse
than filing a row under a slightly wrong client. Only a claimed-but-
unconfigured entrypoint closes the gate.
Measured across 390 real transcripts: every file carries an entrypoint and
exactly one distinct value, so it is a clean per-session discriminator.
Verified by dry run on that corpus - 13 sessions imported with 2 gated
when only `@hypaware/claude` is configured, 15 with 0 gated once Desktop
is added.
`@hypaware/claude` claims `cli` and `sdk-cli`. Without that every ordinary
session took the fail-open path, leaving the gate inert for the common case
and logging an unclaimed value per session; unclaimed values are now
counted per distinct value and reported once in `scan_complete`.
Design: LLP 0140, plus a scope correction to LLP 0139.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bgmcmullen
bgmcmullen marked this pull request as draft July 28, 2026 05:16
@bgmcmullenbgmcmullen added the neutral:review Delegate this PR to neutral for a review pass (approve or request changes; never merges) label Jul 28, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 1 - eacbe94

Verdict: findings. No path was found that captures or exports Desktop content without an opt-in this PR already requires, and nothing here regresses master. The findings are gaps between what the gate is documented to do and what it does.

The consent text itself is the best part of this PR.buildConsentExplanation (consent.js:38-74) states the posture that justifies the gate, enumerates every durable side effect by absolute path, says sudo will be asked for, and names the undo. A user reading only that block can predict every file that changes, and the tests pin the wording.

1. The Desktop gate is always closed during hyp init, so onboarding can never import Desktop history even for a user who ticked the row and consented

resolveOwnersForRun derives "configured" from ctx.plugins, the plugins activated in this process (commands/backfill.js:749). But hyp init boots the all-available profile (cli/dispatch.js:484), whose filter requires allowlist membership, and @hypaware/claude-desktop is in V1_EXCLUDED_FROM_DEFAULT (runtime/bundled.js:82) precisely because it needs the credential capability. The config the picker writes (wizard/pick.js:216) cannot change activePlugins, which were fixed at process start, and the init finale's backfill runs on that same ctx (commands/init.js:34-36).

Verified by booting both profiles against the composed config:

all-available -> active: [ai-gateway, claude, codex, ...] # no claude-desktop
config -> active: [ai-gateway, claude-account, claude-desktop]

and then by a real hyp init --yes --client claude --no-daemon --force with one entrypoint: "claude-desktop" transcript staged:

backfill claude: ok (scanned 0, wrote 0, skipped 0)
claude.backfill.entrypoint_gate session_id=s-desk entrypoint=claude-desktop
owner_plugin=@hypaware/claude-desktop
claude.backfill.scan_complete files_seen=1 sessions_gated=1 sessions_projected=0

This contradicts LLP 0140's own Consequences (llp/0140:100-102, "Attaching Desktop makes its history importable, and it lands as client_name: \"claude-desktop\"") on the one path where attaching actually happens. The user ticks Desktop, reads and accepts the consent prompt, the plist is written, and the init finale imports none of their Desktop history, so the first-look block at the end of init shows nothing from it. A later manual hyp backfill claude (profile config) does import it, so the two surfaces disagree.

A note on severity, since you should be able to push back. The reviewer rated this non-blocking because it is under-capture rather than over-capture, which is the safe direction, and that reasoning is sound on privacy grounds. I am asking for it anyway on functional grounds: this PR lands a doc claim that its own primary path contradicts, and the failure is silent. If you think the init path is out of scope here and would rather land it with LLP 0140 scoped to the manual path, say so and that is a reasonable resolution too.

2. NON-BLOCKING. The consent gate and the backfill gate mean different things by "opted in", so declining consent does not stop Desktop history from being imported

The consent prompt runs in the wizard's configure phase, which happens after the config has been written with the composed plugins (pick.js:216 precedes wizard/index.js:152). Drop-on-failure only prints a hint and never removes the plugin (wizard/configure.js:91-108), which LLP 0139 states outright at :153-157. The backfill gate keys purely on config membership (backfill/entrypoint_owner.js:41), so after a decline configured is true.

Verified with a config containing exactly what a declined-consent init leaves behind, no plist and no credential:

$ hyp backfill claude --since 2026-01-01T00:00:00Z
claude [ok] rows_written=2 rows_skipped=1
$ hyp query sql "select client_name, count(*) ... group by client_name"
claude 2
claude-desktop 2 <- the Desktop session, imported

Not a regression: on master that same session was imported unconditionally as client_name: claude, so nothing new is captured. But the PR body's "Owned + unconfigured -> skipped ... exactly the case that closes the gate" does not hold once the picker has written the plugin into the config, which is the main way it gets there. Related: the decline option reads "Nothing is changed" at a moment when the config on disk already lists both Desktop plugins and the next backfill will act on it.

3. NON-BLOCKING. hyp claude-desktop install hangs on a non-TTY stdin, and the refusal branch written for that case is unreachable

consent.js:104-110 refuses when !cmdCtx.stdin, but cli/dispatch.js:126 is opts.stdin ?? process.stdin, so it is never falsy in a real invocation. A non-TTY stdin takes the readline branch and rl.question at EOF never settles:

$ hyp claude-desktop install < /dev/null
Attach Claude Desktop with the changes above? [y/N]:
Warning: Detected unsettled top-level await at bin/hypaware.js:54
exit=13

Fails closed, so not a consent hole, but it is the same unattended-hang class this PR fixes for --print-commands, it exits 13 rather than a defined code, and the hint naming the escape hatch never prints. The covering test passes stdin: undefined, a state the dispatcher never produces.

4-6. NON-BLOCKING, smaller

entrypointOwners is declared on BackfillPlanContext but only buildRunContext populates it (backfill.js:725 vs :768), so hyp backfill plan estimates over sessions the run will gate out. The registered command summary drifted from the manifest (claude-desktop/src/index.js:113 vs hypaware.plugin.json:53) with nothing asserting they agree. And there is no platform gate on the picker row: it was inert before, but ticking it on Linux now composes both plugins and runs a consent prompt naming /Library/Managed Preferences/....

The consent gate

Capture cannot begin before consent. Every path traced: interactive attach prompts with default no and a decline exits 1 with zero spawnSync; --yes is explicit and injected by no caller; the preset path no-ops runConfigurePhase entirely; the wizard passes only --print-commands, never --yes; re-attach skips via alreadyConfigured; and no daemon path invokes the command. --print-commands is now genuinely side-effect-free including the login and helper steps, which closes a real prior hang.

Consent is deliberately not persisted, so it cannot be silently re-used or widened - there is no stored token to widen. The cost is that it is not auditable, and the alreadyConfigured proxy is state-based rather than intent-based.

The policy machinery is untouched. The entrypoint gate sits after the usage-policy ignore drop (claude/src/backfill.js:200-221, then :232), and hypignore_capture_drop and local_only_export_withhold both pass. Re-attribution does not break dedup: importing a session as claude then re-running with Desktop configured gives rows_written=0, rows_skipped=1 with client_name preserved, exactly as LLP 0140 states.

No R1a or policy-vocabulary violation. The only URL any new surface prints is the local loopback gateway, and R1a binds the enrolling login's destination surfaces, not a local endpoint.

Two documented fail-open paths widen capture on error (an unknown entrypoint imports; a catalog failure yields an empty map). Both are argued in LLP 0140 and both match pre-PR behaviour. Flagged only because they are the failure modes of a privacy gate.

Tests and conventions

476 of the 1280 added lines are test (37%), 279 LLP prose, 525 code. The consent gate has real negative coverage rather than a happy path only: decline is a no-op with exit 1 and zero side effects, a bare enter declines, --print-commands never prompts, the org_key text omits the sign-in promise. Gaps: nothing exercises runClaudeBackfill with a gated session end to end, nothing covers resolveOwnersForRun's wiring, and the non-interactive test asserts a state production cannot produce (finding 3).

Conventions clean: no U+2014 on any added line, no semicolons, and all eight added @ref anchors resolve, as do the cross-refs they cite. Living-docs satisfied per commit.

One observability gap worth knowing: the consent decision itself emits no log or span attribute, so the wizard records a decline as status: dropped / error_kind: configure_nonzero_exit, indistinguishable from a genuine failure. install.js had no logging before either, so this is a gap rather than a regression.

Verification run

npm test: 2774 tests, 2765 pass, 8 fail, all the pre-existing leave-command.test.js set, no changed file participates. npm run typecheck clean. The three changed test files: 58/58. Smokes backfill_claude_fixture, walkthrough_backfill_client_history, hypignore_capture_drop, local_only_export_withhold, status_diagnostics, walkthrough_picker_to_first_query all ok. local_only_query_withhold fails pre-existing, and interestingly it now breaks on hyp ignore --local-only printing the backing store path, which is the #411 vocabulary rule; no file in this diff is on that path.

@philcunliffephilcunliffe added the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Jul 28, 2026

@philcunliffephilcunliffe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at eacbe94. Full record above. Noting it is a draft, so this is early feedback rather than a gate.

Nothing here captures or exports without an opt-in the PR already requires, and nothing regresses master. The consent text is the strongest part: it names every durable side effect by absolute path, says sudo will be asked for, names the undo, and the tests pin the wording. Every route to capture was traced (interactive, --yes, preset, wizard, re-attach, daemon) and none reaches the credential, helper, or plist without an accepted prompt or an explicit flag.

Requesting changes on one finding: the Desktop gate is always closed during hyp init.resolveOwnersForRun derives "configured" from the plugins activated in this process, but hyp init boots all-available, and @hypaware/claude-desktop is in V1_EXCLUDED_FROM_DEFAULT (bundled.js:82) because it needs the credential capability. The config the picker writes cannot change activePlugins, which were fixed at process start. So a user ticks Desktop, accepts the consent prompt, the plist is written, and the init finale imports none of their Desktop history:

claude.backfill.scan_complete files_seen=1 sessions_gated=1 sessions_projected=0

That is from a real hyp init --yes --client claude --no-daemon --force run, not a unit test. It contradicts LLP 0140's own Consequences at :100-102, which this PR lands: "Attaching Desktop makes its history importable." A later manual hyp backfill claude does import it, so the two surfaces disagree.

On severity, so you can push back: my reviewer rated this non-blocking because it is under-capture, the safe direction, and that is sound on privacy grounds. I am escalating it on functional grounds - the PR lands a doc claim its own primary path contradicts, and the failure is silent. If you consider the init path out of scope and would rather scope LLP 0140 to the manual path, that is a reasonable resolution and I would take it.

Two others worth your attention, both non-blocking: declining consent does not stop Desktop history from being imported, because the picker writes the config before the prompt runs and drop-on-failure never removes the plugin (not a regression, but the PR body's "owned + unconfigured -> skipped" does not hold once the picker has written it); and hyp claude-desktop install < /dev/null hangs then exits 13, because the refusal branch guards on !cmdCtx.stdin, which the dispatcher never produces.

Advisory as always: neutral does not ready or merge contributor PRs.

philcunliffe pushed a commit that referenced this pull request Jul 28, 2026
…a pass
Renumber the new decision doc from 0139 to 0141: open PR #422 claims 0139
and 0140, so whichever landed second would have collided. Every @ref and
prose link moves with it; both anchors still resolve.
Also, from review:
- docs/ACCEPTANCE.md step 5 asked for rows written by a backfill of a
session step 3 already captured live. The materializer's part_id dedupe
suppresses that duplicate, so a healthy system read as a failure. The
pass condition now states outright that rows_written: 0 with
rows_skipped >= 1 is the expected result.
- The LLP's backfill bullet now marks its own confidence: the shared
rollout tree rests on the provider's assumption and on fixtures, and
the acceptance procedure is what confirms it on real hardware.
- covered_by drops 211 characters of prose for two tokens,
gateway_live,codex_sessions_rollout, so it stays queryable like every
other attribute on the event. The prose stays in the LLP and README.
- The release checklist points at docs/ACCEPTANCE.md when a release
touched a client adapter.
- Step 7 re-attaches, so the procedure does not leave an operator's own
machine with Codex capture off.
- Test 2 is renamed to what it asserts, and test 3 now pins covered_by on
the structured log as well as the event.
- The picker summary loses its trailing clause; walkthrough.js writes it
as one unwrapped line.
Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe added a commit that referenced this pull request Jul 28, 2026
* Say plainly that the Codex source covers Codex Desktop
The Codex adapter has always captured Codex Desktop, by two routes:
`hyp attach codex` writes the `~/.codex/config.toml` both the CLI and
Desktop read, and the backfill provider reads the `~/.codex/sessions`
rollout tree both write. Nothing in the product surface said so, and
three things implied the opposite: the picker read "capture Codex
conversations", Claude Desktop ships a dedicated setup (so desktop
clients look like they need their own adapter), and the backfill
provider flags `Application Support/Codex` as an unsupported location
with no explanation.
- Picker label, summary, plugin description, both hypaware-reference
skills, README, and PRIVACY.md now name Codex CLI and Codex Desktop.
- New LLP 0139 records why Codex Desktop rides the ordinary adapter
while Claude Desktop needs its own (shared config file and shared
rollout tree vs a root-owned managed plist), and where the unsupported
boundary actually is.
- The `codex_desktop_app` unsupported_location event and log now carry a
`covered_by` attribute naming the live gateway route and
`~/.codex/sessions`, so the flag reads as "this directory", not "this
client".
- New docs/ACCEPTANCE.md carries an opt-in, manual `codex_desktop_capture`
procedure (a human, a real Mac, a real Codex Desktop). AGENTS.md points
at it. `gateway_codex_capture` now states in its own header that its
Desktop-shaped request is synthetic and proves nothing about a real app.
Not done: making `hyp status` (or another product surface) report recent
Codex Desktop traffic from `entrypoint`. `hyp status` boots with no
plugins activated by design, so it has no dataset registry and no cache
read, and putting client-specific knowledge in core cuts against LLP 0130
and LLP 0003. Every route out of that needs a design decision; the
analysis is in LLP 0139's consequences.
Co-Authored-By: Claude <noreply@anthropic.com>
* Review fixes: renumber the Codex Desktop LLP, and say zero writes is a pass
Renumber the new decision doc from 0139 to 0141: open PR #422 claims 0139
and 0140, so whichever landed second would have collided. Every @ref and
prose link moves with it; both anchors still resolve.
Also, from review:
- docs/ACCEPTANCE.md step 5 asked for rows written by a backfill of a
session step 3 already captured live. The materializer's part_id dedupe
suppresses that duplicate, so a healthy system read as a failure. The
pass condition now states outright that rows_written: 0 with
rows_skipped >= 1 is the expected result.
- The LLP's backfill bullet now marks its own confidence: the shared
rollout tree rests on the provider's assumption and on fixtures, and
the acceptance procedure is what confirms it on real hardware.
- covered_by drops 211 characters of prose for two tokens,
gateway_live,codex_sessions_rollout, so it stays queryable like every
other attribute on the event. The prose stays in the LLP and README.
- The release checklist points at docs/ACCEPTANCE.md when a release
touched a client adapter.
- Step 7 re-attaches, so the procedure does not leave an operator's own
machine with Codex capture off.
- Test 2 is renamed to what it asserts, and test 3 now pins covered_by on
the structured log as well as the event.
- The picker summary loses its trailing clause; walkthrough.js writes it
as one unwrapped line.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: neutral-loop <neutral-loop@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
@philcunliffephilcunliffe added neutral:adopt Foreign PR adopted into neutral's reconcile scope neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) and removed neutral:review Delegate this PR to neutral for a review pass (approve or request changes; never merges) labels Jul 29, 2026
Two resolutions, one textual and one semantic. Neither reverses a landed
decision.
1. `claude-desktop/hypaware.plugin.json` (the only git-reported conflict).
#445 (`74aea66`) deleted the `attach_probe` block from
`contributes.client`, restoring the Accepted decision at
LLP 0115#no-attach-on-join and LLP 0135#no-probe. This branch added
`transcript_entrypoints` as a sibling of that block, so the two edits
collided on the same closing brace and nothing more. Took master's
deletion whole and kept only the new field:
"agent_dir": ".claude/agents",
"transcript_entrypoints": ["claude-desktop", "claude-desktop-3p"]
The probe stays gone. `transcript_entrypoints` is a backfill-ownership
label read by `resolveOwnersForRun` (LLP 0140), never by
`probeClientAttachFromDescriptor` or `detachClientFromDisk`, so it does
not reintroduce an attach-on-join path and `test/plugins/
claude-desktop-detach.test.js`'s no-probe assertion still holds.
2. `test/core/compose-picker-config.test.js` (a silent semantic conflict
git auto-merged). #433 (`8eccc04`) stopped `composePickerConfig` pinning
the gateway `listen` so LLP 0114's fixed default applies to a
wizard-created install. The `claude-desktop composes ...` case added
here predates that and still expected
`listen: '127.0.0.1:8787'`, so it was the one gateway slice in the file
carrying a `listen` and it failed on the merge. Dropped the pin to match
master's landed decision and the other eleven cases; the assertion this
test exists for, that the Desktop row composes both plugins its
`configure_command` needs, is untouched.
`src/core/daemon/status.js` merged clean and reads coherently: master's
no-probe guard (`descriptor.attachProbe ? probe : { attached: false }`)
sits directly above this branch's `client_attach_missing` repair fix. Post
#445 that fix matters more, not less: with no probe at all, a configured
claude-desktop always reports `attached: false`, so the repair line is
always printed, and the generic `hyp attach --client claude-desktop` it
used to print answers `unknown client`.
npm test: 2873 pass / 8 fail, exactly the pre-existing
test/core/leave-command.test.js set. npm run typecheck clean. Smokes
walkthrough_picker_to_first_query, cli_bundled_plugins_activated,
status_diagnostics ok.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor

Conflict resolved, eacbe94 -> 19f45c5

master moved five PRs under this branch (#438, #439, #441, #447, #445) and it went DIRTY. Merged origin/master in and pushed the merge commit to this branch. Two things needed deciding, one textual and one that git merged silently and wrongly.

I checked the crux first, because #445 looked like it might have invalidated this PR's premise. It has not. Details below.

1. hypaware-core/plugins-workspace/claude-desktop/hypaware.plugin.json - the only conflict git reported

The two edits landed on the same closing brace and nothing more:

sidewants
master (#445, 74aea66)attach_probedeleted from contributes.client, leaving skill_dir/agent_dir
this branchtranscript_entrypoints: ["claude-desktop", "claude-desktop-3p"] added as a sibling of attach_probe

Took master's deletion whole and kept only the new field. The probe stays gone:

"agent_dir": ".claude/agents",
"transcript_entrypoints": ["claude-desktop", "claude-desktop-3p"]

The reason this is incidental rather than a reversal: transcript_entrypoints is a backfill-ownership label. It is read by resolveOwnersForRun (src/core/commands/backfill.js) and nowhere else - never by probeClientAttachFromDescriptor or detachClientFromDisk, which are what an attach_probe feeds. So it does not reintroduce an attach-on-join path, and test/plugins/claude-desktop-detach.test.js's "no attach_probe on the manifest" assertion still passes unchanged.

Decisions checked before taking that side:

2. test/core/compose-picker-config.test.js - a semantic conflict git auto-merged

Worth flagging, because this one produced no conflict markers and would have gone to CI red.

#433 (8eccc04) stopped composePickerConfig pinning the gateway listen so LLP 0114's fixed default applies to a wizard-created install. The claude-desktop composes the gateway, the credential plugin, and its own adapter case added on this branch predates that and still expected:

{name: '@hypaware/ai-gateway',config: {listen: '127.0.0.1:8787',upstreams: [ANTHROPIC]}}

It was the only one of the twelve gateway slices in the file still carrying a listen, and it was the single extra failure on the raw merge. Dropped the pin, matching master's landed decision and the file's own header comment ("the gateway slice carries no listen, so LLP 0114's fixed default ... appl[ies]"). The assertion this test exists for - that the Desktop row composes both plugins its configure_command needs - is untouched, as is the sibling test that pins provider-before-consumer ordering.

Clean auto-merges worth a second look

src/core/daemon/status.js merged clean and reads coherently: #445's no-probe guard (descriptor.attachProbe ? await probeClientAttachFromDescriptor(...) : { attached: false }) sits directly above this branch's client_attach_missing repair fix.

Post-#445 that fix matters more, not less. With no probe at all, a configured claude-desktop always reports attached: false, so the client_attach_missing warning is now always emitted for it - and the generic hyp attach --client claude-desktop it used to print answers unknown client. This branch replaces it with the row's own configure_command. That is the same direction #445 travelled, so the two changes reinforce each other.

hypaware-core/plugins-workspace/claude-desktop/src/index.js also merged clean, keeping #445's narrowed @ref LLP 0115#no-attach-on-join gloss alongside this branch's claude-desktop install help text and --yes usage.

Verification

  • npm test: 2873 pass / 8 fail, exactly the pre-existing test/core/leave-command.test.js set. No changed file participates.
  • npm run typecheck: clean.
  • Smokes: walkthrough_picker_to_first_query, cli_bundled_plugins_activated, status_diagnostics all ok.
  • Conventions: no semicolons, no U+2014 on any line I touched, manifest parses.

For you to double-check

  1. LLP numbering. This branch adds llp/0139 and llp/0140; master has since added llp/0141 (Codex Desktop rides the ordinary Codex adapter). No filename collision, and 0141 does not touch Desktop's attach surface, but 0141 cites LLP 0115 too, so you may want the three read together.
  2. The listen change is mine, not the author's. If the Desktop row was meant to pin a port for some Desktop-specific reason, my resolution silently drops that intent in favour of LLP 0114. I could find no such reason in LLP 0139 or the PR body, and every other row composes without a listen.
  3. The changes-requested finding is untouched. The hyp init gate (resolveOwnersForRun deriving "configured" from ctx.plugins, which all-available fixes at process start without @hypaware/claude-desktop) is unaffected by this merge and still stands as a finding.

The PR stays a draft. Neutral does not ready or merge contributor PRs.

…be94
Three findings from the round-1 review at `eacbe940`, all still standing at
`19f45c5`.
**The entrypoint gate was always closed during `hyp init`** (blocking).
`resolveOwnersForRun` derived "configured" from `ctx.plugins`, the plugins
activated in this process. `hyp init` boots the `all-available` profile,
which by construction never activates a `V1_EXCLUDED_FROM_DEFAULT` plugin,
and `@hypaware/claude-desktop` is on that list. The picker cannot change an
activation set fixed at process start, so a user who ticked Claude Desktop
and accepted the consent prompt got the plist written and then had their
Desktop history silently gated out of the finale's own backfill: the exact
inverse of LLP 0139's "works end to end" and of what LLP 0140 says the
answer is derived from (the *effective plugin list*). Now unions the
activation set, the boot-resolved `ctx.config`, and a fresh read of the
local config document, the only one of the three that reflects a config
written after boot. Fails open, matching LLP 0140#fail-open-on-unknown.
**The consent prompt hung on a stdin that ends without an answer.** The
`!cmdCtx.stdin` refusal is unreachable in production (`cli/dispatch.js`
defaults it to `process.stdin`), and `rl.question` never settles at EOF, so
`hyp claude-desktop install < /dev/null` blocked forever with the escape-
hatch hint unprinted, the same unattended-hang class `--print-commands`
fixes. Now decided by the `line`/`close` events, so EOF declines with exit 1
and the hint. Still fails closed; a piped `y` still consents.
**`hyp backfill plan` did not get the ownership map**, although
`entrypointOwners` is declared on `BackfillPlanContext`, so a planning
provider would have estimated over sessions the run then gates out.
Plus two smaller ones: the registered `claude-desktop install` summary had
drifted from the manifest (now asserted for every command the manifest
declares), and the decline option claimed "Nothing is changed" at a moment
when the wizard has already written the config.
7 new tests. The ownership test fails against the pre-fix resolver.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 2 - 19f45c5, healed to 8c64a56

Verdict: findings, all actionable ones fixed and pushed. The blocking finding from round 1 still stood at 19f45c5 and is now closed. Both of the conflict resolutions in eacbe94 -> 19f45c5 check out. Nothing actionable remains.

Head is now 8c64a56. CI green on it (test and typecheck, Node 22 and 24).


1. Verifying neutral's two conflict resolutions

claude-desktop/hypaware.plugin.json - correct.contributes.client is { name, skill_dir, agent_dir, transcript_entrypoints }. The probe is gone and the new field is intact:

$ grep -rn "attach_probe" hypaware-core/plugins-workspace/claude-desktop/
src/index.js:54: * descriptor deliberately carries **no `attach_probe`**: ...
src/index.js:60: * @ref LLP 0115#no-attach-on-join [constrained-by]: no `attach_probe` ...

Both hits are prose. test/plugins/claude-desktop-detach.test.js:95 ("the claude-desktop manifest declares no attach_probe") passes. Independently confirmed the claim that the new field cannot reintroduce an attach path: transcriptEntrypoints has exactly two readers, plugin_catalog.js:81 (which sets it) and resolveEntrypointOwners. probeAttachedClients skips any descriptor without attachProbe (daemon/status.js:1123), so a contributes.client with no probe is inert for attach, which is what master itself keeps. LLP 0115#no-attach-on-join and LLP 0135#no-probe both hold.

test/core/compose-picker-config.test.js - correct, and it was the right call to make. No gateway slice in the file pins listen any more; the only surviving listen* keys are the two @hypaware/otellisten_host/listen_port pairs at :120 and :201, which are a different plugin's config. That matches master's LLP 0114 decision and the file's own header comment at :21.

I looked specifically for a Desktop-specific reason to pin a port, since the resolution was neutral's and not the author's. There is none: the Desktop profile reads its endpoint from resolveInputs, which derives it from the effective gateway listen at install time, and runInstall already refuses up front on an ephemeral 127.0.0.1:0. A pinned 8787 in the composed config would have been a coincidence of the default, not a requirement. The assertion the test exists for is untouched, and the sibling provider-precedes-consumer case at :321-330 still asserts the ordering.


2. Prior review's findings, per finding, at 19f45c5

#Round-1 findingStatus at 19f45c5Now
1Desktop gate always closed during hyp initSTOOD (blocking)FIXED
2Consent decline does not stop transcript importSTOOD (non-blocking)Not fixed - design-accepted, see below
3install hangs on a non-TTY stdin; refusal branch unreachableSTOODFIXED
4entrypointOwners never populated on BackfillPlanContextSTOODFIXED
5Registered command summary drifted from the manifestSTOODFIXED
6No platform gate on the picker rowSTOOD (non-blocking)Not fixed - needs a new manifest key, out of scope

Finding 1, blocking, FIXED

Confirmed standing before fixing. computeSelectedPlugins (runtime/boot.js:532-542) filters all-available on !V1_EXCLUDED_FROM_DEFAULT.has(name), and @hypaware/claude-desktop is at runtime/bundled.js:82. hyp init takes that profile (cli/dispatch.js:483). Nothing in cli/wizard/index.js touches ctx.config after pick.js:220 writes the file, so the finale's backfill really did read a boot-time snapshot that could never contain the plugin the user had just ticked.

resolveOwnersForRun now derives "configured" from the effective config, which is what LLP 0140#manifest-declares-ownership and the BackfillPlanContext.entrypointOwners doc comment both already said it was. Three sources unioned (commands/backfill.js:803-816): the activation set, the boot-resolved ctx.config, and a fresh read of the local config document, the last being the only one that reflects a config written after boot. Unioning fails open, matching LLP 0140#fail-open-on-unknown.

The local read deliberately does not go through loadConfigFile: that helper emits a config.load_failedERROR row on ENOENT, and daemon/status.js counts ERROR rows for hyp status. A host with no config document is an ordinary state for a membership probe, not an error.

Positively verified: the new test fails against the pre-fix resolver and passes after.

# with `configured` derived from ctx.plugins only
not ok 1 - test/core/backfill-command.test.js # fail 1
# with the fix
ok 22 - the entrypoint gate counts a config-listed plugin as configured
even when this process never activated it # pass 25

Finding 3, FIXED

Reproduced first, against 19f45c5, as an unsettled promise rather than by inference:

$ node repro.mjs # Readable.from([]) as cmdCtx.stdin
RESULT: HUNG (no settle after 3s)

Two things were wrong. !cmdCtx.stdin is unreachable (cli/dispatch.js:126 is opts.stdin ?? process.stdin), and rl.question never settles when the input reaches EOF without a line.

Fixed at consent.js:139-152 by deciding on the line/close events instead of the question promise. Worth flagging for anyone reading the diff: my first attempt raced rl.question against a close promise and that was wrong - it lost a microtask-ordering race and made Readable.from(['y\n']) decline, which the existing "accepting the consent prompt runs the steps" test caught immediately. One promise settled by whichever event fires first is decidable; racing two promises is not.

$ node repro.mjs
STDERR: claude-desktop install: needs an interactive terminal to confirm.
Re-run with --yes ... or --print-commands ...
RESULT: settled -> false

Still fails closed, now with exit 1 rather than 13, and the escape-hatch hint actually prints. A piped y still consents, which a new test pins so the guard cannot silently close the answered path.

Finding 4, FIXED

hyp backfill plan now resolves the same map and passes it on the plan context (commands/backfill.js:238-257), resolved once and only when a selected provider actually plans. Latent today - no bundled provider implements plan - but the field was declared on BackfillPlanContext, so the first one to consult it would have over-estimated in exactly the direction the gate exists to correct.

Finding 5, FIXED

index.js:120 gained the explain and confirm, clause the manifest already had. Added the missing guard: a test that activates the plugin and asserts every command the manifest declares is registered with the same summary, and that nothing undeclared is registered. That is what was absent when the two drifted.

Finding 2 - NOT fixed, deliberately, and my fix touches it

Not fixed because both available fixes reverse an Accepted decision, which is not mine to do on a contributor's PR:

  • removing the plugins from the config on a decline contradicts LLP 0139 Consequences verbatim ("That is the converging state, not a broken one") and LLP 0131's drop-on-failure rule;
  • persisting a consent token contradicts LLP 0139's "Consent is deliberately not persisted".

But you should know my fix to finding 1 moves this boundary, so I am flagging it rather than burying it. Before, on the init path, a decline left Desktop history un-imported for that run only - not by design, but as a side effect of the bug in finding 1; the next hyp backfill claude imported it anyway. After, the decline path imports in the same run, because config membership is now read correctly and config membership is what LLP 0140 keys the gate on.

Why I judged that acceptable rather than blocking:

  • No new capture surface versus master. On master that same session was imported unconditionally as client_name: "claude". The change is attribution, not admission. The pre-fix behaviour deferred by one command; it did not protect.
  • The consent prompt never claimed to cover this.buildConsentExplanation enumerates the credential, the helper, the plist and the restart. It is about making this machine hold a credential, not about reading local transcript files, and LLP 0140 Context says so outright ("That gate covers the credential and plist path; it does not cover the door history actually arrives through"). Backfill has its own consent prompt.
  • The alternative - threading configure-phase outcomes into the finale's backfill - is a new mechanism and a new decision, not a review fix.

I did make the one honest correction available without reversing anything: the decline option read Nothing is changed at a moment when, on the wizard surface, the config on disk already lists both Desktop plugins. It now reads This command changes nothing, which is true on both surfaces.

Finding 6 - NOT fixed

Would need a new platform key on the picker manifest plus a validator and the matching manifest.js probe-key handling. Real, but a schema addition rather than a review fix. Mitigation as before: detect.app_bundle stats /Applications/Claude.app, so on Linux the row is never pre-checked and ticking it is an explicit act.


3. The consent gate

Re-traced independently, and it holds.

It defaults to no on every path. TUI: select({ default: 'no' }) with the decline option listed first. Non-TUI: [y/N], and only y/yes after trim and lowercase pass. Everything else is a no, and now that includes EOF.

It cannot be bypassed by a stray flag.--yes is argv.includes('--yes') on the command's own argv. The only in-process caller of any configure_command is wizard/configure.js:94, and it passes printCommandsFlag(opts), which is ['--print-commands'] or [] - never --yes, and it only passes --print-commands if the wizard itself was invoked with it. The non-interactive init callers (--yes, --dry-run, presets, --from-file) set opts.picks, which no-ops runConfigurePhase at :48 before any descriptor is reached. Grepped every commands.run( call site in src/ and hypaware-core/: the other three are claude-account status, claude-account login and claude-desktop install-helper, all from inside install.js itself and all after the gate.

No non-TTY path bypasses it - the one that existed hung rather than consenting, and now declines.

Declining leaves nothing written. The gate sits at install.js:130-149, before steps.push(...). Everything above it is a read: resolveInputs, plistUpToDate, computeDesiredPlistContent, fs.existsSync. Verified by assertion, not just by reading: the decline tests assert commandCalls empty (no credential login, no helper write), spawnCalls.length === 0 (no sudo), and the new EOF test additionally asserts no plist file exists afterwards.

--print-commands remains genuinely side-effect-free including the login and helper steps, which is what makes it correct for the gate to skip it.

The alreadyConfigured skip is still state-based rather than intent-based (plist matches and helper exists). Unchanged from round 1 and argued in LLP 0139.


4. transcript_entrypoints ownership gating

No off-by-one, and structurally there cannot be a path-prefix one. I went looking for the sibling-directory case specifically. The map is keyed on the transcript's entrypointfield value, not on a path, and the lookup is an exact Map.get (entrypoint_owner.js:71). No startsWith, no path.relative, no directory comparison anywhere in the gate. walkTranscriptFiles still decides which files are scanned; ownership only decides what happens to a session already found. So claude-desktop cannot match claude-desktop-3p or any sibling by prefix.

Owned + unconfigured is gated before projection.backfill.js:232-243, continue before projectedExchangeFromEntries, i.e. before any row exists - and after the usage-policy ignore drop at :207, so the policy machinery still wins. A gated session is counted into sessions_gated and logged once.

Re-attribution cannot touch rows it does not own.classifyTranscriptEntrypoint returns clientName: scanningClient for both no-entrypoint and unknown-entrypoint, and only substitutes owner.client when an owner exists and is configured. That one value feeds both projectedExchangeFromEntries and the item's client_name, so the projected exchange and the row can never disagree.

A claimed-but-unconfigured value cannot be silently filed under claude - that is the one case that closes the gate, and after the finding-1 fix "configured" finally means what the doc says, so the gate now opens on the path where the opt-in actually happens instead of being stuck shut.

Two lesser properties checked: first-declaration-wins on a duplicate claim (:38) is unreachable today, since claude claims cli/sdk-cli and claude-desktop claims claude-desktop/claude-desktop-3p, disjoint; and plugin_catalog.js:81-85 filters non-string and empty entries, so a malformed manifest cannot install an empty-string key that would collide with a missing entrypoint.

sessionEntrypoint reads sessionEntries, not windowed, so a time window cannot clip the lines carrying the field and accidentally reopen the gate. Good, and the comment says why.


5. LLP numbering, refs, conventions

  • No collision.0139 and 0140 from this branch, 0141 from master. Read 0141 against 0139/0140: it routes Codex Desktop through the ordinary Codex adapter and cites LLP 0115 for the same no-attach-on-join reason, so the three are consistent - 0141 adds no client, no attach_probe, and no transcript_entrypoints, and nothing in it needs updating for this branch.
  • Every @ref anchor resolves. All nine distinct anchors used across the branch (0131#idempotent-rerun, 0139#compose-the-whole-dependency-set, #informed-consent, #default-no, #print-commands-applies-nothing, #repair-must-be-runnable, 0140#manifest-declares-ownership, #gate-before-projection, #fail-open-on-unknown) exist as <a id=...> targets, including after my doc edits.
  • No stale cross-reference found, and I re-read the two @refs I touched rather than assuming: 0139#default-no now states that every non-answer is a no, and 0140#manifest-declares-ownership now states what "configured" means. Both glosses match.
  • No U+2014 on any added line. No semicolons. Living-docs satisfied: the 0139 and 0140 edits are in the same commit as the code that changed their meaning.

Verification run, 8c64a56

  • npm test: 2880 pass / 8 fail, exactly the pre-existing test/core/leave-command.test.js set. Baseline before my changes on the same worktree was 2873/8, so +7 tests, no new failures.
  • npm run typecheck: clean.
  • Smokes all ok: core_boot_noop, cli_bundled_plugins_activated, backfill_claude_fixture, walkthrough_backfill_client_history, walkthrough_picker_to_first_query, status_diagnostics, hypignore_capture_drop, local_only_export_withhold.
  • CI on 8c64a56: test (22), test (24), typecheck (22), typecheck (24) all pass.

Confirmed the push landed in the committed tree, not just locally, by reading it back from the API at 8c64a56.

The PR stays a draft. Neutral does not ready or merge a contributor's PR.

@philcunliffephilcunliffe added neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) and removed neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) labels Jul 29, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Verdict: approved (adopted PR, LLP 0025) - neutral:changes-requested cleared

Round 2 reviewed 19f45c5 and healed to 8c64a56. Nothing actionable remains.

  • The round-1 blocking finding (the Desktop entrypoint gate permanently closed during hyp init) still stood at 19f45c5 and is fixed, with a test that fails against the pre-fix resolver.
  • Three of the four non-blocking findings are also fixed: the non-TTY consent hang, the unpopulated BackfillPlanContext.entrypointOwners, and the manifest/runtime summary drift (now guarded by a test).
  • Both of neutral's conflict resolutions in eacbe94 -> 19f45c5 verified correct: the attach_probe deletion holds and the new transcript_entrypoints field cannot reintroduce an attach path; the dropped gateway listen pin matches master's landed LLP 0114 decision, with no Desktop-specific reason to pin found.
  • The consent gate defaults to no on every path, cannot be reached with an injected --yes, no longer hangs on a redirected stdin, and a decline writes nothing (asserted: no credential, no helper, no sudo, no plist).
  • The ownership gating cannot mis-attribute: exact-value Map lookup with no path-prefix logic anywhere, gated before projection and after the usage-policy drop, and re-attribution only ever substitutes for an entrypoint an installed plugin explicitly claims.

Two things to look at before you merge, neither blocking:

  1. My finding-1 fix moves the decline boundary, on purpose but worth your eyes. Reading "configured" correctly means a hyp init where the user declines the Desktop consent prompt now imports their Desktop transcripts in the same run, because LLP 0140 keys the gate on config membership and LLP 0139 deliberately leaves the plugins in the config after a decline. This captures nothing master did not already capture (it imported the same sessions unconditionally as client_name: "claude"), and the consent prompt is explicitly about the credential and the plist rather than transcript reads. Closing it properly means reversing one of two Accepted decisions - dropping plugins on a configure decline, or persisting consent - which is your call, not neutral's. Detail in the round record above.
  2. LLP 0139/0140 still carry the unverified items the author listed under "Not addressed", in particular that no plist was ever installed during development, so this PR makes the Desktop attach reachable without proving capture works.

CI green on 8c64a56: test (22), test (24), typecheck (22), typecheck (24). Local npm test 2880 pass / 8 fail, exactly the pre-existing leave-command.test.js set; typecheck clean; eight smokes ok.

The PR remains a draft. Readying and merging are yours, including for an adopted PR.

@philcunliffephilcunliffe added neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) and removed neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) labels Jul 29, 2026

@philcunliffephilcunliffe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neutral healed this PR under its neutral:adopt grant and pushed the fixes (head 8c64a56, CI green). Requesting changes for the residuals it deliberately did not fix, because each needs a decision that is yours, not neutral's.

Fixed and pushed (details in the round record): the always-closed hyp init gate (the blocking one), the non-TTY consent hang (it now declines rather than hanging, decided on line/close rather than rl.question), entrypointOwners missing from the plan context, and manifest/runtime summary drift plus a guard test. resolveOwnersForRun now derives "configured" from the effective config, deliberately avoiding loadConfigFile because it emits a config.load_failed ERROR row that hyp status counts.

Residual 1 - declining consent does not stop transcript import. LLP 0140 keys the transcript gate on config membership, and LLP 0139 deliberately leaves Desktop's plugins in the config after a decline. So a hyp init where the user declines still imports Desktop transcripts in the same run. This captures nothing master did not already capture (master imported the same sessions, filed as client_name: "claude"), and the consent text is about the credential and plist rather than transcript reads - so it is arguably out of scope for this PR. But it is a decline that does not decline everything a user might reasonably expect, and closing it means reversing one of two Accepted decisions. Neutral will not reverse an Accepted LLP on its own.

Residual 2 - no platform gate on the picker row. Claude Desktop's managed-preferences path is macOS-only, but the picker row has no platform predicate, so it is offered on Linux and Windows where claude-desktop install cannot succeed. Fixing it needs a new manifest key (a platforms or detect.platform field), which is a kernel-contract addition - again yours to sanction, and worth deciding alongside whether other rows need the same.

Also worth your eye: the finding-1 fix moves the decline boundary rather than merely repairing it, which is why residual 1 exists in its current shape. Neutral flagged this rather than quietly picking a side.

Nothing here blocks on neutral. Reply on this thread with a direction for either residual and neutral will implement it; or if you consider both out of scope for this PR, say so and it can be approved as-is.

bgmcmullenand others added 3 commits July 29, 2026 16:45
…plugin
hyp init boots the all-available profile, which never activates a
V1_EXCLUDED_FROM_DEFAULT plugin, and the command registry is fixed at
process start while the picker writes its composed config later in the
same process. So on a first-run init the configure phase's in-process
claude-desktop install missed dispatch (exit 2), drop-on-failure printed
the catch-up hint, and the LLP 0139 consent prompt was unreachable from
the wizard, the surface it was built for.
On a registry miss the seam now re-reads the effective config from disk
and, when a config-profile boot of that fresh read would select the
plugin declaring the missed command's head token, activates it into the
running kernel with its config-selected dependency closure in dependency
order (claude-account before claude-desktop). Same fresh-read rule as
the entrypoint gate fix in 8c64a56. Scoped to the in-process seam only;
on any failure the LLP 0098 unavailable-plus-repair miss path reports
exactly as before.
Design: LLP 0139#seam-fresh-activation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every surface the install touches is macOS-specific; on Linux the
privileged sequence would half-succeed (sudo mkdir creates root-owned
junk under /Library while configuring nothing). Both commands now refuse
loudly off-platform before consent, mutating nothing. --print-commands
still passes: it applies nothing and printing the would-be commands is
useful anywhere.
Authored in a parallel working session on this branch; committed with
its tests and LLP section after verifying all 28 install tests pass.
Design: LLP 0139#macos-only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Attached Desktop (managed 3p profile) does not write into the shared
~/.claude/projects: it boots a separate Claude-3p container and runs
each conversation's embedded CLI in a per-session sandbox home, so the
transcript lands in a .claude/projects tree nested there, tagged
entrypoint local-agent (Desktop app 1.13576.0 / CLI 2.1.177+; the
earlier build LLP 0133 live-tested used a nested container and
claude-desktop-3p). Live projection therefore fell back to gateway
identity with no entrypoint, backfill never saw the sessions, and
Desktop traffic was indistinguishable from Claude Code.
The claude adapter now discovers the nested .claude/projects dirs under
both observed container layouts: loadTranscript falls back to them on a
primary-tree miss (the common CLI path pays nothing), and the backfill
scan walks them after the shared tree, resolved fresh per run. The
Desktop manifest claims local-agent beside its two earlier entrypoint
values so the LLP 0140 ownership gate and attribution cover the current
build, and the verify hint names the value that actually appears.
Verified against a real attached Desktop: live rows carry entrypoint
local-agent with native transcript identity (provider_uuid/parent_uuid),
and backfill imports the sandbox sessions attributed to claude-desktop.
Design: LLP 0133#attribution (drift recorded as dated observations),
LLP 0140 (vocabulary and dedup consequences updated).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullen
bgmcmullen marked this pull request as ready for review July 29, 2026 23:46
@bgmcmullenbgmcmullen removed the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Jul 29, 2026
@bgmcmullenbgmcmullen added neutral:review Delegate this PR to neutral for a review pass (approve or request changes; never merges) and removed neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) labels Jul 29, 2026
@bgmcmullen

Copy link
Copy Markdown
ContributorAuthor

Pushed three follow-up commits after live-testing the branch on a real macOS host with Claude Desktop.

1. f26c921 - the wizard could never actually show the consent prompt.hyp init boots the all-available profile, which by construction never activates a V1_EXCLUDED_FROM_DEFAULT plugin, and the command registry is fixed at process start while the picker writes its composed config later in the same process. So on a first-run init the configure phase's in-process claude-desktop install missed dispatch (exit 2) and drop-on-failure printed the catch-up hint - the LLP 0139 gate was unreachable from the one surface it was built for; only the standalone re-run ever showed it. The ctx.commands.run seam now resolves a miss by re-reading the effective config from disk and activating the freshly enabled plugin (plus its config-selected dependency closure, in dependency order) into the running kernel - the same fresh-read rule as the entrypoint-gate fix in 8c64a56. Scoped to the in-process seam; the LLP 0098 miss messaging is unchanged on any failure. New section: LLP 0139#seam-fresh-activation.

2. f668d81 - install/verify refuse off-macOS before consent (LLP 0139#macos-only). Authored in a parallel session on this branch; committed with its tests after verifying they pass.

3. c01ef0e - the current Desktop build broke attribution, in both directions. Attached Desktop (app 1.13576.0, embedded CLI 2.1.177+) does not write transcripts into ~/.claude/projects at all: it boots a separate Claude-3p container and sandboxes each conversation's embedded CLI, so transcripts land in a nested .claude/projects tree tagged entrypoint: "local-agent" - not the claude-desktop-3p this PR's docs and verify hint promised (that value came from the earlier build LLP 0133 live-tested; the vocabulary drifted once within a week). Net effect observed live: capture worked but rows landed as gateway_fallback with no entrypoint, indistinguishable from Claude Code, and backfill never saw the sessions. The claude adapter now scans both observed 3p container layouts (live fallback on a primary-tree miss, so the common CLI path pays nothing; backfill walks them after the shared tree), and the Desktop manifest claims local-agent so the LLP 0140 gate and attribution cover the current build. LLP 0133#attribution now records the layouts/values as dated per-build observations rather than constants.

Live verification on the real app: a Desktop conversation captured after the fix carries entrypoint: local-agent with native transcript identity (provider_uuid/parent_uuid, no fallback marker) stamped at capture time, and hyp backfill claude imports the sandbox sessions attributed to client_name: claude-desktop.

Known gaps left out of this push: client_attach_missing fires forever for probe-less clients (Desktop shows "not attached" even after a verified install - follow-on from #444/#445), and the install's residue-clear still targets the old nested Claude-3p path only (deliberately: the new sibling container holds live session data and must never be cleared).

🤖 Generated with Claude Code

@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 3 - c01ef0e (review-only, neutral:review)

Verdict: findings. This round is review-only, so nothing was pushed and nothing was fixed for you. One blocking finding, and it is in the highest-severity class for this repo: c01ef0e adds a new transcript scan root that belongs entirely to Claude Desktop, and the existing fail-open rule lets sessions from it be imported and filed as client_name: "claude" on a machine where @hypaware/claude-desktop was never configured. Reproduced below, not inferred.

Everything the previous rounds verified about the credential and plist gate still holds at this head, and f26c921 fixes something round 2 missed: the consent prompt was unreachable from hyp init at all. Credit for finding that from a real machine.


1. Previous round's asks

Round 2 (8c64a56) requested changes on two residuals, both flagged as maintainer decisions.

Round-2 residualStatus at c01ef0e
1. Declining consent does not stop transcript importNot addressed, and now materially wider. See below.
2. No platform gate on the picker rowPartly addressed.f668d81 refuses install/verify off darwin before consent, mutating nothing (install.js:126-130, verify.js:57-61), with tests that assert no login, no helper, no sudo, no plist. That is a real improvement and it makes the off-platform failure loud. The requested decision is still open though: the picker row itself has no platform predicate, so on Linux the row is still offered, still composes both plugins into the config, and the configure phase now ends in a refusal instead of a missing command. If you consider the manifest platforms key out of scope for this PR, say so on the thread and it can be closed as scoped-out.

Residual 1 got wider, which is why it is worth re-reading rather than carrying forward. Two things changed under it:

  • f26c921 makes the consent prompt actually appear during hyp init. Before this commit, ticking Desktop in the wizard never showed the prompt (registry miss, exit 2), so "a user who declines" was mostly hypothetical on the init surface. Now it is the normal path, and a decline still leaves both plugins in the config (LLP 0139 Consequences), which is what LLP 0140 keys the gate on.
  • c01ef0e changes what "configured" admits. It is no longer only Desktop-tagged sessions sitting in the user's own ~/.claude/projects; it is now every session under Desktop's Claude-3p container. So a user who reads the Desktop consent prompt, declines it, and continues gets Desktop's sandboxed conversation history imported in the same run, from a directory master could not read at all.

That is a decision the maintainer deferred, and neutral is not reversing an Accepted LLP. Flagging it because the deferral was made against a smaller corpus than the one that now rides on it.


2. BLOCKING. The Desktop 3p container is scanned unconditionally, and the fail-open rule admits its sessions under client_name: "claude"

createClaudeBackfillProvider resolves the 3p roots on every run with no reference to whether Desktop is configured (hypaware-core/plugins-workspace/claude/src/backfill.js:106-107), and they are walked as ordinary roots (:186). Admission is then decided purely on the session's entrypointvalue: an absent or unclaimed value fails open and imports (src/core/backfill/entrypoint_owner.js:69-70).

Reproduced against the real provider with the owners map resolveOwnersForRun builds for a default install (@hypaware/claude configured, @hypaware/claude-desktop in the catalog and not configured), and two transcripts placed in the sibling sandbox layout this PR added: one tagged with a drifted value (local-agent-v2), one whose records carry no entrypoint at all.

imported items: 2
native_id= sess-drift client_name= claude
source= ~/Library/Application Support/Claude-3p/local-agent-mode-sessions/aaaa/0000/local_x/.claude/projects/sandbox-outputs/sess-drift.jsonl
native_id= sess-noep client_name= claude
source= ~/Library/Application Support/Claude-3p/.../sess-noep.jsonl
scan_complete: files_seen=2 sessions_projected=2 messages_projected=4 sessions_gated=0
unclaimed_entrypoints="local-agent-v2=1"

Four message rows written from Claude Desktop's private container, on a machine that never configured Desktop, filed as Claude Code.

Why the two triggering cases are not hypothetical:

  • The value drifts. Your own commit message and LLP 0133#attribution record claude-desktop-3p becoming local-agent inside one week. #fail-open-on-unknown names "the first time a client ships a new entrypoint value" as the case it is designed to let through. Applied to the shared tree that is a benign mis-filing; applied to Desktop's container it is admission of a client the user never opted into.
  • A file can carry no value.LLP 0140 itself notes attachment and summary records omit the field, and sessionEntrypoint returns undefined when no record carries it, which imports.

Three things make this the wrong root for a value-based fail-open:

  1. Ownership of these roots is known by construction. Every file under local-agent-mode-sessions/**/.claude/projects is Desktop's, whatever string it happens to be tagged with. The gate is guessing an answer it already has from the path it just walked.
  2. .hypignore offers no cover. The usage-policy drop keys on the session cwd (backfill.js:215), which for a sandboxed Desktop session is inside the container, so no ancestor .hypignore of the user's can ever reach it. The entrypoint gate is the only control on this root.
  3. The prompt the user answered says something else. The wizard asks Import local claude history now (last N days)? with the summary Reads local transcripts into the query cache. (src/core/cli/walkthrough.js:206,245). Your own "Not addressed" note about that prompt not mentioning Desktop was written when the scan was confined to the shared tree. It now covers a different application's container.

The ask: make ownership root-derived for the roots you added, so the fail-open stays confined to the scanning client's own tree. Either shape works:

  • skip findDesktop3pProjectsDirs entirely unless the claiming plugin is configured (a couple of lines, since ctx.entrypointOwners already carries configured); or
  • pass the extra roots with an owner attached, and have the classifier treat any session found under one as owned by @hypaware/claude-desktop regardless of its entrypoint value.

Either way please add the negative test the suite is missing (a 3p sandbox session with an absent or unclaimed entrypoint, Desktop unconfigured, asserted gated), and update LLP 0140#fail-open-on-unknown to say the fail-open applies to the scanning client's own tree, not to a root another client owns. The existing tests in test/plugins/claude-desktop-3p-transcripts.test.js:198,224 only cover the claimed value in both configured states, which is exactly the case that already works.


3. NON-BLOCKING. local-agent names a CLI mode, not a client, so claiming it for Desktop cuts both ways

hypaware.plugin.json:21 claims local-agent for claude-desktop. Unlike claude-desktop and claude-desktop-3p, that string describes how the embedded CLI was run, not who ran it. If anything other than Desktop ever writes local-agent into the shared ~/.claude/projects (Claude Code gaining a local agent mode is the obvious candidate), then with Desktop unconfigured those sessions are silently gated out of hyp backfill claude, and with Desktop configured they are attributed to claude-desktop.

Visible in sessions_gated and entrypoint_not_configured, so not silent to an operator reading logs, but it is under-capture of Claude Code history caused by a Desktop claim. The root-aware fix in finding 2 resolves this for free: local-agent inside the Claude-3p container is Desktop's, local-agent in the shared tree is not. Worth choosing that shape for this reason as well.


4. NON-BLOCKING. The live path re-walks the 3p container on every Desktop exchange

loadTranscript calls findDesktop3pProjectsDirs on each primary-tree miss (transcripts.js:165-166), and the projector now passes homeDir on every projection (projector.js:228). For an attached Desktop every exchange is a primary miss by construction (that is the premise of the commit), so each captured exchange triggers a fresh depth-6 recursive readdirSync sweep of the container, then a walkJsonlFiles pass over each discovered root. The number of per-session sandbox homes grows monotonically with conversations, which your own comment at backfill.js:103-105 says outright.

Cost is on the daemon's projection path rather than the user's LLM call, and a machine with no container pays two ENOENT stats, so this is not urgent. The ask is to bound it: cache the discovered roots for the projector's lifetime (or a short TTL), the same "resolve fresh per run" rule you already applied to backfill, rather than resolving per exchange.


5. NON-BLOCKING, smaller

  • backfill.js:177: loadAgentMeta({ projectsDir }) is still primary-tree only, so the 3p sandbox sessions this PR imports carry no spawned_by_tool_use_id provenance for subagent rows, unlike every other backfilled session. Please either pass the extra roots or record it as a known gap in LLP 0140.
  • src/core/cli/dispatch.js:866: the seam's catch {} swallows every failure with no record, and the only log line is on the success path (:858). A seam activation that fails leaves the user at "unknown command" with nothing in the logs saying an activation was attempted and why it failed. Given this repo's log-driven-development rule, please log in the catch with an error_kind.
  • src/core/cli/dispatch.js:812: providerByCap.set(cap, m.name) keeps the last writer, and inactive manifests are iterated after the active ones, so an inactive config-selected provider overwrites an already-active one. Three bundled capabilities have two providers each (hypaware.encoder, hypaware.blob-store, hypaware.completion), so the closure's membership depends on iteration order. Benign today (both candidates are config-selected, so boot would have activated them anyway), but please prefer an already-active provider, or collect all providers instead of one.
  • Testability of the new platform gate.index.js:133 and :145 call runInstall/runVerify with no platform, so the gate reads process.platform with no injection point above the unit-test seam. On Linux CI that means no test can drive the wizard seam through to the applying path: the one integration test that exercises the seam uses --print-commands (test/core/command-dispatch.test.js:900-949), the flag that skips consent by design. The unit coverage of consent is genuinely good, so this is a note rather than a demand, but the seam-to-prompt path is now only verifiable by hand on a Mac.

6. What was verified and holds

The credential and plist gate still cannot be reached without consent.runInstall refuses off-platform before anything else (install.js:126), then the ephemeral-listen refusal, then the consent gate, and every step above it is a read. --yes is still only ever set on the command's own argv and no in-process caller injects it. The wizard passes printCommandsFlag(opts) only. Non-interactive stdin declines (round 2's fix) and a bare enter declines.

f26c921 closes a hole round 2 approved over. At 8c64a56, ticking Desktop in hyp init wrote both plugins into the config, missed dispatch on claude-desktop install, and printed a catch-up hint, so the config said "Desktop configured" and the consent prompt had never been shown. The seam's fresh read fixes the reachability. The negative test (command-dispatch.test.js:951-983, a config listing only local-fs leaves the plugin inactive) pins the opt-in boundary correctly, and the closure ordering (claude-account before claude-desktop) is resolved through resolveDependencies rather than assumed.

The seam does not start capture.activatePlugins only imports the entrypoint and calls activate; sources are registered, never started, and both claude-account and claude-desktop activate with registration only. obsEnv.stateDir is the same stateRoot boot derives, so the helper and credential paths named in the consent text do not diverge between the wizard and a standalone run. Re-entry is idempotent: the freshly pushed plugin lands in activePlugins, so a second seam call finds no owner.

The residue clear is correctly untouched.residueDirPath (install.js:37-40) still points at the nested Claude/Claude-3p path, so the new sibling container holding live session data is never backed up or deleted. That matches the caution in your commit message and the new LLP 0133#attribution note. Good that this was called out explicitly rather than left to a reader.

Gate placement and attribution are still correct. The entrypoint gate sits after the usage-policy ignore drop and before projectedExchangeFromEntries, ownership lookup is an exact Map.get with no path-prefix logic, and sessionEntrypoint reads the unwindowed entries so a time window cannot reopen the gate.

Docs and conventions. No U+2014 on any added line; no semicolons; no @typedef and no inline import('...') types added; all eleven distinct @ref anchors added on this branch resolve, including the three new ones (0139#seam-fresh-activation, 0139#macos-only, 0133#attribution). Living-docs is satisfied per commit, and LLP 0133#attribution recording the layouts as dated per-build observations rather than constants is the right call for a surface that has already drifted once.

Verification run at c01ef0e (own worktree, node_modules linked from the checkout): npm test 2891 pass / 8 fail, exactly the test/core/leave-command.test.js set, confirmed failing identically on origin/master (c551d6e) in a second worktree, so pre-existing and unrelated. npm run typecheck clean. CI on the PR is the authority for green.


Nothing was pushed on this round. Readying and merging remain yours.

@philcunliffephilcunliffe added the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Jul 30, 2026

@philcunliffephilcunliffe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed c01ef0e in review-only mode (neutral:review), so nothing was pushed and no finding was fixed for you. Full round record in the comment above.

Requesting changes on one finding, in the consent class.c01ef0e adds Claude Desktop's Claude-3p container as a transcript scan root (claude/src/backfill.js:106-107, walked at :186), but admission is still decided on the session's entrypointvalue, and an absent or unclaimed value fails open (src/core/backfill/entrypoint_owner.js:69-70). Reproduced against the real provider with the owners map a default install builds (@hypaware/claude configured, @hypaware/claude-desktop not):

imported items: 2 # both from ~/Library/Application Support/Claude-3p/...
sess-drift entrypoint=local-agent-v2 -> client_name: claude
sess-noep no entrypoint field -> client_name: claude
scan_complete files_seen=2 sessions_projected=2 messages_projected=4 sessions_gated=0

Four message rows out of Desktop's private container on a machine that never configured Desktop, filed as Claude Code. Neither trigger is hypothetical: your own LLP 0133#attribution note records the entrypoint value drifting inside one week, and LLP 0140 says attachment and summary records omit the field. .hypignore cannot cover this root either, because the policy drop keys on cwd and a sandboxed session's cwd is inside the container. Unlike master, which never read that directory, this is a new read surface, which is why I am blocking on it rather than filing it as attribution polish.

The ask is small: derive ownership from the root you added rather than from the value found inside it. Either skip findDesktop3pProjectsDirs unless the claiming plugin is configured, or attach the owner to the extra roots so any session under one is Desktop's whatever it is tagged. Please add the missing negative test (3p session, absent or unclaimed entrypoint, Desktop unconfigured, asserted gated) and narrow LLP 0140#fail-open-on-unknown to the scanning client's own tree. That shape also resolves the local-agent claim being a CLI-mode string rather than a client one.

Credit where it is due:f26c921 fixes something round 2 approved over. At 8c64a56 the wizard wrote both Desktop plugins into the config, missed dispatch on claude-desktop install, and printed a catch-up hint, so the config said "configured" while the consent prompt had never been shown. The seam's fresh read makes the gate reachable from the surface it was built for, and the negative test pins the opt-in boundary. f668d81's off-darwin refusal before consent is also a real improvement.

Two things that are yours to decide, not neutral's:

  1. Round-2 residual 1 (a decline leaves both plugins in the config, so LLP 0140's config-membership gate still opens) is not fixed, and c01ef0e widens what it admits: no longer only Desktop-tagged sessions in the user's own tree, but everything in Desktop's container. Closing it still means reversing one of two Accepted decisions.
  2. Round-2 residual 2 is partly addressed. The command now refuses off-darwin, but the picker row still has no platform predicate, so the row is still offered on Linux and still composes both plugins. If the manifest platforms key is out of scope for this PR, say so and it can be closed as scoped-out.

Verified at this head: the credential and plist gate still cannot be reached without consent (platform refusal, then ephemeral-listen refusal, then the prompt, everything above it a read); the seam activates registration only and starts no sources; the residue clear still targets the nested path, so the sibling container holding live session data is never deleted; the gate still sits after the usage-policy drop and before projection. npm test 2891 pass / 8 fail, exactly the leave-command.test.js set, confirmed failing identically on origin/master. npm run typecheck clean. All eleven @ref anchors resolve; no U+2014, no semicolons, no @typedef.

Nothing was pushed. Readying and merging remain yours.

The review on c01ef0e blocked on the container scan failing open: admission
was decided on the session's entrypoint value, so an absent or unclaimed
value (both observed: the field drifted to local-agent-v2 within a week,
and attachment/summary records omit it) imported Desktop's private
Claude-3p container as Claude Code on a machine that never configured
Desktop.
Ownership now derives from the root the session was found under. The walk
tags container files, and classifyContainerSession keys admission on the
owning plugin's config membership alone: unconfigured, or not installed at
all, closes the gate, since failing closed over a foreign container drops
no history the user opted into (master never read that directory).
classifyTranscriptEntrypoint and LLP 0140#fail-open-on-unknown are
narrowed to the scanning client's own tree, and a new
LLP 0140#container-root-owns records the container rule.
With the root deciding, the manifest's local-agent claim (a CLI-mode
string, not a client marker) is dropped; Desktop keeps its shared-tree
claims. Negative tests pin the review's repro: 3p sessions with absent and
unclaimed entrypoints gate off while Desktop is unconfigured, and an
absent owners map reads nothing from the container.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullenbgmcmullen removed the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Jul 30, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 4 - 49a89d7 (review-only, neutral:review)

Verdict: findings, none of them blocking and none of them about consent.

The blocking finding from round 3 is FIXED, and fixed in the shape that was asked for. Ownership of the Claude-3p container now derives from the root that was walked, not from the entrypoint value inside the file, so a drifted or absent tag cannot open the gate. I reproduced round 3's exact scenario against this head rather than reading for the fix, and it now imports nothing. Detail and evidence below.

Since this is a review-only round nothing was pushed and nothing was fixed for you. Four carried-forward items remain, one of which (the per-exchange container sweep) I would like to see bounded before this lands, plus one new LOW that the new classifier introduces. All are small and none touch the consent boundary.


1. The round-3 BLOCKING consent finding: FIXED

What was wrong at c01ef0e: the 3p container was walked unconditionally and admission was decided on sessionEntrypoint(...), so #fail-open-on-unknown applied to a root that belongs entirely to Claude Desktop. Reproduced then as 2 sessions and 4 message rows out of Desktop's private container with @hypaware/claude-desktop never configured, filed as client_name: "claude", sessions_gated=0.

Reproduced at this head, same conditions. Real createClaudeBackfillProvider, owners map built by resolveEntrypointOwners over the real bundled catalog (discoverBundledPlugins plus buildPluginCatalog, loaded and excluded) with @hypaware/claude configured and @hypaware/claude-desktop installed and not configured. Two transcripts: one tagged local-agent-v2 in the sibling layout, one carrying no entrypoint field at all in the nested layout.

owners: cli->@hypaware/claude(cfg) sdk-cli->@hypaware/claude(cfg)
claude-desktop->@hypaware/claude-desktop(uncfg)
claude-desktop-3p->@hypaware/claude-desktop(uncfg)
imported items: 0
scan_complete: files_seen=2 sessions_projected=0 messages_projected=0 sessions_gated=2

Was 2 sessions / 4 rows / gated=0, is now 0 / 0 / gated=2. Both gate lines name owner_plugin=@hypaware/claude-desktop, so an operator can see who the skipped sessions belonged to.

It is structural, not a patch a drifted value slips past.walkRootsWithOrigin (hypaware-core/plugins-workspace/claude/src/backfill.js:317-334) yields the shared tree with inContainer: false and extraProjectsDirs with inContainer: true, and extraProjectsDirs has exactly one producer (findDesktop3pProjectsDirs at :108). classifyContainerSession (src/core/backfill/entrypoint_owner.js:100-110) never reads the entrypoint at all: it takes no such parameter. So there is no value a transcript can carry that reaches the admission decision for a container session. I checked the whole matrix:

roottagDesktopresult
containerlocal-agent-v2 (drifted)unconfiguredgated
containerabsentunconfiguredgated
containerlocal-agent (current build)unconfiguredgated
containerlocal-agentconfiguredimported as claude-desktop
containerabsentconfiguredimported as claude-desktop
sharedbrand-new-modeunconfiguredimported as claude (fail-open preserved)
sharedclaude-desktopunconfiguredgated

The fail-open direction is preserved exactly where it was argued for and inverted exactly where it was dangerous. classifyContainerSession also fails closed on an absent or empty owners map, which is the right asymmetry and the one the doc now states: master never read that container, so degrading toward master for a container root means reading nothing.

The negative test exists and is load-bearing.test/plugins/claude-desktop-3p-transcripts.test.js:229-259 covers both triggering cases (absent field, unclaimed value) with Desktop unconfigured, asserting zero items, two gate lines, owner_plugin, and sessions_gated=2. :262-281 covers the no-owners-map case. I checked it genuinely fails without the fix rather than trusting it: reverting :246-248 to the unconditional classifyTranscriptEntrypoint call in my own throwaway worktree turned that file from 8/8 pass to 4 pass / 4 fail, the new test failing on 1 !== 0. Restored immediately; nothing was committed.

LLP 0140#fail-open-on-unknown is narrowed as asked, and the new #container-root-owns anchor carries the reasoning (drift, attachment and summary records omitting the field, and why failing closed here drops no history the user opted into). The Consequences list now records the per-root degradation asymmetry explicitly. All four @refs to the new anchor resolve.

The .hypignore observation still holds and the fix is the right answer to it. The usage-policy drop keys on session cwd, which for a sandboxed session is inside the container, so no ancestor .hypignore of the user's can reach these files. The entrypoint gate really is the only control on this root, which is why root-derived ownership was the right shape rather than a wider allowlist.


2. Carried-forward findings from round 3

#FindingStatus
3claude-desktop/hypaware.plugin.json:21 claims local-agent, a CLI modeFIXED
4Live path re-walks the 3p container on every Desktop exchangeStill open
5abackfill.js:179loadAgentMeta is primary-tree onlyStill open
5bdispatch.js:866 seam catch {} swallows every failure silentlyStill open
5cdispatch.js:812providerByCap keeps the last writerStill open

Finding 3, FIXED, and fixed the better way

transcript_entrypoints is now ["claude-desktop", "claude-desktop-3p"]. local-agent is gone, so a non-Desktop local-agent session in the shared tree is no longer gated out or misattributed by a Desktop claim. The test at test/core/backfill-entrypoint-owner.test.js:155-166 pins the new list with the reason inline, and the "every known real-world value is claimed" test was correctly re-scoped to shared-tree values with the exclusion argued rather than just narrowed. LLP 0140:84-89 records the decision. This is exactly the free resolution the root-aware shape offered, and taking it rather than keeping both mechanisms overlapping is the right call.

One residual worth knowing, not an ask: with local-agent unclaimed, if some future Desktop build writes local-agent into the shared tree, those sessions import as client_name: "claude" instead of being gated. Verified (sess-shared-la imports as claude, counted once in unclaimed_entrypoints). That is the documented fail-open inside the user's own tree rather than a foreign container, so it is the accepted trade. Flagging only because the argument in LLP 0140:84-89 is stated one direction (a future CLI mode misfiled as Desktop) and this is the other direction (a future Desktop mode misfiled as Claude Code).

Finding 4, still open. MEDIUM, and the one I would most like closed before merge

transcripts.js:182 still calls findDesktop3pProjectsDirs(opts.homeDir) inside loadTranscript on every primary-tree miss, and projector.js:228 still passes homeDir on every projection. For an attached Desktop every exchange is a primary miss by construction, which is the premise of c01ef0e, so each captured exchange still triggers a fresh depth-6 recursive readdirSync sweep of a container whose per-session sandbox homes grow monotonically with conversations. Your own comment at backfill.js:104-107 says that growth outright.

Unchanged since round 3 with no pushback on the thread, so I am carrying it as an ask rather than closing it: cache the discovered roots for the projector's lifetime, or a short TTL, the same "resolve fresh per run" rule you already applied to the backfill provider at :108. If you would rather land it and bound it separately, say so on the thread and it can be closed as scoped-out.

Finding 5a, still open

backfill.js:179 is still loadAgentMeta({ projectsDir }), primary-tree only, so the container sessions this PR imports carry no spawned_by_tool_use_id provenance for subagent rows, unlike every other backfilled session. The round-3 ask offered either branch, pass the extra roots or record it as a known gap in LLP 0140. Neither was taken: grepping spawned_by_tool_use_id and agentMeta across llp/0140 and llp/0133 returns nothing. Given the living-docs rule in CLAUDE.md, the doc branch is a one-line ask and enough.

Findings 5b and 5c, still open

dispatch.js:866 is still a bare catch {} with the only log on the success path at :858. This is code the branch added, and this repo's log-driven-development rule asks that a failure identify the broken step; a seam activation that throws currently leaves the user at "unknown command" with nothing recorded saying an activation was even attempted. Please log in the catch with an error_kind.

dispatch.js:812 is still providerByCap.set(cap, m.name) over active manifests followed by inactive ones, so an inactive config-selected provider overwrites an already-active one and closure membership depends on iteration order. Benign today for the reason given last round, but three bundled capabilities have two providers each. Prefer an already-active provider, or collect all of them.


3. NEW finding. LOW. Container admission reads configured out of the entrypoint-owners map, so it silently depends on a declaration LLP 0140 now says is irrelevant for containers

src/core/backfill/entrypoint_owner.js:100-108 finds configured by scanning owners.values() for a matching plugin. That map only ever contains entries for plugins that declare at least one transcript_entrypoints value (resolveEntrypointOwners:29-45 iterates descriptor.transcriptEntrypoints ?? []). So container admission for a root-owned session is still gated behind Desktop declaring a value.

Reproduced: Desktop configured, isConfigured returning true for everything, but the claude-desktop client descriptor declaring no transcript_entrypoints:

owners keys: [ 'cli', 'sdk-cli' ]
imported: 0
claude.backfill.entrypoint_not_configured session_id=sess-a entrypoint=local-agent
owner_plugin=@hypaware/claude-desktop

A configured Desktop importing none of its own container history, reported as entrypoint_not_configured. It fails closed, so this is under-capture and not a consent problem, and it cannot happen with the manifest as shipped since Desktop still declares two values. What makes it worth a line is that LLP 0140:84-92 now argues those values decide nothing for container sessions, and that the current build's value is deliberately unclaimed. A future maintainer following that reasoning to its conclusion and dropping the now-vestigial claim would silently switch off Desktop backfill, and no test pins the case.

The ask: derive container configured from the effective plugin list rather than from the owners map. resolveOwnersForRun already holds the isConfigured predicate it built the map with, so threading it onto BackfillRunContext beside entrypointOwners, or passing it into classifyContainerSession, decouples root ownership from value declaration completely. A test that a configured Desktop declaring no entrypoints still imports its container would pin it.


4. What was verified and holds at this head

Nothing captures or records before consent, and the credential and plist gate cannot fail open. Re-traced rather than carried over. runInstall refuses off-darwin first (install.js:126), then the ephemeral-listen refusal, then the consent gate at :130-149, and every step above it is a read. --yes is still only ever read off the command's own argv and no in-process caller injects it; the wizard seam passes printCommandsFlag(opts) only. Non-TTY stdin declines, a bare enter declines. This commit touches none of that path.

Only two things read the container, and both are now correct. Grepping findDesktop3pProjectsDirs and claudeDesktop3pSessionRoots gives exactly two consumers: the backfill provider, now root-gated, and loadTranscript's live fallback. The live fallback is identity enrichment for an exchange that already reached the gateway: the row's content comes from the wire, and the transcript supplies uuid/parentUuid and the entrypoint column. A container file is only opened when a gateway exchange's session id matches a file name inside it, which means it is that session. So the live path reading the container without an ownership check is not a content path, and finding 4 stays a resource ask rather than a consent one.

Gate placement is unchanged and still correct. The entrypoint gate sits after the usage-policy ignore drop and before projectedExchangeFromEntries, so a gated session produces no row and the policy machinery still wins. sessionEntrypoint still reads the unwindowed entries, so a time window cannot reopen the gate. Container sessions skip the unclaimed_entrypoints tally because they continue first, which is right: a container value is not a property of the install. The per-session gate log still carries the raw entrypoint, so the drift signal is not lost.

No re-import trap. Gated sessions write no watermark: the only state read is the session-context join (createSessionContextReader), and the window comes from resolveWindow(ctx). Configuring Desktop later imports the previously gated sessions rather than finding them already seen.

Attribution stays consistent. One owned.clientName feeds both projectedExchangeFromEntries and the item, so the projected exchange and the row cannot disagree. verify.js:90 still tells the user rows land under entrypoint 'local-agent', which remains true: the entrypoint column keeps the raw transcript value while client_name becomes claude-desktop. Consistent, not drifted.

Conventions clean. No U+2014 anywhere in the diff. No semicolon-terminated added JS lines. No @typedef and no inline import('...') types added; the one new type import went through the @import header (transcripts.js:31). All twelve distinct @ref anchors on the branch resolve to real <a id=...> targets, including the four new 0140#container-root-owns sites. Living-docs satisfied: the LLP 0140 edit is in the same commit as the code that changed its meaning, and DESKTOP_3P_CONTAINER_OWNER living beside the path list with the "two facts are one piece of knowledge" note is a good call.

Verification run at 49a89d7 (own detached worktree, node_modules linked from the checkout): npm test 2897 pass / 8 fail, exactly the pre-existing test/core/leave-command.test.js set, no changed file participating. npm run typecheck clean. Smokes backfill_claude_fixture, hypignore_capture_drop, walkthrough_backfill_client_history, local_only_export_withhold all ok. test/plugins/claude-desktop-3p-transcripts.test.js plus test/core/backfill-entrypoint-owner.test.js: 27/27. CI on the PR is green (test and typecheck, Node 22 and 24) and is the authority.

The two round-2 residuals are unchanged and remain maintainer decisions, not asks to you: that declining the Desktop consent prompt does not stop transcript import (which the container gate now materially narrows, since a decline leaves Desktop unconfigured only in the sense LLP 0139 allows), and that the picker row has no platform predicate.


Summary of asks

One commit closes all of them, and none is a redesign:

  1. Bound the live container sweep (transcripts.js:182), or say it is scoped out.
  2. Either pass the container roots to loadAgentMeta or record the subagent-provenance gap in LLP 0140 (backfill.js:179).
  3. Log in the seam's catch with an error_kind (dispatch.js:866).
  4. Prefer an already-active capability provider (dispatch.js:812).
  5. Derive container configured from the plugin list rather than the owners map (entrypoint_owner.js:100).

For the record on the round that mattered: the consent defect was the hard one, the fix took the structural option rather than the cheap one, it came with the negative test that was missing and with the doc narrowed to match, and it resolved the local-agent claim finding as a side effect instead of leaving two mechanisms overlapping. That is the right way to close a blocking consent finding.

Nothing was pushed on this round. Readying and merging remain yours.

@philcunliffephilcunliffe added the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Jul 30, 2026

@philcunliffephilcunliffe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: changes-requested at 49a89d7 (review-only, neutral:review)

The blocking consent finding from round 3 is FIXED and verified by reproduction. Round 3's exact scenario (a drifted local-agent-v2 tag plus a transcript with no entrypoint at all, in the Claude-3p container, @hypaware/claude-desktop installed and not configured) went from 2 sessions / 4 message rows imported as client_name: "claude" to 0 imported, sessions_gated=2. The fix is structural: classifyContainerSession takes no entrypoint parameter, so no value a transcript can carry reaches the admission decision for a container session. The missing negative test exists and is load-bearing (reverting the classifier choice turns that file from 8/8 to 4 pass / 4 fail), and LLP 0140#fail-open-on-unknown is narrowed to the scanning client's own tree with the new #container-root-owns anchor carrying the reasoning.

Nothing blocking remains and nothing that remains touches consent. I am requesting changes only because five small asks are open, four of them carried from round 3 without being addressed or pushed back on:

  1. transcripts.js:182 (MEDIUM). The live path still re-walks the growing 3p container on every Desktop exchange, which is every exchange by construction. Cache the discovered roots for the projector's lifetime, or say it is scoped out and it can be closed.
  2. backfill.js:179 (LOW).loadAgentMeta is still primary-tree only and the gap is still not recorded in LLP 0140. Either branch of the round-3 ask closes it.
  3. dispatch.js:866 (LOW). The seam's catch {} still swallows every failure with no log, against this repo's log-driven-development rule, on code this branch added.
  4. dispatch.js:812 (LOW).providerByCap still keeps the last writer, so closure membership depends on iteration order.
  5. src/core/backfill/entrypoint_owner.js:100 (LOW, new). Container admission reads configured out of the entrypoint-owners map, which only holds plugins that declare transcript_entrypoints. Reproduced: a configured Desktop that declares no entrypoints imports none of its own container history, logged as entrypoint_not_configured. It fails closed so it is not a consent risk, but it couples root-based admission to a value declaration LLP 0140 now says decides nothing for containers, which is a live invitation to a silent regression.

The two round-2 residuals are unchanged and remain your decisions rather than asks to the contributor: a declined consent prompt not stopping transcript import, and the picker row having no platform predicate.

CI green at 49a89d7 (test and typecheck, Node 22 and 24). Local npm test 2897 pass / 8 fail, exactly the pre-existing leave-command.test.js set; typecheck clean; four smokes ok; the two changed test files 27/27.

Full evidence in the round-4 review record above. Nothing was pushed on this round. Readying and merging remain yours.

1. Bound the live 3p container sweep: loadTranscript's fallback now
resolves the Desktop container roots through a TTL cache
(createDesktop3pDirsCache) with a forced re-sweep on a lookup miss,
so per-exchange enrichment stops re-walking a container that grows
with every conversation while never finding less than the uncached
walk did.
2. Container subagent provenance: the backfill scans agent-meta
sidecars in the 3p sandbox trees too, so container sessions carry
spawned_by_tool_use_id like every other backfilled session.
3. The dispatch seam's catch now logs dispatch.seam_activate_failed
with an error_kind instead of swallowing the failure silently.
4. providerByCap is first-wins with active manifests first, so an
inactive capability provider can no longer shadow an already-active
one by iterating later.
5. Container admission derives configured from the runner's
plugin-list predicate (BackfillRunContext.isPluginConfigured), not
from the entrypoint-owners map, so root ownership no longer depends
on Desktop declaring transcript_entrypoints values LLP 0140 calls
irrelevant for containers. Absent predicate still fails closed.
LLP 0140 records the predicate split; LLP 0133's stale "claims all
three values" sentence is corrected and now notes the bounded live
sweep. New tests pin each behavior, including the reviewer-requested
"configured Desktop declaring no entrypoint values still imports its
container".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullen

Copy link
Copy Markdown
ContributorAuthor

Round-4 asks: all five closed in 65b238f

One commit, as suggested. Per ask:

  1. Live container sweep bounded (transcripts.js). Root discovery in loadTranscript's 3p fallback now goes through createDesktop3pDirsCache, a 30s-TTL cache keyed by home dir, rather than a fresh depth-6 sweep per primary miss. A miss inside the cached list forces one re-sweep, so a sandbox home created after the cached sweep (a brand-new Desktop session) is still found - the cache is invisible to correctness, it only bounds the steady state where the session is found. ttlMs/now are injectable for tests; two tests pin the TTL semantics and the refresh-on-miss behavior. LLP 0133#attribution records the bound.

  2. Container subagent provenance - took the code branch rather than the doc branch. runClaudeBackfill now merges loadAgentMeta over the 3p roots (primary tree wins a collision), so container sessions carry spawned_by_tool_use_id like every other backfilled session. Test: backfill stamps subagent provenance from sidecars inside the 3p container.

  3. Seam catch logs.activateSeamCommandPlugins' catch now emits dispatch.seam_activate_failed with error_kind: 'seam_activation_failed', command_name, and the error message.

  4. Active provider preferred.providerByCap is first-wins with active manifests iterated first, so an inactive config-selected provider can no longer overwrite an already-active one.

  5. Container configured decoupled from value declarations.classifyContainerSession now takes the runner's plugin-list predicate ((plugin) => boolean) instead of scanning the owners map; resolveOwnersForRun returns the predicate it built the map with and the runner threads it onto run and plan contexts as BackfillRunContext.isPluginConfigured (kernel-types documented as optional, absent fails closed for containers). The exact scenario from the finding is pinned twice: unit (classifyContainerSession is independent of entrypoint value declarations) and end-to-end through the real provider (backfill imports the container for a configured Desktop that declares no entrypoint values). LLP 0140#container-root-owns now states the predicate split and why the map lookup was wrong.

Also fixed while in there: LLP 0133's "the Desktop manifest claims all three observed entrypoint values" had gone stale after the round-3 local-agent unclaim; it now says two shared-tree values with the container-root reasoning.

Verification:npm test 2909 pass / 2 fail - blob-store (fails identically on clean HEAD, environment) and one GC-timing flake in query-sql-budget that passes in isolation; neither file participates in this change. npm run typecheck clean. Smokes backfill_claude_fixture, gateway_claude_capture, hypignore_capture_drop, walkthrough_backfill_client_history, local_only_export_withhold all ok. The two touched test files: 32/32. All @ref anchors in changed files resolve.

🤖 Generated with Claude Code

@bgmcmullenbgmcmullen removed the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Jul 30, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 5 - 65b238f (review-only, neutral:review)

Verdict: approved. All five round-4 asks are closed in one commit, including the MEDIUM I said I most wanted bounded, and the round-3 blocking consent finding is still fixed. I re-verified each one by reproduction against this head rather than by reading the diff, and mutation-tested two of the new tests to confirm they are load-bearing. One LOW remains plus one performance nit, neither blocking; they are written up so the maintainer can weigh them, not as another round of asks.

Worth saying plainly at round five: this PR closed a HIGH consent defect with the structural fix rather than the cheap one, then closed every follow-up ask in a single commit, with tests that fail when the fix is reverted and LLP edits landing in the same commit as the code. That is the shape a review is supposed to produce.


1. The round-3 HIGH consent finding: STILL FIXED

Re-ran round 3's exact scenario at this head with the real createClaudeBackfillProvider, the real bundled catalog (loadClientDescriptors over a fresh state dir), and the real resolveEntrypointOwners, with @hypaware/claude configured and @hypaware/claude-desktop installed and not configured. Two container transcripts: one tagged with a drifted local-agent-v2 in the sibling layout, one carrying no entrypoint field at all in the nested layout, plus one genuine cli session in the user's own tree as a control.

owners: cli->@hypaware/claude(cfg) sdk-cli->@hypaware/claude(cfg)
claude-desktop->@hypaware/claude-desktop(uncfg)
claude-desktop-3p->@hypaware/claude-desktop(uncfg)
isPluginConfigured("@hypaware/claude-desktop") = false
imported items: 1
native_id=sess-cli client_name=claude messages=2
source=~/.claude/projects/repo/sess-cli.jsonl
scan_complete: files_seen=3 sessions_projected=1 messages_projected=2 sessions_gated=2
GATE session=sess-drift entrypoint=local-agent-v2 owner_client=claude-desktop owner_plugin=@hypaware/claude-desktop
GATE session=sess-noep entrypoint=undefined owner_client=claude-desktop owner_plugin=@hypaware/claude-desktop

At c01ef0e that was 2 sessions and 4 message rows out of Desktop's private container filed as client_name: "claude", sessions_gated=0. It is still 0 imported and sessions_gated=2, and the user's own cli session still imports normally, so the gate has not overshot into dropping real history.

Both directions of the matrix still hold:

  • Desktop configured: all three sessions import, and the two container ones are attributed client_name: claude-desktop while keeping their raw entrypoint (local-agent-v2, absent) in the entrypoint column.
  • No owners map and no predicate at all (a degraded catalog, or an older host): the container fails closed, sessions_gated=2, only the shared-tree session imports. That is the right asymmetry, and it is now enforced by the predicate rather than by a map lookup.

65b238f changes no file under claude-desktop/src/, so round 4's trace of the credential and plist gate (platform refusal, then ephemeral-listen refusal, then the consent prompt, everything above it a read) is untouched and still stands.


2. The five round-4 asks

#AskStatus
1Bound the live container sweep (transcripts.js)FIXED, measured
2Container subagent provenance (backfill.js:183-188)FIXED (code branch)
3Log in the seam's catch (dispatch.js:876)FIXED
4Prefer an already-active capability provider (dispatch.js:817)FIXED
5Container configured from the plugin list (entrypoint_owner.js:108)FIXED, mutation-tested

1. The MEDIUM sweep, FIXED, and the numbers are good

createDesktop3pDirsCache (transcripts.js:131), consumed by the live fallback at :233. Measured rather than assumed: a synthetic container with 200 sandbox homes, counting fs.readdirSync calls over 10 live exchanges of an attached-Desktop session.

uncached (the pre-65b238f live path): 602 readdirSync per exchange
cached, steady state (session found): ~9 readdirSync per exchange

Root discovery drops out of the steady state entirely; what is left is the per-dir session lookup, which is unavoidable and independent of this fix. The growth cliff is gone: the sweep no longer scales with conversation count on every exchange.

And the cache is invisible to correctness, which is the part I checked hardest. A sandbox home created after the cached sweep is still found through the forced re-sweep on a miss, verified end to end through the real loadTranscript:

loadTranscript(sess-cont) entries=2 # warms the shared cache
loadTranscript(sess-newest, created after the warm) entries=2 <- refresh-on-miss

That behavior is pinned. Deleting the refresh-on-miss block in my own throwaway worktree turned test/plugins/claude-desktop-3p-transcripts.test.js from 12/12 to 11 pass / 1 fail on loadTranscript finds a sandbox home created after the root cache was primed. Restored immediately; nothing was committed.

2. Container subagent provenance, FIXED

loadAgentMeta now merges over the 3p roots with primary-tree-wins on a collision (backfill.js:183-188), and a test covers a sidecar inside the container. Taking the code branch over the doc branch was the better call. See the new LOW in section 3, which is about where that merged map is visible, not about the fix itself.

3. Seam catch, FIXED

dispatch.js:876 emits dispatch.seam_activate_failed with error_kind: 'seam_activation_failed', command_name, and the message. A throwing activation now leaves a record that the seam ran, which is what the log-driven-development rule asks for.

4. providerByCap, FIXED

dispatch.js:817 is if (!providerByCap.has(cap)) over active manifests first, so closure membership no longer depends on iteration order and an inactive config-selected provider cannot shadow an already-active one.

5. Container configured, FIXED, and the exact finding scenario is now pinned

classifyContainerSession(containerOwner, isPluginConfigured) (entrypoint_owner.js:108) takes the runner's predicate; resolveOwnersForRun returns the predicate it built the map with, and the runner threads it onto both run and plan contexts. Reproduced round 4's scenario, Desktop configured but its client descriptor declaring notranscript_entrypoints:

owners keys: cli, sdk-cli # Desktop declares nothing, so it is absent from the map
imported: 1 sess-cont=claude-desktop
gated=0 projected=1

Was imported: 0 reported as entrypoint_not_configured. Root ownership is now fully decoupled from value declarations, which is what LLP 0140#container-root-owns says it should be, and the doc states the split and why the map lookup was wrong.

The new tests are load-bearing, checked rather than trusted: reverting classifyContainerSession to the owners-map scan in a throwaway worktree took the two touched test files from 32/32 to 27 pass / 5 fail, including the reviewer-requested backfill imports the container for a configured Desktop that declares no entrypoint values. Restored; nothing committed.


3. NEW finding. LOW, not blocking. Container reads are unconditional, so container agent metadata is visible to shared-tree sessions

hypaware-core/plugins-workspace/claude/src/backfill.js:108 and :183-188.

Root discovery and the container walk are unconditional: the provider resolves the 3p roots and reads their files before the gate decides anything. That was already true at round 4 for transcripts, and I accepted it then because reading is not recording. What is new at this head is that the sidecar merge puts container-derived values into a map that shared-tree sessions also read.

Container files really are opened on a Desktop-installed-but-unconfigured machine, before the gate rejects the sessions:

imported items: 0 sessions_gated=1
container files opened:
READ ~/Library/.../Claude-3p/.../sess-secret/subagents/agent-COLLIDE.meta.json
~/Library/.../Claude-3p/.../sess-secret.jsonl

And the consequence, which is the part worth a line. agentMeta is one map for the whole run keyed by agent id, so a shared-tree subagent line whose agent_id matches a container sidecar picks up the container's tool_use_id. Desktop unconfigured, container session correctly gated, and yet:

item sess-cli client=claude
role=assistant agent_id=a1b2 spawned_by_tool_use_id=toolu_FROM_DESKTOP_CONTAINER
sessions_gated=1 projected=1

Why this is LOW rather than a blocker: it needs an agent-id collision across two trees, the comment at :181-182 already states the uniqueness assumption it rests on, and what crosses is an opaque tool-use id, not prompt or response content. I looked specifically for content from a gated container reaching a row, a log, or the cache, and there is none.

The ask, for whenever you are next in this file, and one change covers both halves: skip the container entirely when the owner is not configured. ctx.isPluginConfigured is now on the run context, so an early ctx.isPluginConfigured?.('@hypaware/claude-desktop') !== true check before findDesktop3pProjectsDirs at :108 would mean an unconfigured Desktop's private directory is never opened, never parsed, and never contributes metadata, and it would also remove the container walk cost from every backfill on a Desktop-installed-but-unconfigured machine. The narrower alternative is to keep container agent-meta in its own map that only container sessions read. Either is a few lines; neither is a redesign.


4. One nit on the cache, for the record

The refresh-on-miss is unconditional, so an exchange whose session is genuinely not in the container pays the cached walk, then a forced sweep, then the walk again. On the same 200-sandbox container that is ~1400 readdirSync per such exchange against ~1000 for the old uncached path, so the miss path is roughly 1.4x more expensive than before while the hit path is roughly 70x cheaper. That is a good trade, and the miss path costs nothing on a machine with no container (two failed readdirSync), so I would leave it as is. Worth knowing only if a Desktop-attached host ever sees sustained shared-tree misses, where a short negative-lookup memo would flatten it.


5. What else was checked at this head

  • Gate placement unchanged and still correct. The entrypoint gate is still after the usage-policy ignore drop and before projectedExchangeFromEntries, so a gated session produces no row and the policy machinery still wins. sessionEntrypoint still reads the unwindowed entries, so a time window cannot reopen the gate.
  • Attribution stays single-sourced. One owned.clientName feeds both the projected exchange and the item's client_name, so they cannot disagree.
  • Master drift.65b238f merges into current master with no conflicts. The merged tree runs npm test at 2975 pass / 8 fail, exactly the pre-existing test/core/leave-command.test.js set, confirmed identical on a pristine origin/master worktree with the same node_modules link (2917 pass / 8 fail, the same eight names). npm run typecheck is clean on the merged tree. The new ref-hygiene gate passes on the merge: every @ref resolves to a live LLP document and one of its anchors, no @ref annotation separates its gloss with an em dash, and the ignore markers hide illustrative annotations without hiding live ones are all ok, so no @ref on this branch trips it. No overlap with the landed Codex work: this branch touches no src/core/codex/** file, so the shared session_meta reader (LLP 0150) and the body-client_metadata lineage (LLP 0151) are untouched. No LLP number collision: 0139 and 0140 here, 0141 onward on master.
  • Conventions clean. No U+2014 on any added line. No semicolon-terminated added JS lines (the two grep hits are semicolons inside prose comments). No @typedef, no inline import('...') types. Living docs satisfied: the LLP 0140 and LLP 0133 edits ride the same commit as the code that changed their meaning, including the correction of 0133's stale "claims all three observed entrypoint values" sentence.
  • Verification run at 65b238f (own detached worktree, node_modules linked from the checkout): npm test 2902 pass / 8 fail, the same leave-command.test.js set, no changed file participating. npm run typecheck clean. Smokes backfill_claude_fixture, gateway_claude_capture, walkthrough_backfill_client_history, hypignore_capture_drop, local_only_export_withhold all ok. CI is green on the PR (test and typecheck, Node 22 and 24) and is the authority.

6. Unchanged maintainer decisions, not asks to the contributor

Both are carried verbatim from round 2 and neither is affected by this commit. Listed so the record is complete, not to reopen them.

  1. A declined Desktop consent prompt does not stop transcript import, because LLP 0139 deliberately leaves the plugins in the config after a decline and LLP 0140 keys the gate on config membership. Closing it means reversing one of two Accepted decisions.
  2. The picker row has no platform predicate, so the row is still offered off macOS. The command now refuses off-darwin before mutating anything, which is the loud half of the fix.

Related, and already disclosed by the author in the PR body: the wizard's backfill prompt reads Import local claude history now, and with Desktop configured that run also reads Desktop's container. One clause in that string (walkthrough.js:245) would be an honest improvement whenever someone is next in there.

Nothing was pushed on this round. Readying and merging remain yours.

@philcunliffephilcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 30, 2026

@philcunliffephilcunliffe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approved at 65b238f (review-only, neutral:review)

Round 5 reviewed 65b238f. All five round-4 asks are closed in one commit, and the round-3 blocking consent finding is still fixed.neutral:changes-requested is cleared and this supersedes the four earlier change requests on the thread.

  • HIGH consent finding: still fixed. Round 3's exact scenario re-run at this head against the real provider and the real bundled catalog (drifted local-agent-v2 tag plus an entrypoint-less transcript in the Claude-3p container, Desktop installed and unconfigured) imports 0 items with sessions_gated=2, while the user's own cli session still imports normally. A degraded run with no owners map and no predicate at all still fails closed for the container. Evidence in the round record above.
  • The MEDIUM is genuinely bounded, measured not assumed. Against a 200-sandbox container, live steady-state root discovery went from 602 readdirSync per exchange to about 9, and the refresh-on-miss keeps the cache invisible to correctness (verified end to end, and pinned by a test that fails when the block is deleted).
  • The three remaining LOWs are closed too: the seam's catch now logs with an error_kind, providerByCap is first-wins with active manifests first, and container configured comes from the runner's plugin-list predicate rather than the entrypoint-owners map. Reverting that last one fails 5 tests, so the new coverage is load-bearing.
  • Merges into current master with no conflicts. Merged tree: npm test 2975 pass / 8 fail, exactly the pre-existing leave-command.test.js set that fails identically on pristine origin/master; typecheck clean; the new llp-ref-hygiene gate passes, so no @ref on this branch trips it. No overlap with the landed Codex session_meta or lineage work.

One LOW left, deliberately not blocking, for whenever this file is next open. The 3p container is read unconditionally before the gate, and the new agent-meta merge puts container-derived values in a map that shared-tree sessions also read, so a shared-tree subagent row whose agent_id collides with a container sidecar picks up the container's tool_use_id (reproduced). No content crosses, and it needs an id collision, but ctx.isPluginConfigured is now on the run context, so an early check before findDesktop3pProjectsDirs (claude/src/backfill.js:108) would mean an unconfigured Desktop's container is never opened at all. Detail in the record.

The two round-2 residuals are unchanged and remain yours rather than asks to the contributor: a declined consent prompt not stopping transcript import, and the picker row having no platform predicate.

Nothing was pushed on this round. Readying and merging remain yours.

@philcunliffe
philcunliffe merged commit 5ba5b64 into masterJul 30, 2026
8 checks passed
@philcunliffe
philcunliffe deleted the claude-desktop-consent-and-entrypoint-gate branch July 30, 2026 17:10
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approvedneutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)neutral:reviewDelegate this PR to neutral for a review pass (approve or request changes; never merges)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@bgmcmullen@philcunliffe