From 87f89d9e219be892b0a45721cd0b3ac3edb9fe33 Mon Sep 17 00:00:00 2001 From: neutral Date: Fri, 31 Jul 2026 01:15:49 +0000 Subject: [PATCH] detach: document restoredValue as single-primary only (#443 finding B, option D) `DetachFromDiskResult.restoredValue` is a single scalar, but the `json_path` undo's `set` loop can restore more than one prior. Both restores land on disk correctly; only one is reported back, and nothing said which. Per the maintainer decision on issue #443 this lands as option D, documentation only: state in the contract docs that `restoredValue` is meaningful only for a single-primary record, and defer first-wins vs last-wins vs a plural `restoredValues: string[]` until a real multi-entry producer exists. Minting a "the first `set` entry is primary" convention today would promote a plugin-local assumption into the published kernel contract for a case nothing shipped can produce. No behaviour change, so no regression test: a test here would assert the very answer that was deliberately left open. Also corrects the stale "(codex)" tag on the CLI payload's `restored_value`: the `json` and `json_path` undos emit it too. Co-Authored-By: Claude --- hypaware-plugin-kernel-types.d.ts | 10 +++++ llp/0109-openclaw-client-adapter.decision.md | 46 ++++++++++++++++++++ src/core/cli/types.d.ts | 9 +++- src/core/config/types.d.ts | 19 +++++++- 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/hypaware-plugin-kernel-types.d.ts b/hypaware-plugin-kernel-types.d.ts index f1f2b2f5..9e4bfa05 100644 --- a/hypaware-plugin-kernel-types.d.ts +++ b/hypaware-plugin-kernel-types.d.ts @@ -204,6 +204,16 @@ export interface PluginAttachProbeManifest { * `json_path` only: dotted path RELATIVE to the marker object to a * string property holding the JSON-encoded self-describing undo * record (e.g. `headers.x-hypaware-marker`). + * + * The record's `set` list is **unordered**: core replays every entry and + * nothing in this contract makes the first one primary. A record whose + * replay restores more than one prior value restores them all on disk, + * but the single `DetachFromDiskResult.restoredValue` reported back (and + * the `restored_value` field of `hyp detach --json`) is only meaningful + * for a record with at most one restoring entry; for several, which one + * is reported is unspecified. Declare at most one restoring `set` entry + * per record if the restored value must be reported (LLP 0109 + * §restoredValue is single-primary only). */ marker_record?: string } diff --git a/llp/0109-openclaw-client-adapter.decision.md b/llp/0109-openclaw-client-adapter.decision.md index 72d5aa1d..9496ffb0 100644 --- a/llp/0109-openclaw-client-adapter.decision.md +++ b/llp/0109-openclaw-client-adapter.decision.md @@ -143,6 +143,47 @@ core format dispatch (probe + `detachClientFromDisk`): This keeps LLP 0045's invariants: one core undo, driven entirely from the settings file on disk, and the marker is a self-describing undo record. +#### restoredValue is single-primary only + +The undo restores **every** still-ours `set` entry that carries a `prev`, +but `DetachFromDiskResult.restoredValue` is a single scalar. A record whose +replay restores more than one prior therefore restores them all correctly +**on disk** and reports only one of them to the caller. That field is +defined here as **meaningful only for a single-primary record**: one with at +most one restoring `set` entry. For a multi-entry record the reported value +is unspecified, and no caller may depend on which one it is. + +Unlike `warning`, folding is not available. `restoredValue` has exactly two +consumers, both in [`src/core/commands/clients.js`](../src/core/commands/clients.js): +`restored_value` in the `hyp detach --json` payload, and ` Restored ` on +stdout. Both render it as a bare scalar, so joining two values would be +actively wrong rather than merely ugly. (The attach handler's span `detail` +consumer reads `warning`, not this field.) + +The multi-entry semantics are **deliberately left unstated rather than +settled by a guard**. A first-wins guard, matching the one `removed` already +carries three lines later in the same loop, would not fix the defect: it +would pick a different arbitrary survivor and still drop the rest. Worse, it +would promote "the first `set` entry is primary" into behaviour of the +published `marker_record` contract. That convention exists today only inside +the OpenClaw plugin (`openclaw/src/settings.js` reads `setEntries[0]` as +primary); this section's own rule - core knows `json_path` semantics, never +"OpenClaw" - says core must not learn it. + +Nothing shipped can reach the case: `claude` is `json` and `codex` is +`toml`, so neither enters this loop, and OpenClaw, the only real +`json_path` + `marker_record` producer, always emits exactly one `set` +entry. It is unreached, not unreachable by construction: `marker_record` +is part of the published kernel contract, so a third-party plugin can +declare two `set` entries on day one. When such a producer exists, decide +then between first-wins, last-wins, and a new plural +`restoredValues: string[]`, with a concrete consumer in front of you. + +The sibling `removed` field's first-wins guard is itself undocumented, so a +multi-entry record can describe one entry by one rule and another by the +opposite rule. That incoherence is **recorded here, not resolved**: +resolving it is the same deferred decision. + ### Gateway capture - The plugin requires `hypaware.ai-gateway ^2.0.0`, registers the client @@ -194,6 +235,11 @@ settings file on disk, and the marker is a self-describing undo record. - Whether OpenClaw session JSONL (`~/.openclaw/agents//sessions/`) should feed a settlement enricher for native session identity, like the Claude transcript enricher (LLP 0027). +- What `restoredValue` should report when a `json_path` undo restores + more than one `set` entry (first-wins and a primacy convention, + last-wins, or a plural `restoredValues: string[]`). Deferred until a + real multi-entry producer exists; documented above as single-primary + only in the meantime (issue #443). ## References diff --git a/src/core/cli/types.d.ts b/src/core/cli/types.d.ts index b5daa661..4f7c1210 100644 --- a/src/core/cli/types.d.ts +++ b/src/core/cli/types.d.ts @@ -348,7 +348,14 @@ export interface ClientResult { prev_value?: unknown /** Value removed on detach, when one was present. */ removed?: string - /** Prior value restored on detach (codex). */ + /** + * Prior value restored on detach. Emitted by every core undo format that + * records a prior (`json`, `toml`, `json_path`), not just codex. + * + * Single-primary only: an undo that restores several priors restores them + * all on disk but reports one unspecified value here (LLP 0109 + * §restoredValue is single-primary only). + */ restored_value?: string /** Non-fatal warning emitted by the adapter. */ warning?: string diff --git a/src/core/config/types.d.ts b/src/core/config/types.d.ts index 3502c829..45f55922 100644 --- a/src/core/config/types.d.ts +++ b/src/core/config/types.d.ts @@ -645,7 +645,24 @@ export interface DetachFromDiskResult { settingsPath?: string /** The managed value deleted (e.g. the gateway base URL) when there was no prior to restore. */ removed?: string - /** The prior value restored from the undo record. */ + /** + * The prior value restored from the undo record. + * + * **Single-primary only.** An undo whose replay restores more than one + * prior (a `json_path` record with two still-ours `set` entries that each + * carry a `prev`) restores them all on disk but reports only one of them + * here, and *which* one is unspecified. Unlike `warning` there is no fold + * available: both consumers render this as a bare scalar + * (`src/core/commands/clients.js` puts it in the `hyp detach --json` + * payload as `restored_value` and prints ` Restored ` on stdout), so + * joining two values would be wrong rather than merely ugly. + * + * No shipped producer emits a multi-restore record, and the multi-entry + * semantics are deliberately unstated rather than pinned by a guard (LLP + * 0109 §restoredValue is single-primary only). A caller that needs every + * restored value should get a new `restoredValues: string[]` field, not + * read meaning into this one. + */ restoredValue?: string /** * Set when a managed value was overridden externally and left in place. The