detach: restoredValue is single-primary only (#443 finding B, option D)

[philcunliffe]

Documentation-only. Implements the maintainer decision recorded on issue #443
(comment):
finding A already resolved, no action; finding B is option D, document only.

Finding A: PR #441's "one consumer" claim - prose, and already fixed on master

No change in this PR. Verified on origin/master (73b4618), not assumed:

• src/core/config/types.d.ts DetachFromDiskResult.warning already says
  "Display only: do not parse or split this field" and names both consumers
  (action_attach.js span detail, hyp detach stdout + --json).
• llp/0045-client-attach.design.md §never-clobber-a-user-edit already says the
  same in prose.
• PR detach: report every externally-overridden managed key, not just the last (issue #440 finding 2) #441's body was corrected at triage and now reads "it has two consumers, not
  one".

The issue's own framing of finding A had a further error, caught by triage in the
thread: it listed action_attach.js:346 under restoredValue. It is not a
consumer of restoredValue - it reads warning only. restoredValue has
exactly two consumers, both in src/core/commands/clients.js: restored_value
in the --json payload and Restored <v> on stdout. Confirmed by grep on
master; the docs added here state it that way.

Finding B: restoredValue multi-entry semantics - prose (option D)

detachJsonPathMarker restores every still-ours set entry that carries a
prev. All of them land on disk correctly. But DetachFromDiskResult
restoredValue is a single scalar, so a multi-entry record reports only one of
them, and no contract said which.

Per the decision, this lands as a stated contract, not a silent one-liner:
restoredValue is documented as meaningful only for a single-primary record,
and the first-wins / last-wins / plural restoredValues: string[] question is
deferred until a real multi-entry producer exists. Documented in:

• src/core/config/types.d.ts - DetachFromDiskResult.restoredValue.
• hypaware-plugin-kernel-types.d.ts - PluginAttachProbeManifest.marker_record,
  stating that set is unordered and that this contract does not make the first
  entry primary.
• src/core/cli/types.d.ts - the restored_value field of hyp detach --json.
• llp/0109-openclaw-client-adapter.decision.md - a new
  #### restoredValue is single-primary only subsection under
  §Probe and detach (core-owned), plus the deferred question added to
  §Open questions.

The LLP text records why a first-wins guard was rejected rather than merely
noting the gap: it would pick a different arbitrary survivor rather than fix the
defect, and it would promote "the first set entry is primary" (a convention
that exists today only inside openclaw/src/settings.js) into behaviour of the
published marker_record contract, which the same section forbids core from
learning. It also records the sibling incoherence - removed carries an
undocumented first-wins guard in the same loop - as recorded, not resolved.

Small adjacent prose correction: the CLI payload's restored_value was tagged
"(codex)". The json and json_path undos emit it too.

Reproduction evidence

None, by construction, and deliberately so. Both findings are prose. No
behaviour changes in this PR, so nothing here has a failing-then-passing test.

For finding B specifically, a regression test would have to assert which value
a multi-entry record reports - the exact question the decision left open.
Pinning an unratified answer in a test is worse than no test, so none was
written. The existing single-primary coverage
(test/core/client-detach-json-path.test.js:216,
test/core/client-detach-disk.test.js) already pins the documented case.

Checks

• npm run typecheck: clean.
• npm test: 3028 pass / 8 fail, identical to a pristine origin/master
  baseline run in a separate worktree. All 8 are the pre-existing
  test/core/leave-command.test.js failures (leave after join removes the seed and reports the server, leave clears an applied central slot..., leave reverses org-driven attaches..., leave after join also warns..., leave is idempotent..., leave still tears down..., leave removes the assets...,
  leave self-heals an org attach...). Same eight by name on both sides.

Sequencing

No file here overlaps any of the five open, held PRs. LLP 0045 and
src/core/config/client_detach_disk.js were both deliberately avoided because
PR #495 changes them; the contract statement lives in the type docs and the LLP
0109 lineage instead, which is where the decision put it. git merge-tree --write-tree of this branch against the heads of #456, #462, #491, #493 and
#495 exits 0 with no conflicts.

One consequence worth flagging for a follow-up: the natural @ref LLP 0109#restoredvalue-is-single-primary-only annotation belongs above the set
loop in src/core/config/client_detach_disk.js, which #495 is holding. It was
left out rather than seeding a conflict into the queue.

Fixes #443

[philcunliffe]

Verdict: clean. Round 1 of 2, head 87f89d9. No actionable findings, nothing pushed.
Direct audit: the code-review skill is not installed in this clone and codex is not on
PATH, so this is a hand audit, not a tool-assisted one.

Does it implement the maintainer's decision?

Yes, faithfully (reasoned, from reading every added line against the
option-D comment).

I checked the two ways this could have gone wrong:

1. It does not mint the primacy convention. Every one of the four statements says the
multi-entry answer is unspecified, and none implies first-wins:

• src/core/config/types.d.ts:648 - "reports only one of them here, and which one is unspecified"
• hypaware-plugin-kernel-types.d.ts:208 - "nothing in this contract makes the first one primary"
• src/core/cli/types.d.ts:352 - "reports one unspecified value here"
• llp/0109:153 - "no caller may depend on which one it is"

The diff adds no test file (executed: git diff --stat is 4 files, all docs). The author's
reasoning for writing none holds: any multi-entry regression test would have to assert which
value is reported, which is the exact question the decision deferred. Pinning it in a test would
have been a de-facto answer, so the omission is correct rather than a gap.

2. It did no work on finding A. The diff touches neither LLP 0045 nor warning's doc comment
(executed). I re-verified A really is already resolved on origin/master (73b4618) rather than
taking the PR's word:

• src/core/config/types.d.ts:650-664 already carries "Display only: do not parse or split this
  field" and names both consumers (action_attach.js span detail; hyp detach stdout +
  --json).
• llp/0045-client-attach.design.md:242 §"Never clobber a user edit" already says warning is
  "displayed, never parsed" and names the same two.

So "no action" was the right call and the PR correctly took it.

Consumer counts, verified on origin/master (all executed)

The PR claims the issue body was itself wrong. It was, and the PR's correction is right:

• action_attach.js:346 is not a consumer of restoredValue. grep restoredValue src/core/config/action_attach.js returns nothing; the file reads result.warning only
  (:338 guard, :346 detail:). The issue body listed it under restoredValue in error.
• restoredValue has exactly two consumers, both in src/core/commands/clients.js:
  :589 (payload.restored_value = result.restoredValue) and :597
  ( Restored ${result.restoredValue}). A whole-tree grep finds no third read of the field off a
  DetachFromDiskResult; the other hits are producers (client_detach_disk.js, the codex
  plugin) or mirrored types.
  Note for the record: the stuck comment cited :587/:595; they are :589/:597 on current
  master. The PR's docs cite no line numbers, only the file, so nothing it states drifts.
• The "(codex)" correction is right. All three core undo branches set restoredValue:
  client_detach_disk.js:182 (json), :503 (json_path), :644 (toml). The new
  src/core/cli/types.d.ts text naming json, toml, json_path is accurate.

Other prose claims I spot-checked and confirmed: removed's first-wins guard really is three
lines below the restoredValue assignment (:503 → :506) and really is undocumented;
openclaw/src/settings.js:175 emits exactly one set entry while :244/:295 read
setEntries[0] as primary; claude=json, codex=toml, openclaw=json_path+marker_record
per the three manifests. The LLP's "nothing shipped can reach this" enumeration is complete on
today's tree - claude-desktop no longer declares an attach_probe at all, so its omission is
correct and the LLP text is more current than the issue body.

LLP 0109

Executed. #### restoredValue is single-primary only lands at :146, correctly nested inside
### Probe and detach (core-owned) (:111) and before ### Gateway capture (:187). That is the
right lineage: it is the section that defines the json_path set replay and is the anchor
client_detach_disk.js:450 already @refs. Its claim to be bound by "this section's own rule -
core knows json_path semantics, never OpenClaw" quotes text that genuinely exists at :139.

All 24 existing @ref LLP 0109#... annotations still resolve - no heading was renamed or moved,
and the new #### only adds an anchor. The relative link ../src/core/commands/clients.js
resolves. The §Open questions entry is present at :238 and leaves all three options live.

Checks

Check	Result
npm test (PR head, 87f89d9)	3037 tests / 3028 pass / 8 fail
npm test (pristine origin/master, 73b4618)	3037 tests / 3028 pass / 8 fail
npm run typecheck	clean, exit 0
@ref hygiene	clean - no @ref added, none broken
House style	clean

Executed. Both suites run in my own detached worktrees with node_modules symlinked from the
clone. Counts are identical on both sides, so this docs-only PR moves no test counts, and the
failure sets are identical by name: all 8 are the pre-existing test/core/leave-command.test.js
failures, confirmed against my own baseline, matching the eight the PR body lists.

Methodology note, so the numbers are reproducible: my first pass ran the two suites
concurrently and reported 3053 vs 3037 with divergent name lists. That was my artifact - the
logs contained interleaved TAP lines (findDesktop3pProjectsDirs is empty when # Subtest: install: org_key mode ...). Re-run serially, both sides are bit-identical at 3037/3028/8.
The delta was mine, not the PR's.

House style (executed, on added lines only): no U+2014 em dashes introduced in any of the four
files; no @typedef; no inline import('...') types; no new type-import specifiers; the sole
added ; is prose inside a JSDoc sentence, not code.

Sequencing

Executed. git merge-tree --write-tree of 87f89d9 against the heads of #456 (0bc6336),
#462 (4dcd246), #491 (1685288), #493 (1c5c620) and #495 (c829325) exits 0 for all five,
no conflicts. File sets are disjoint - the types.d.ts files those PRs touch are all different
files (ai-gateway/src/, codex/src/, claude/src/, src/core/cache/), none of this PR's four.

The body's §Sequencing does disclose the deferred @ref LLP 0109#restoredvalue-is-single-primary-only above the set loop in
src/core/config/client_detach_disk.js. I confirmed the rationale is genuine, not a
rationalisation: #495 really does hold both src/core/config/client_detach_disk.js and
llp/0045-client-attach.design.md. Leaving the annotation out rather than seeding a conflict into
the held queue is the correct call under the sequencing rule, and I am not filing it as a
finding. It should land as a follow-up once #495 merges.

Findings

None actionable. One observation recorded for the maintainer, deliberately not acted on:

• (nit, design, no change made) hypaware-plugin-kernel-types.d.ts:207 opens with "The
  record's set list is unordered". The sentence that follows it ("nothing in this contract
  makes the first one primary") is the precise claim and is exactly right; "unordered" is a
  slightly stronger summary of it. Two small tensions worth knowing about, neither of which I would
  change here: core's shipped behaviour is order-sensitive today for the sibling removed
  (its first-wins guard at client_detach_disk.js:506 takes the first still-ours entry), and LLP
  0109 §Open questions keeps "first-wins and a primacy convention" live, which a flat "unordered"
  in the published contract would have to be walked back to adopt. This sits on top of the
  deferred question the maintainer explicitly parked, so it is recorded rather than edited.
  Non-blocking either way.

No design disagreement with option D itself, which is not this round's to revisit.