Uh oh!
There was an error while loading. Please reload this page.
claude-desktop: drop the attach_probe core cannot read or reverse - #445
Conversation
`hyp detach --client claude-desktop` failed on exactly the machines where
`hyp claude-desktop install` had succeeded. The manifest declared a
`json_path` attach_probe over
`/Library/Managed Preferences/com.anthropic.claudefordesktop.plist`, and
core's probe/undo pair `JSON.parse`s the settings file, so detach raised
`MALFORMED_JSON` on the XML plist and `hyp status` surfaced the same parse
failure as a client probe `error`.
The probe was wrong three ways over, any one of them fatal:
- the file is XML, not JSON
- `settings_file` is $HOME-relative by contract, so the absolute
`/Library/...` path re-anchored to `~/Library/...` and never named the
real file
- the plist carries no `marker_record`, and per LLP 0045 Part 3 a
record-less marker is refused rather than half-reversed. Adding one
would not help either: the file is root-owned, so an unprivileged
detach cannot write it back
LLP 0115#no-attach-on-join (Accepted) had already settled this: Desktop
registers no attach_probe, because the LLP 0044 loop needs a reversible
settings-file write and the managed plist is not one. LLP 0135's design
bullet reintroduced one anyway, "reflecting the attribution finding" -
but an attach_probe is not a label, it is the input to
`probeClientAttachFromDescriptor` / `detachClientFromDisk`.
So: remove it. `contributes.client` keeps `skill_dir`/`agent_dir`, the
picker row and the install/verify commands are untouched, and attach on
join stays inert exactly as before (the plugin registers no runtime
`ctx.clients` adapter). `detachClientFromDisk` now returns
`{ changed: false }` at its no-probe guard, so the command is an honest
no-op instead of an error over a file core can neither read nor reverse.
Desktop's state surface remains `claude-desktop verify` and its undo
remains removing the plist with sudo.
No core code changes: the MALFORMED_MARKER guard is left exactly as it
is, since refusing beats a half-reversal.
Co-Authored-By: Claude <noreply@anthropic.com>The new detach test built its plist fixture with buildManagedProfile({
baseUrl, authScheme, models, helperPath }), missing the required
bundleId field ProfileInputs declares, which failed typecheck (TS2345)
on both Node 22 and 24. Import DEFAULT_BUNDLE_ID from profile.js and
pass it through; the test's three assertions (no attach_probe on the
manifest, detach is a changed:false no-op, and the plist never
surfaces as an attach-probe error) are unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>…s stale ask
Two doc-accuracy defects on top of the (correct) attach_probe removal.
1. LLP 0135 {#no-probe} item 1 claimed `hyp detach --client claude-desktop`
raised MALFORMED_JSON "on a machine where install had succeeded", but
item 2 of the same list proves that impossible: `resolveClientSettingsPath`
re-anchors the absolute `/Library/Managed Preferences/...` under $HOME, and
`install` writes only the root-owned absolute path, so the probe never read
the plist at all. Defect 2 masked defect 1: the field symptom was a silent
wrong "not attached", and MALFORMED_JSON is what the probe produces once the
path is corrected (which is the state the new test reproduces). Reworded so
the section is internally consistent and so the reason for removing rather
than repairing the probe is explicit.
2. LLP 0136 T12 still instructed an implementer to declare "an `attach_probe`
reflecting the `entrypoint: claude-desktop-3p` attribution finding" - the
exact stale guidance this change set exists to retire. Corrected to record
no probe, with refs to LLP 0115#no-attach-on-join and LLP 0135#no-probe.
Also narrows the `@ref LLP 0115#no-attach-on-join` gloss in the plugin's
`activate()` JSDoc, which repeated the same overstated field claim.
No code or test behaviour changes.
Co-Authored-By: Claude <noreply@anthropic.com>philcunliffe
commented
Jul 29, 2026
Verdict: approve the change, with two doc-accuracy findings (both fixed and pushed)The central design question, does LLP 0135 legitimately supersede LLP 0115, resolves in this PR's favour. Removing the probe is the correct restoration, not a regression. Two findings, both about the rationale rather than the change: one left the new LLP section internally self-contradictory, and one left the exact stale instruction this change set exists to retire sitting in another LLP. 1. The LLP 0115 vs 0135 question, verified independently
And
0133 then lists exactly three corrections to 0115 ( So a 2. Nothing regresses. Verified from the code, not the PR body
3. Test quality: genuinely red on masterReverting only the manifest hunk to All three bind to the shipped artefacts: the manifest is read from FindingsFinding 1 - Medium - |
Round 1 verified the {#no-probe} rationale itself is now accurate (install
writes only the root-owned absolute /Library/Managed Preferences path, so
the $HOME-re-anchored path the probe read never existed), but the section
sits mid-way through `## hyp claude-desktop install`, so the six-step
`src/install.js` spec and its idempotency paragraph rendered as content of
a `###` subsection titled "No attach probe for Desktop" rather than of the
parent section they document.
Move the subsection to the end of the parent section. The forward link at
"See [no probe for Desktop](#no-probe) below" still resolves, the anchor is
unchanged, and no prose was reworded.
Also names the tracking issue for the `resolveClientSettingsPath`
absolute-path trap (#446) so "tracked on its own" is checkable.
Co-Authored-By: Claude <noreply@anthropic.com>philcunliffe
commented
Jul 29, 2026
Round 2 (final) — verdict: approve, with one LOW doc-structure fix pushedRound 1's two doc corrections hold up. One structural defect in 1. Round 1's rewritten causal claim is TRUE — verified from code, not prose
Conclusion: the causal chain in 2. |
philcunliffe
commented
Jul 29, 2026
Triage verdict: no true blocker. Classification: PREFERENCE (stale doc wording only). Both recorded review rounds (d524e9c, 65e8a31) fixed everything they found in-round; round 2's verdict was Approve with nothing deferred. I independently re-verified the one residual round 2 flagged as non-blocking and cosmetic, rather than trusting the summary:
Nothing else looked wrong: the manifest/index.js diff is a clean, minimal Action taken: corrected the stale Not touching draft/ready state or merging. Marker to follow as a separate edit so a partial failure re-triages rather than being skipped. |
… not the first character
The isAbsolute guard rejects '/etc/passwd' and lets '../../../etc/passwd'
through to exactly the same place. Both land on a file the manifest never
named, which is the whole harm this guard exists to stop, and the escaping
spelling is the one that survives the check:
resolveClientSettingsPath('codex', '../../../etc/passwd', {}, '/Users/hyp')
-> '/etc/passwd' (pre-fix)
.codex/../../../etc/passwd + CODEX_HOME=/tmp/ch -> '/etc/passwd'
It matters more here than in a read-only resolver: this is also the write
side, detachClientFromDisk reads and rewrites whatever it is handed, and
contributes.client is unvalidated, so the value can arrive from a
remotely-installed or org-pushed plugin.
Require the resolved path to stay under the base it resolved against
(settings_file_escapes_base). Each branch checks its own base - $HOME
normally, $<CLIENT>_HOME when set, since the override is exactly a licence
to leave $HOME. A '..' that normalizes away stays legal.
Also drops two overclaims the section had picked up: attach_probe really
does lose expressiveness (it has no absolute sibling, which is why #445
deletes Desktop's probe rather than respelling it), and the one-resolver
argument holds for core but not for the per-plugin attach writers - the
claude plugin hardcodes ~/.claude/settings.json and ignores $CLAUDE_HOME.
And the ClientSettingsPathError JSDoc no longer claims callers avoid
message-text matching when its only consumer flattens to a message.…o it The errored `claude-desktop` row was pushed onto whatever `collectHypAwareStatus` had already produced, and the bundled catalog produces a `claude-desktop` row of its own carrying this very error - the manifest defect #445 removes. So the list held two same-named rows and the JSON assertion's `find` answered from the catalog's, passing for the wrong reason. It stops passing the moment #445 lands: the catalog row loses its error, `find` still returns it first, and `the JSON renderer carries the same client error` fails. Verified both ways - `git merge origin/fix/issue-444` into this branch failed that test before this commit and passes after, with the 8 known leave-command failures unchanged in both trees. Replace the same-named row rather than joining it. These tests are about the two renderers, so they own the whole client list and assert nothing about which clients the bundled catalog happens to ship. Co-Authored-By: Claude <noreply@anthropic.com>
…f re-anchoring it under $HOME (#447) * resolveClientSettingsPath rejects an absolute settings_file instead of re-anchoring it under $HOME An absolute `settings_file` was not an error: `path.join(homeDir, ...settingsFile.split('/'))` swallows the leading empty segment, so `/Library/Managed Preferences/com.anthropic.claudefordesktop.plist` silently became `$HOME/Library/Managed Preferences/...`. The env-override branch was wrong the same way (`parts.slice(1)` assumes a relative first segment, so it dropped the leading `/` and grafted the rest onto `$<CLIENT>_HOME`). The probe then answered about a file the manifest never named. The usual answer was ENOENT, which reads exactly like a correct "not attached", and that wrong-negative-indistinguishable-from-a-right-one is how the Claude Desktop attach_probe defect (#444) stayed invisible. It can also be a wrong positive: the new probe test builds the marked decoy at the re-anchored `$HOME` location and the old code reports `attached: true`. Reject rather than honour: the `$<CLIENT>_HOME` override relocates a config home, which an absolute path does not have, so honouring would publish a second silently-different resolution rule for one field. The resolver is shared by the read side (attach probe, picker detect) and the write side (the disk-driven undo), so a value core cannot resolve must fail rather than resolve to something else. Each caller renders the throw on its own terms: the status probe returns `{ attached: false, error }` (already carried through to `hyp status`), `hyp detach` fails loudly, and the init picker keeps its documented best-effort "not present". Load-time manifest validation is left to follow-up, sequenced after the Desktop manifest is corrected. LLP 0045 Part 3 gains the governing section, and the kernel type now states the home-relative contract it only ever implied. Co-Authored-By: Claude <noreply@anthropic.com> * hyp status: print a client probe error on the text surface, not only under --json The guard makes an unresolvable settings_file an `error` on the probe result, but `renderStatusText` dropped the field: it printed the client as a bare `not attached` and, because such a client is typically not `configured`, usually collapsed the whole section into `clients: (none)`. On stock master + this branch that is exactly what happened to claude-desktop, so the wrong negative the PR set out to end survived one layer up, on the surface a human actually reads. --json already carried it. Print the message under the client's line, and keep an errored client out of the `(none)` collapse. LLP 0045 Part 3 records that both surfaces carry it. * resolveClientSettingsPath: enforce the contract on the resolved path, not the first character The isAbsolute guard rejects '/etc/passwd' and lets '../../../etc/passwd' through to exactly the same place. Both land on a file the manifest never named, which is the whole harm this guard exists to stop, and the escaping spelling is the one that survives the check: resolveClientSettingsPath('codex', '../../../etc/passwd', {}, '/Users/hyp') -> '/etc/passwd' (pre-fix) .codex/../../../etc/passwd + CODEX_HOME=/tmp/ch -> '/etc/passwd' It matters more here than in a read-only resolver: this is also the write side, detachClientFromDisk reads and rewrites whatever it is handed, and contributes.client is unvalidated, so the value can arrive from a remotely-installed or org-pushed plugin. Require the resolved path to stay under the base it resolved against (settings_file_escapes_base). Each branch checks its own base - $HOME normally, $<CLIENT>_HOME when set, since the override is exactly a licence to leave $HOME. A '..' that normalizes away stays legal. Also drops two overclaims the section had picked up: attach_probe really does lose expressiveness (it has no absolute sibling, which is why #445 deletes Desktop's probe rather than respelling it), and the one-resolver argument holds for core but not for the per-plugin attach writers - the claude plugin hardcodes ~/.claude/settings.json and ignores $CLAUDE_HOME. And the ClientSettingsPathError JSDoc no longer claims callers avoid message-text matching when its only consumer flattens to a message. * client_settings_path: return the path the guard checked, and pin the prefix test Round-2 review of the containment guard added in 623eb6b. The guard holds against every escape spelling I could build (absolute, `..` in either branch, prefix-sharing siblings, `..` landing on the base), but three things about it were either untested or unwritten: - The `base + separator` suffix in the prefix test is the entire check - a bare `startsWith(base)` accepts `/home/username` under a `/home/u` base - and deleting the suffix made no test fail. Pinned. - `withinBase` validated `path.resolve(joined)` and returned the raw `joined`. They diverge whenever the base is relative: `CODEX_HOME=..` returned the relative `../config.toml`, i.e. a value re-resolved against `process.cwd()` at read time rather than the one validated at call time, and not the absolute path the resolver's contract promises. Return the checked path. - The check is lexical, not realpath-based, so a config home that is itself a symlink out of $HOME still passes. That is the right boundary - the field is resolved before the file must exist, and planting such a symlink already needs write access to $HOME, whereas the untrusted input is the unvalidated manifest value - but it was nowhere stated, and an undocumented limit on a security control invites over-trust. Written down in the JSDoc and in LLP 0045 Part 3. Co-Authored-By: Claude <noreply@anthropic.com> * status-client-error tests: own the client list instead of appending to it The errored `claude-desktop` row was pushed onto whatever `collectHypAwareStatus` had already produced, and the bundled catalog produces a `claude-desktop` row of its own carrying this very error - the manifest defect #445 removes. So the list held two same-named rows and the JSON assertion's `find` answered from the catalog's, passing for the wrong reason. It stops passing the moment #445 lands: the catalog row loses its error, `find` still returns it first, and `the JSON renderer carries the same client error` fails. Verified both ways - `git merge origin/fix/issue-444` into this branch failed that test before this commit and passes after, with the 8 known leave-command failures unchanged in both trees. Replace the same-named row rather than joining it. These tests are about the two renderers, so they own the whole client list and assert nothing about which clients the bundled catalog happens to ship. 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>
Uh oh!
There was an error while loading. Please reload this page.
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>
What was actually wrong
The issue's headline (
always fails with MALFORMED_MARKER) turns out to be thewrong symptom, but it is pointing at a real and worse bug underneath.
hypaware-core/plugins-workspace/claude-desktop/hypaware.plugin.jsondeclared:Reproduced against
origin/masterby drivingdetachClientFromDiskandprobeClientAttachFromDescriptorwith the descriptorbuildPluginCatalogderives from that manifest:
changed:falseclaude-desktop installactually rendersMALFORMED_JSONerror: Unexpected token '<'changed:falseinferenceCredentialHelperas an objectMALFORMED_MARKERMALFORMED_MARKERneeds the last row, which cannot occur:inferenceCredentialHelperis a plain string in the app's own schema(LLP 0115#real-profile-keys). The failure a user with Desktop configured
actually hits is a silent, wrong "not attached":
resolveClientSettingsPathre-anchors the absolute
settings_fileunder$HOME, so the probe neverreads the real, root-owned plist at
/Library/Managed Preferences/...atall - it reads a path that never exists - and
probeClientAttachFromDescriptortreats that
ENOENTasattached:falsewith noerror(
status.js:1012-1013). The table'sMALFORMED_JSONrow is real, but it iswhat the probe throws only once that path is corrected and it actually
reaches the XML plist; the new test demonstrates this by writing that plist
directly at the re-anchored path, which is not where
hyp claude-desktop installplaces it today.(Corrected post-review: an earlier version of this line claimed the field
failure users actually hit is
MALFORMED_JSON, firing "on exactly themachines where install had succeeded". Round 2 found that backwards -
defect 2 masks defect 1 in the field, so the quiet wrong answer is what ships
today, not the loud one.
llp/0135#no-proberecords the corrected account;this body line is fixed here as part of triage.)
The probe was wrong three independent ways, any one fatal:
json/json_pathpathJSON.parses the settings file.settings_fileis$HOME-relative by contract(
resolveClientSettingsPath), so the absolute/Library/...re-anchored to~/Library/Managed Preferences/...and never named the real file.marker_record, and per LLP 0045 Part 3 arecord-less marker is refused rather than half-reversed. Adding one would
not help: the plist is root-owned, so an unprivileged detach cannot write it
back at all.
The fix
Manifest-only (plus the doc and comment it made dishonest): remove the
attach_probe.This is not a new decision, it is restoring one.
LLP 0115#no-attach-on-join
is Accepted and says Desktop registers no
contributes.clientwith anattach_probe, because the LLP 0044 loop requires a reversible settings-filewrite that Desktop cannot offer. LLP 0135's design bullet asked for one anyway,
"reflecting the
entrypoint: claude-desktop-3pattribution finding" - acategory error, since an
attach_probeis not a label but the input toprobeClientAttachFromDescriptor/detachClientFromDisk.hypaware.plugin.json: dropattach_probe.contributes.clientkeepsname/skill_dir/agent_dir, the picker row and the install/verifycommands are untouched.
src/index.js: theactivate()JSDoc justified the client descriptor by"the plist is a real local surface", which is what led here. It now says why
there is deliberately no probe, with
@ref LLP 0115#no-attach-on-join.llp/0135: corrected the bullet and added a{#no-probe}section recordingall three reasons, so the next reader does not re-add it.
Behaviour after:
detachClientFromDiskreturns{ changed: false }at itsexisting no-probe guard, so
hyp detach --client claude-desktopprintsnothing to doinstead of erroring over a file core can neither read norreverse. Desktop's state surface stays
hyp claude-desktop verify; its undostays removing the plist with sudo (
@ref LLP 0133#one-surface). Attach onjoin is unchanged and still inert: the plugin registers no runtime
ctx.clientsadapter, soaction_attach.js'sdesired()never named it.The
MALFORMED_MARKERguard is untouched. Refusing beats a half-reversaland that is the documented intent (LLP 0045 Part 3); the bug was declaring a
probe over a file that guard was never meant to see.
Test
New
test/plugins/claude-desktop-detach.test.js, binding to the shippedmanifest (not a hand-written copy) and to the plist
installactually rendersvia
renderManagedPreferencesPlist:attach_probe;detachClientFromDiskreturnschanged:falseand does not throw;errorinhyp status.All three fail on
origin/master(2 and 3 with theMALFORMED_JSONparseerror) and pass with this change.
npm test: 2795 pass, 8 fail - all 8 intest/core/leave-command.test.js,verified identical on a pristine
origin/mastercheckout (pre-existing,unrelated).
Conflict note
No overlap with the held PR #441. This branch does not touch
src/core/config/client_detach_disk.js,src/core/config/types.d.ts, orLLP 0045 at all. Files changed: the claude-desktop manifest, that plugin's
src/index.js, LLP 0135, and one new test file.Follow-up worth its own issue (not fixed here)
resolveClientSettingsPathsilently re-anchors an absolutesettings_fileunder
$HOMErather than honouring or rejecting it. That is the trap that letdefect 2 above ship unnoticed, and it will catch the next manifest author too.
Fixes#444