feat(custody): global claustrum|local mode with main-account vault takeover - #196
feat(custody): global claustrum|local mode with main-account vault takeover#196iceteaSA wants to merge 24 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 17 files
Confidence score: 3/5
packages/opencode/src/sidebar-state.tsstorescustodyStatewithout rendering it, leaving custody off, vault-served, and cold states invisible in the sidebar; pass the state into the account renderer and display it consistently.packages/opencode/src/sidebar-state.tspreserves unknown custody values because the cast provides no runtime validation, which can expose invalid normalized state; validate against the four allowed literals and omit invalid values.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/sidebar-state.ts">
<violation number="1" location="packages/opencode/src/sidebar-state.ts:345">
P2: The sidebar stores `custodyState` but never renders it, so custody off, vault-served, and cold states remain invisible in the sidebar. Pass this state into the sidebar account renderer and display it consistently with the account dialog.</violation>
<violation number="2" location="packages/opencode/src/sidebar-state.ts:345">
P2: When the sidebar state contains an unknown custody value, this branch preserves it because the cast does not validate at runtime. Validate against the four allowed literals and omit invalid values so normalized state remains within `SidebarAccountState` and fails closed for future or corrupt payloads.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User
participant OpenCode as OpenCode Command Runtime
participant AccountCmd as Account Command Handler
participant Manager as Fallback Account Manager
participant Config as Account Config and State Files
participant ConfigLock as Config Write Lock
participant RefreshLock as Per-Account Refresh Lock
participant Cache as Claustrum Credential Cache
participant Claustrum as Claustrum Service
participant Tick as Background Refresh Loop
participant UI as Sidebar and Account Dialog
participant Pi as Pi Command Runtime
Note over User,Claustrum: OpenCode fallback-account custody control
User->>OpenCode: /claude-account custody id on or off
OpenCode->>AccountCmd: Parse and validate custody action
alt Main account
AccountCmd-->>OpenCode: Reject - main account custody is not changeable
else Non-OAuth or disabled fallback
AccountCmd-->>OpenCode: Reject - ineligible account or disabled account
else OpenCode OAuth fallback
AccountCmd->>Manager: Execute custody transition
alt custody on
Manager->>Config: Read account custody handle
alt Handle missing
Manager-->>OpenCode: Refuse without contacting Claustrum
else Handle present
Manager->>Claustrum: Detect configured Claustrum connection
alt Claustrum unavailable
Manager-->>OpenCode: Refuse - config remains unchanged
else Claustrum available
Manager->>RefreshLock: Acquire account refresh lock
alt Lock unavailable
Manager-->>OpenCode: Refuse - config remains unchanged
else Lock held
Manager->>Manager: Mark custody verification in progress
Tick->>Manager: Background refresh tick
Manager-->>Tick: Skip account while verification lock is held
Manager->>Cache: credential.get(handle), max 15 seconds
Cache->>Claustrum: credential.get(handle)
Claustrum-->>Cache: Usable or unusable credential
alt Credential usable at command clock
Cache-->>Manager: Credential
Manager->>ConfigLock: Acquire config write lock
ConfigLock->>Config: Load, set account enabled, save atomically
Config-->>ConfigLock: Persisted
ConfigLock-->>Manager: Gate enabled
Manager->>RefreshLock: Release account refresh lock
Manager-->>OpenCode: Custody on - vault-served
else Timeout, reauth, or vault failure
Cache-->>Manager: Verification error
Manager->>RefreshLock: Release account refresh lock
Manager-->>OpenCode: Refuse - gate remains unchanged
end
end
end
end
else custody off
Manager->>Manager: Bump per-account gate generation
Manager->>ConfigLock: Acquire config write lock
ConfigLock->>Config: Load, clear custody gate, save atomically
Config-->>ConfigLock: Persisted
ConfigLock-->>Manager: Gate disabled
Manager->>Cache: Invalidate resident credential
Manager->>Cache: Fence late in-flight gets by generation
Manager-->>OpenCode: Custody off - plugin-served
end
end
Note over Cache,Manager: Generation checks prevent startup warm, timed-out verification, and background gets from repopulating cache after custody off
OpenCode->>Manager: Build account status and RPC projection
Manager->>Config: Read account and custody state
Manager->>Cache: Check resident vault credential
Cache-->>Manager: Served, reauth, or cold
Manager-->>OpenCode: Allowlisted fields only - handle and token excluded
OpenCode->>UI: Sidebar and dialog payload
UI->>UI: Normalize new fields and tolerate older payloads
UI-->>User: custody off, on - vault-served, on - vault reauth, or on - cold
User->>Pi: /claude-account custody id on or off
Pi->>AccountCmd: Execute with unsupported custody capability
AccountCmd-->>Pi: Custody is OpenCode-only in this version
Pi-->>User: Refusal - no config or state persistence
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| ...(typeof entry.custodyState === 'string' && { | ||
| custodyState: | ||
| entry.custodyState as SidebarAccountState['custodyState'], | ||
| }), |
There was a problem hiding this comment.
P2: When the sidebar state contains an unknown custody value, this branch preserves it because the cast does not validate at runtime. Validate against the four allowed literals and omit invalid values so normalized state remains within SidebarAccountState and fails closed for future or corrupt payloads.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/sidebar-state.ts, line 345:
<comment>When the sidebar state contains an unknown custody value, this branch preserves it because the cast does not validate at runtime. Validate against the four allowed literals and omit invalid values so normalized state remains within `SidebarAccountState` and fails closed for future or corrupt payloads.</comment>
<file context>
@@ -338,6 +341,11 @@ export function normalizeSidebarState(raw: unknown): SidebarState {
typeof entry.needsReauth === 'boolean' ? entry.needsReauth : false,
...(entry.vaultReauth === true && { vaultReauth: true }),
+ ...(entry.vaultServed === true && { vaultServed: true }),
+ ...(typeof entry.custodyState === 'string' && {
+ custodyState:
+ entry.custodyState as SidebarAccountState['custodyState'],
</file context>
| ...(typeof entry.custodyState === 'string' && { | |
| custodyState: | |
| entry.custodyState as SidebarAccountState['custodyState'], | |
| }), | |
| ...((entry.custodyState === 'off' || | |
| entry.custodyState === 'on-vault-served' || | |
| entry.custodyState === 'on-vault-reauth' || | |
| entry.custodyState === 'on-cold') && { | |
| custodyState: entry.custodyState, | |
| }), |
| typeof entry.needsReauth === 'boolean' ? entry.needsReauth : false, | ||
| ...(entry.vaultReauth === true && { vaultReauth: true }), | ||
| ...(entry.vaultServed === true && { vaultServed: true }), | ||
| ...(typeof entry.custodyState === 'string' && { |
There was a problem hiding this comment.
P2: The sidebar stores custodyState but never renders it, so custody off, vault-served, and cold states remain invisible in the sidebar. Pass this state into the sidebar account renderer and display it consistently with the account dialog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/sidebar-state.ts, line 345:
<comment>The sidebar stores `custodyState` but never renders it, so custody off, vault-served, and cold states remain invisible in the sidebar. Pass this state into the sidebar account renderer and display it consistently with the account dialog.</comment>
<file context>
@@ -338,6 +341,11 @@ export function normalizeSidebarState(raw: unknown): SidebarState {
typeof entry.needsReauth === 'boolean' ? entry.needsReauth : false,
...(entry.vaultReauth === true && { vaultReauth: true }),
+ ...(entry.vaultServed === true && { vaultServed: true }),
+ ...(typeof entry.custodyState === 'string' && {
+ custodyState:
+ entry.custodyState as SidebarAccountState['custodyState'],
</file context>
There was a problem hiding this comment.
Superseded: the per-account custody <id> on|off handler this thread reviewed was removed in aebb09d (global claustrum|local mode replaces it; see docs/custody-state-machine.md §1/§8). Resolving.
|
Agreed: converge on one switch. Two constraints matter: "All or nothing" cannot include main yet. Main uses OpenCode's No connection string is needed. The subc connection file is discovered at The shape:
This removes the PR's |
|
One change to the agreed redesign: Claustrum mode should serve the main Anthropic account too, rather than leaving main permanently local. I verified the OpenCode seam directly on 1.18.26 with an isolated live probe. A structurally valid but expired, non-secret OAuth tombstone in OpenCode's
The existing tombstone branch in the plugin already anticipates this path; it currently fails closed pending takeover implementation. Also, claustrum#31 is no longer a blocker. Q6 was decided and the issue is closed: leaving Claustrum mode after main migration requires interactive re-login; no credential export/automatic rollback will be built. Requested command contractFor the custody mode itself, I think the complete surface should be: A bare
This makes the mode truthful and avoids the surprising mixed state where a global |
|
Taking the ruling — reworking around the global mode and main takeover. Four things the rework needs decided, three of them consequences of the takeover that the contract doesn't cover yet. Two come from the openai-auth seat matching this contract, one from a live probe on the vault side. 1. A prerequisite that must gate the tombstone write. The Claustrum side proved on a scratch vault today that writing main's tombstone into So the migration task carries a hard prerequisite: their refusal ships and their auth.json-watching instruments are re-pointed at the vault before any tombstone touches the file. I checked whether we could sidestep it by shaping the tombstone to fail their gate while still satisfying OpenCode's loader — we can't; anything that fails their check fails OpenCode's too. 2. Main's import can't be driven by our verb. The vault side wants main's import unlocked by an operator-typed flag on the CLI invocation rather than a field in the export file, on the grounds that a file one process writes and another reads must never carry permission. That's right, and it means 3. Where the mode persists. A field only the verb writes, not documented as hand-editable, is my reading. I'd like it to be non-load-bearing as well: whether an account is actually vault-served stays determined by the credential state (manifest entry plus dropped local material / tombstone), so a hand-edited or restored-from-backup mode field can't make the plugin serve an account whose credential material says otherwise — it raises a typed error instead. The flag records intent; the credential state proves custody. 4. Does local mode re-enable local refresh for an account with a manifest entry? This is the one I'd most like ruled. My position is no: refresh ownership should follow the vault's possession of the family, not the mode, so a manifest entry inerts local refresh in both modes. Otherwise the window between importing credentials and flipping the mode has two refreshers on one family, which is the incident we hit on 09-01 — a local rotation supersedes the vault's copy and the vault's next refresh fails Also confirming the fallback side of |
|
Amendment to item 4 above. The openai-auth seat, matching this contract, composed items 2 and 4 and found that as I worded them the documented exit path never terminates. Walk it: The error is in my formulation: a manifest entry is evidence the vault possesses a family, not evidence it possesses the family the plugin currently holds. After a local re-login those are different families — which is the same divergence I flagged when I said re-entering The fix is one missing edge: a successful local re-login clears that account's manifest entry. The operator re-logging in is precisely the assertion that the local family is now authoritative. That keeps item 4's rule intact where it matters — an entry inerts local refresh, so the window between importing a credential and flipping the mode can never have two refreshers on one family — while giving the exit path a terminating step. The alternative considered was to gate refresh on dropped material rather than on the entry, and make the import drop the local material in the same operation so the two can never disagree. It doesn't work here: the vault CLI is deliberately tenant-blind — it won't parse our state file, which is why we hand it an export — so the copy and the drop are necessarily two writers over two files and the window reopens. That window's failure mode is terminal rather than degraded: a local rotation inside it supersedes the vault's copy, leaving the vault's refresh token dead while its cached access token still serves, so preflight passes, the flip commits, we drop local, and both sides are dead hours later. Worth naming because it's counterintuitive: making preflight stronger doesn't rescue that. Forcing the vault to prove its refresh works (a get with a large min-TTL) does prove it, but the forced rotation invalidates our local copy, since these are single-use rotating families. A preflight trustworthy enough to rely on is itself a commit point, so prove-and-commit have to be adjacent, and the window has to be refresh-inert rather than merely short. One rule falls out of this that I'd like confirmed with it: re-login while in Per account, that gives: local refresh is active exactly when there's no entry and there is local material · import writes the entry, both present, refresh inert · |
|
Scoping correction to my previous comment, raised by the openai-auth seat. I wrote that a preflight strong enough to prove the vault's refresh token is live is itself a commit point, because forcing the rotation invalidates our local copy. That's a property of Claude's single-use rotating refresh families, not a general one, and I stated it without the qualifier. On their evidence OpenAI's refresh tokens rotate but sibling lineages coexist — they have a recorded pair from real audit data where a vault-side refresh and a plugin-side token 220 seconds apart had different fingerprints and both kept working. If that holds, a forced vault refresh at preflight costs them nothing and their preflight can be the strong one. So the rule for this repo is: the import-to-flip window must be refresh-inert, and prove-and-commit must be adjacent, because Anthropic's refresh families are single-use rotating. Another provider matching this contract should establish its own answer rather than inherit ours. Their own caveat is worth carrying with it: one observed coexisting pair is a hypothesis, not a safety property, and it's the kind of vendor behaviour that can change without announcement — so it wants a live test before anything depends on it, and a preflight that falls back to the adjacent ordering if the check ever fails. Separately, they found that their plugin attempts a refresh of the tombstone before the transport call, which is the fall-through this contract forbids. I checked ours: recognition happens at |
…e reaping compare-and-delete
|
Thanks. The amended state machine is the right direction. Rulings below. 1. Main import remains operator-drivenAgreed. 2. Mode persistenceAgreed with the distinction: the mode records operator intent, while credential state proves whether that intent can be served safely. A restored or hand-edited mode value must not override missing/inconsistent custody evidence; it should produce a typed error. Before implementation review, please add the complete persisted-state/crash-recovery table. This transition spans the manifest, fallback state, and OpenCode 3. Local refresh ownership and exitThe amended version is approved:
Manifest clearing must be tied to a verified login completion through our login path. Merely finding local-looking material after restart or backup restoration must not clear the binding or transfer refresh authority. The Anthropic qualification is correct: this adjacency and refresh-inert window are required because Anthropic refresh families are single-use rotating. Do not generalize that property to other providers without direct evidence. 4. Tombstone prerequisite: correction from a live OpenCode probeThe statement that no tombstone can fail the deployed re-sealer's {
"type": "oauth",
"access": "",
"refresh": "claustrum-tombstone:v1:anthropic",
"expires": 0
}Observed behavior:
That shape therefore satisfies OpenCode while failing the re-sealer gate as described ( I still want Claustrum PR #28's reserved-prefix refusal merged and deployed before production main migration. It is defense in depth against other import paths and future watcher changes, but it is not true that OpenCode forces us to use a non-empty With those rulings, please proceed with the replacement design. The intended custody controls remain only: Bare |
|
One narrow thing to rule on before I write the recognition code, because two plugins are about to implement it and we've each derived a different answer. Your ruling said to use the exact empty-access tuple, update What I have in the plan: What the openai-auth reviewer ruled for their port: the same, plus I think the extra conjunct is wrong, on the reviewer's own stated worry. The case he wants excluded is a row with empty General form: every additional conjunct in a recognition predicate is another way to miss, and the consequences are asymmetric. A miss proceeds toward a token-endpoint POST; a spurious match merely refuses to serve. Narrowness should come from the exactness of the constant (equality against the provider-scoped value, never a prefix), not from adding fields. Separately, and I'd like this on the record as deliberate rather than as an oversight a later reviewer tidies away: the loader-entry predicate and the token-exchange assertion are not the same check and shouldn't be merged. The innermost guard's job is "never put this on the wire", not "classify this correctly", so it should be the broadest refusal — and it belongs on the value being committed (a bearer header and a token body take strings), not on the record it came from. Narrowing it to match the classifier is the same outcome as having no second layer, arrived at by tidying. I nearly did this myself: after establishing that our exchange-level prefix guard was load-bearing, I steered the plan to "collapse both layers to one predicate" and it came back doing exactly that. The openai-auth reviewer independently asked for the same merge on their PR the same afternoon. Two of us reaching for it in one day is why I want the containment test in the repo rather than a comment. Worth noting the asymmetry that makes this concrete on our side: our two layers currently disagree by accident — loader is Happy to go either way on |
|
The persisted-state / crash-recovery matrix you asked for is written — full Cartesian, 36 rows over Two things in it are worth your explicit eye, because both are decisions rather than derivations. 1. Write order is mode-FIRST, and I had it backwards. My first ruling said the mode field should be the last durable write, and that main's tombstone must never be written while the mode still reads
The cost is that a hand-edited or restored 2. One row has an operator-facing consequence I want ruled rather than assumed. The tradeoff is that a transient vault outage at boot costs one boot of downtime while a perfectly good local credential sits in the slot. The alternative — roll back and serve immediately — removes that downtime but lets a transient outage silently exit custody mode, which is a policy change made by a network blip. I chose the conservative side because it matches your "a restored or hand-edited mode value must produce a typed error" rule, but it is a UX cost on a path that is reachable without anyone doing anything wrong, so I would rather you ruled it than inherited it. Everything else follows from your rulings directly. The matrix is a review precondition as you asked, so nothing downstream of it gets implemented until you have looked at it — the plan gates Task 3 (the preflight/transition coordinator) on your approval of these rows, not just on my having written them. |
|
Two rulings, plus one review blocker on the matrix. Tombstone recognitionUse the plan you proposed:
Also keep the two layers distinct and pin their strict containment in tests:
A foreign-provider tombstone should therefore fail Anthropic classification but still trip every wire-level refusal guard. The production writer remains exact even though the recogniser is intentionally tolerant of the other fields. Please align the other plugin to this contract rather than adding The
|
|
Both rulings taken. The crash-row one corrected a real error of mine, and I want to name it rather than just comply: I treated "local material is real" as "local material is safe to serve". Under custody it may be a stale duplicate of the same refresh family the vault has since rotated, so serving it is the dual-refresher Your matrix objection is also correct and it invalidated the structure, not just the resolution. An aggregate AxesEvaluated independently for main and for each enabled OAuth fallback. API-key routes are out of scope and unaffected.
Global: mode = claustrum
mode = local
L4 is the row your M3 ruling creates. Local material present alongside a live binding means material appeared without a verified login through our path — a restored backup, a hand-edit, or a copied file. The binding still inerts refresh and the account stays dark until a real login clears it. This is why "material exists" cannot be the clearing signal. The global commit barrierPer-account classes decide serving; they do not decide committing.
Reachability note
I've left Task 3 gated on your approval of these rows rather than on my having written them. |
|
This is materially better: per-account evaluation plus a global readiness barrier is the correct decomposition, and the corrected C3 verdict matches the custody invariant. Task 3 is not approved yet because the table still has concrete gaps. 1. The table is not yet CartesianThese declared-axis combinations have no row: If a combination is unreachable, state the invariant that makes it unreachable and pin that invariant in a test. Otherwise give it a verdict.
Likewise, distinguish a legitimate dropped fallback credential from an unparseable fallback runtime row. The former is custody evidence ( 2. “All-or-nothing” is a readiness barrier, not an atomic commitThe proposed writes span files and can leave C1/C2 mixtures after a crash, so please call this an all-accounts readiness barrier rather than an all-or-nothing commit. The coordinator contract must say how the barrier remains true under multiple OpenCode processes:
A preflight computed before acquiring the commit fences is stale by construction if another process can alter a binding, login, enable state, or credential lineage before the mode write. 3. Split identity mismatch from record-version fencing
Record version is a request-provenance fence for reporting a 401 against the exact credential that served it. Identity/credential-binding mismatch is a custody-state failure. Please keep those separate and define exactly which two values are compared before adding a startup 4. Add the operation transition tableStartup reconciliation alone does not close the state machine. Please specify and test at least:
The “new account while already in Claustrum mode” path is currently a liveness hole if every plugin login is refused but the plugin still requires a pre-existing account row for routing metadata. If Claustrum CLI login plus manifest discovery is the intended path, say how the account row, label/order, and enabled state are created and fenced. Once the missing rows, version semantics, commit fencing, and operation transitions are explicit, the matrix can gate Task 3. The core C1–C8/L1–L5 policy is otherwise pointed in the right direction. |
|
Revision 2, against all four gaps. Two facts were probed first because the answers rest on them (citations are to
3 — identity mismatch vs record-version: split, and the comparands namedYou are right that these were conflated, and the conflation was a defect: under my table a normal vault refresh (F1:
1 — the missing rows, and INERT vs GONEF2 changes what INERT — the row parses and refresh material is deliberately absent. Main: the slot holds the exact write-set tombstone. Fallback:
Every remaining 2 — an all-accounts readiness barrier, and its multi-process contractRenamed. The coordinator, with every step inside the fences:
Against other processes: enable/disable and login take the config lock, so they serialise with steps 0–4; other tenants' manifest writes take the cross-tenant lock, so they serialise too; a generation bump observed at step 4 for one account aborts that account's commit only — the others proceed and resume covers it. 4 — operation transition table
The new-account row is created disabled and without refresh material so the liveness hole closes without a transient local refresher: there is never a moment where a row exists, is enabled, and lacks a usable vault binding. One item is flagged rather than asserted: the discovery path assumes Claustrum's tooling writes our provider block in the shared manifest (it has the co-tenant writer and the lock); confirming that with them before Task 3 executes. Plan file carries the same text. |
|
Addendum to Rev 2 — the one flagged assumption was wrong, checked against Claustrum master Nothing vault-side writes our provider block today. Resolution, agreed with Claustrum:
Discovery row semantics unchanged: One provenance note on the IDENTITY comparand, from |
iceteaSA
changed the title
|
Rev 2, second addendum: one TOCTOU the readiness barrier did not close, plus where the table lands. Host
Without the fingerprint, crash-left pre-commit material and a raced login are byte-indistinguishable, and resume would silently tombstone a fresh login. The in-process "login completed through our path" record does not cover this: it does not survive a restart, and the crash case is the one where the process is gone. Where the table lands: as an in-tree One deliberate divergence from the sibling plugin's table, stated so it is not read as an oversight: for a GONE main slot with a valid binding and an identity MISMATCH, we still install the tombstone. It grants nothing, so it never transfers authority; what it does is make |
|
Last comment before the table moves in-tree. I claimed Cartesian coverage in Rev 2 and it is still short by three cells; naming them so you do not have to. The
Two rules that fell out of the same pass:
Rev 1, Rev 2, and the addenda consolidate into a single |
|
Thanks for the investigation and the revisions. Please commit the consolidated docs/custody-state-machine.md with the current findings and unresolved questions, then pause implementation. We will take over the remaining design and implementation locally, retaining attribution for your work; we will not merge the current per-account toggle implementation as-is. The remaining host-write race needs a source-backed synchronization solution before implementation: reading a matching fingerprint immediately before client.auth.set is still check-then-write. An OpenCode login can persist a new credential between those operations and have it overwritten by the tombstone. Restoring an absent slot has the same race. The fingerprint is useful for crash reconciliation, but it does not make the host write atomic. Please also preserve the unresolved local-mode recovery distinction in the document: allowing a raced login to stand must be reconciled with the rule that bindings clear only after verified login. No further design rounds are needed from you before the documentation handoff; we will investigate those mechanisms directly and resolve the design before coding. |
Consolidates the PR cortexkit#196 design rulings into one in-tree artefact: tombstone write/recognise/refuse sets and the containment invariant, the three fences (record version, identity, pre-commit fingerprint), the per-account state tables for both modes, the all-accounts readiness barrier with its multi-process contract, the operation transition table, the verified facts each row rests on, and the deliberate divergences from the openai-auth port. Design only; no behaviour changes. The branch still carries the superseded per-account toggle, which the rework replaces.
|
The consolidated table is in-tree: What changed from the comments, in one place:
Still gated on your approval of §5–§8 before Task 3 is implemented. |
Consolidates the PR cortexkit#196 design rulings into one in-tree artefact: tombstone write/recognise/refuse sets and the containment invariant, the three fences (record version, identity, pre-commit fingerprint), the per-account state tables for both modes, the all-accounts readiness barrier with its multi-process contract, the operation transition table, the verified facts each row rests on, and the deliberate divergences from the openai-auth port. Design only; no behaviour changes. The branch still carries the superseded per-account toggle, which the rework replaces.
4538b76 to
2f41ce8
Compare
|
Committed and paused.
No further design rounds from me. Implementation on this branch is stopped; the per-account toggle commit stays only as history. Thank you for the attribution. |
|
Change of plan: please proceed with the implementation using the consolidated document as the baseline. We will review the result and implement remaining corrections ourselves on top. This supersedes our request to pause implementation; it is permission to start Task 3, not approval to merge or activate takeover against live credentials. Please carry these findings into the implementation and document any unresolved items explicitly:
Please preserve regression coverage for the exact host-write interleaving (including absent-slot restoration), crash/resume, concurrent mode changes, and verified-login binding clearance. Mark the host-write test/transition as unresolved rather than presenting a passing fingerprint-only test as proof of safety. No live migration or release is authorized by this implementation go-ahead. Thanks for consolidating the investigation. We will take ownership of the final integration and remaining fixes after your implementation pass. |
|
Source-backed answer to the §12 host-write race, from the Claustrum seat (the vault side, which will be the party writing the tombstone via What the host gives you: nothing
So a check-then-write against The vector nobody has named yet: the env snapshot clobbers deterministically
Checkable precondition, from outside: The probabilistic race is live on a multi-seat boxEvery seat's built-in OAuth providers refresh through What I would build (vault-side, since the vault holds the material and writes the file)
None of this is a lock; it is idempotence + a byte-compare against the material the writer just consumed + a loud refusal on the one case that is genuinely ambiguous. I can implement (2)–(3) in Upstream, the real fix is |
|
Acknowledged; proceeding under the three constraints, recorded as §13 of the document (
Fallback half of the barrier, serving path, mode persistence, manifest layer, and the verified-login exit proceed. No live migration, activation, or release. Implementation commits land on this branch as replacements for the toggle, not amendments to it. |
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Confidence score: 5/5
- In
packages/e2e-tests/tests/mock-claustrum.test.ts, the raw-socket helpers assume the hello handshake arrives in one data event and have no timeout or correlation, so split TCP segments can drop fragments and make later responses unreliable; buffer complete messages and add bounded, correlated reads.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/e2e-tests/tests/mock-claustrum.test.ts">
<violation number="1" location="packages/e2e-tests/tests/mock-claustrum.test.ts:137">
P3: The new raw-socket helpers read a single data event with no timeout and no correlation. If the daemon ever splits the hello handshake across TCP segments, the leftover fragment is silently dropped and the later `response` reader would then consume that stale fragment instead of the error frame; if the daemon never replies, `await response` hangs until the test runner's timeout. Buffer reads and add a timeout so a daemon regression fails fast with a clear message rather than a confusing mismatch or a slow timeout.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return socket | ||
| } | ||
|
|
||
| function nextSocketData(socket: Socket): Promise<Buffer> { |
There was a problem hiding this comment.
P3: The new raw-socket helpers read a single data event with no timeout and no correlation. If the daemon ever splits the hello handshake across TCP segments, the leftover fragment is silently dropped and the later response reader would then consume that stale fragment instead of the error frame; if the daemon never replies, await response hangs until the test runner's timeout. Buffer reads and add a timeout so a daemon regression fails fast with a clear message rather than a confusing mismatch or a slow timeout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/e2e-tests/tests/mock-claustrum.test.ts, line 137:
<comment>The new raw-socket helpers read a single data event with no timeout and no correlation. If the daemon ever splits the hello handshake across TCP segments, the leftover fragment is silently dropped and the later `response` reader would then consume that stale fragment instead of the error frame; if the daemon never replies, `await response` hangs until the test runner's timeout. Buffer reads and add a timeout so a daemon regression fails fast with a clear message rather than a confusing mismatch or a slow timeout.</comment>
<file context>
@@ -54,4 +64,86 @@ describe('fake Claustrum daemon', () => {
+ return socket
+}
+
+function nextSocketData(socket: Socket): Promise<Buffer> {
+ return new Promise((resolve, reject) => {
+ socket.once('data', (chunk) => resolve(Buffer.from(chunk)))
</file context>
There was a problem hiding this comment.
Declined for this PR: the raw-socket helpers are negative-path probes; a split hello would fail the assertion, not pass it. Tracked for the post-merge hygiene pass.
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
… state machine spec Squashed from: feat(custody): add /claude-account custody on|off for vault-served fallbacks docs(custody): add the global-mode custody state machine docs(custody): record the handoff and the unresolved design questions docs(custody): record the implementation go-ahead and its three binding constraints docs(custody): source the host-write race and collapse unobservable main-slot states
…lock Squashed from: feat(core): add a secure reader and resolver for the Claustrum handle manifest feat(opencode): write our provider block into the handle manifest and resolve handles from it at every site fix(custody): harden the cross-tenant manifest lock to the shared contract docs(custody): withdraw plugin-side install into an absent main slot docs(custody): under the block, main's tombstone is a barrier precondition, not a plugin write docs(custody): reword manifest onboarding for global custody mode feat(opencode): migrate legacy custody handles into the manifest at startup fix(custody): close review findings on the landed manifest layer fix(test): restore the account fixture's gate field removed by the wording pass style(tui): reformat the custody status line touched by the wording pass fix(claustrum): keep the manifest temp file inside the lock directory so an evicted writer cannot overwrite its successor fix(claustrum): refuse a group-writable non-sticky manifest parent refactor(claustrum): one discriminator, one credential-id builder, portable sleep test(custody): pin sticky-parent, unknown-owner-key, and ugly-foreign-block behaviour docs(claustrum): explain why, not who, at the lock's pinned checks
…he per-account handler Squashed from: feat(core): persist a global claustrum mode with one locked writer feat(core): parse /claude-account claustrum|local and retire the per-account custody vocabulary refactor(core): derive vault ownership from mode, enabled OAuth, and a resolved binding docs(custody): a resolved binding of either source satisfies the ownership predicate fix(core): vault ownership accepts any resolved binding, not only manifest-sourced feat(opencode): retire the per-account custody handler in favour of the global mode test(core): retire tests of the removed per-account gate helper fix(opencode): restore the startup migration's imports and finish the custody-mode projection types test(opencode): migrate custody fixtures to the global mode and retire toggle tests test(opencode): retire the per-account custody handler tests test(opencode): drive legacy-handle migration and lock coverage from startup test(opencode): seed the global claustrum mode in the handle-blindness fixtures test(opencode): seed the global claustrum mode where fixtures relied on the per-account gate test(opencode): let fixtures choose local mode explicitly and flip the mode mid-session test(opencode): await both startup migrations before asserting the shared manifest test(custody): pin legacy gates outside ownership test(custody): synchronize concurrent manifest migration fix(core): merge partial claustrum config fields
…under custody, TUI mode control Squashed from: feat(core): split the custody tombstone into write, recognise, and refuse sets feat(opencode): mode-aware tombstone handling at loader entry and a value guard at the send boundary feat(opencode): refuse OAuth login while claustrum mode is committed fix(core,opencode): assert the bearer value is not a tombstone inside the header builders test(opencode): pin every tombstone entry path test(opencode): state the tombstone containment invariant as one assertion refactor(opencode): drop the transition override that duplicates the executor default fix(core): guard the profile fetch's bearer value against a custody tombstone docs(custody): reconcile the main-slot refusal with the C2/C2' classification test(opencode): seed legacy flag fixture docs: align global Claustrum custody guidance fix(core): normalize custody status and handles path fix(opencode): guard custody startup and login edges fix(custody): align duplicate-label detection with resolution feat(tui): show the global custody mode with one control and treat an absent mode as unknown docs(custody): the barrier's eligibility step names the main-slot precondition fix(core): guard persistent custody mode fix(tui): retain account projection on apply failure test: harden custody hygiene checks docs: align global custody guidance
… policy and the identity fence Squashed from: feat(core): mode-aware vault ownership prevents local refresh inside the manager feat(opencode): project per-account custody state into status feat(opencode): vault-only routing under claustrum with the cold-route policy test(opencode): retire sidecar-fallback tests superseded by the cold-route policy test(opencode): re-express cold-vault coverage as absence from the candidate set test(opencode): expect the projected custody state labels feat(custody): serve warmed main bindings fix(custody): fence vault-owned refreshes fix(core): corrupt-binding is a malformed entry, not an unreadable manifest feat(custody): derive identity mismatch from vault account id test(custody): cover vault account identity mismatch test(custody): pin distinct main refusal guidance
…ord version Squashed from: feat(opencode): fence main vault 401 reports to the send-time record version feat(opencode): bind main identity to the vault credential and extend the identity fence to main test(opencode): fence tombstones before refresh chore(custody): use the status label helper for cold rows and index sticky fallbacks once test(opencode): fence stale main vault 401 reports test(opencode): show unknown main custody identity test(opencode): classify main custody refusals
…er brand the slot key as a provider uuid Squashed from: feat(opencode): bind main quota identity to vault credential fix(opencode): never brand the local slot key as a provider uuid fix(opencode): preserve served fallback uuid in quota feed
…re-login through the plugin Squashed from: feat(custody): remove local manifest bindings safely feat(custody): gate manifest clear on local login proof feat(custody): reject unobservable local logins fix(custody): preserve loader fast path without completion test(custody): mutation-pin manifest removal and two-factor clear feat(custody): clear bindings after fallback logins feat(custody): persist local divergence fence fix(custody): satisfy local login lint fix(custody): fence every manifest clear path fix(custody): narrow authoritative OAuth account fix(custody): use served vault version for TUI fence test(custody): pin fence persistence ordering test(custody): pin auth content guard ordering fix(custody): type review regression tests test(custody): consume main completion after refusal test(custody): guard fresh local install takeover test(custody): TUI fallback re-login fences at the served vault record version refactor(opencode): share the Claustrum 401 relay hook between main and fallback fix(opencode): cli builds custody credential ids through the canonical helper refactor(opencode): cli fence baseline goes through the never-served path of the helper
…rrier, generic vault-main route, structural gate Squashed from: feat(custody): add preflight startup reconcile fix(custody): type preflight contracts feat(custody): add takeover rollback coordinator feat(custody): add live coordinator adapters feat(custody): reconcile startup before refresh fix(custody): takeover main lock is the production main refresh lock fix(custody): resolve the manifest path from loaded storage fix(custody): reject mismatched vault credential identities fix(custody): tolerate null storage during divergence preflight test(custody): update main lock contention expectations fix(custody): isolate live adapter test state test(custody): equal-UUID vault fallback waits for readiness fix(custody): reconcile every OAuth loader state fix(custody): require operator-migrated main fix(custody): route vault main through resolver fix(custody): bound the startup vault warm so a cold vault never blocks readiness test(custody): ruled-row fixtures for fallback Claustrum credential resolution (batch A, part 1) test(custody): ruled-row fixtures for fallback Claustrum credential resolution (batch A, part 2) test(custody): ruled-row fixtures for fallback Claustrum credential resolution (batch A, part 3) test(custody): share the ruled-row fixture helper test(custody): assert manifest stays off request path test(custody): migrate vault 401 fixtures test(custody): cover vault expiry fixture paths test(opencode): migrate ruled custody rows batch c1 test(opencode): migrate ruled custody rows batch c2 test(custody): restore the shared fallback fixture quota shape test(custody): ruled-row fixture boots from the storage path the plugin reads test(opencode): rule TUI divergence fixture test(opencode): rule credential blindness rows test(opencode): rule handle report dedupe test(opencode): preserve ruled handle sentinels test(opencode): rule handle blindness rows test(opencode): rule sticky vault rows test(opencode): rule killswitch vault refresh test(opencode): use ruled fixture wrapper test(opencode): type ruled custody fixture fix(custody): the construction-phase custody gate is structural; vault residency is route darkness test(custody): vault reauth row boots in the ruled state test(custody): type the ruled-row fixture
…ck, and an all-refusals preflight Squashed from: feat(custody): commit takeover writes mode last feat(custody): route account mode commands through takeover test(custody): update mode transition fixtures test(custody): prove takeover write fences fix(custody): skip absent vault credential ids feat(custody): report every takeover refusal fix(custody): a failed post-commit read-back reverts the mode before restoring sidecars
…de-write capability, manifest hygiene, identity attribution Squashed from: fix(custody): clear refused vault main access fix(custody): privatize mode write capability fix(custody): preserve non-expiring vault credentials fix(tui): render custody mismatch verdicts fix(custody): clear duplicate manifest bindings fix(custody): replace corrupt manifest bindings fix(cli): validate account labels before saving fix(custody): reap stale manifest lock quarantines fix(custody): a transient manifest refusal keeps the completed local login pending fix(custody): align cache-result and dialog payload types after the expiry and mismatch changes fix(custody): bind fallback identity to served account fix(custody): preserve bootstrap fallback identity fix(quota): retain newer main refresh slot fix(custody): prefer current sidebar quota credential
… residency, cold-tick recovery Squashed from: fix(opencode): drop credential cache test seam test(custody): keep fallback residency route-local test(custody): recover cold vault quota refreshes on tick test(custody): keep bound real fallback dark test(custody): the takeover command never calls the host auth writer test(custody): preserve resident tick evidence
…gainst the takeover Squashed from: test(custody): fence takeover behind refresh lock fix(custody): fence stale writes over tombstones test(custody): ban process dependencies fix(custody): admit fresh login lineages
…ion out of the plugin entry Squashed from: docs(custody): name the identity fence refactor(custody): remove unused mode commit export refactor(custody): close main refusal states refactor(custody): move takeover command orchestration refactor(custody): extract custody dimensions
Conflicts resolved: PR cortexkit#198 owns the feed schema, entry projection, validator, lease reaping and the anthropic_account_uuid semantics; this branch keeps the vault-identity plumbing (ProviderAccountUuid brand, quotaKey/providerAccountUuid separation, vault account_id as identity). Includes the post-merge type-contract completion.
…t with no local material is vault-owned when bound Squashed from: test(custody): record fresh install routing gap test(custody): cover mixed-version account dialogs fix(custody): admit bound fallback roster rows fix(custody): classify bound empty fallbacks as vault owned fix(custody): route resident vault fallbacks before quota test(custody): type first-run status capture test(custody): cover cold main fallback routing test(custody): pin vault 401 provenance test(custody): document cold main refusal coverage test(custody): exclude sidecar 401 reports test(custody): cover local exit refusal test(custody): cover fresh fallback re-login test(custody): cover re-entry after fallback login
…ient manifest refusals stay pending Squashed from: fix(custody): revert mode when committed verifier throws fix(custody): defer transient manifest refusal retries test(custody): remove ineffective mode-write option
…e Claustrum daemon Squashed from: test(e2e): runnable fake Claustrum daemon fixture test(e2e): custody mode serves main and fallback from the vault test(e2e): a cold vault main is a typed refusal, not a transport test(e2e): report fallback vault auth failures test(custody): pin fallback 401 reporting
…, identity projection, tombstone metadata freshness Squashed from: fix(custody): apply policy to empty vault fallbacks fix(custody): main is not projected vault-served without identity evidence fix(custody): serialize local exit with takeover fix(custody): retain stored tombstone metadata fix(custody): merge tombstone metadata by freshness test(custody): type the identity projection fixture test(custody): complete identity manifest fixture
Squashed from: docs: document global claustrum custody docs: define custody matrix axes
0d7823c to
6c01a04
Compare
…tore creation before locking; one missing-credential predicate Squashed from: fix(e2e): capture reported provider status fix(e2e): reject unassigned claustrum routes fix(core): skip quota refresh during custody verification fix(custody): create local store before locking refactor(custody): centralize missing credential check refactor(identity): centralize provider UUID casts docs: tighten Claustrum custody guidance docs(custody): distinguish the cold main case docs(identity): explain persisted UUID naming docs(custody): describe the latched main case without settling §12.4; name the persisted uuid field's owners fix(core): a fallback quota poll skips while another process verifies that account's custody test(opencode): scope the custody manifest env to the test that sets it fix(custody): do not name a vault import verb that does not ship yet test(core): isolate the account storage path in every core test test(opencode): never detect the host's live Claustrum connection file from tests
6c01a04 to
5d42c6c
Compare
2 participants
Summary
Adds one global custody mode with two commands:
Bare
/claude-accountreports status. The TUI exposes the same single mode control.Under
claustrum, every enabled OAuth route, including main, is served from the Claustrum vault through a handle manifest written by Claustrum tooling. The plugin never runsck, imports or migrates credentials, or writes the hostauth.jsonslot.Main must already be onboarded into the vault with Claustrum's tooling before
claustrumis entered; the takeover preflights every account and changes nothing if any account refuses. All refusals are listed.Dependency (blocks activation of the main flip, not review): the dedicated Claustrum import verb for main (
migrate-pluginwith an operator gate for the main label) is not in any deployed or in-flight Claustrum build as of 2026-09-07 — it is queued on the Claustrum side behind their PR #33. Until it lands, main onboarding follows Claustrum's runbook; fallback-only custody does not depend on it.Stacked on #198
Branch:
feat/custody-toggle· head5d42c6c· 20 commits plus the #198 merge.This is stacked on PR #198 (
feat/feed-account-uuid), merged at6b5d9fc. GitHub cannot base one fork branch on another, so the base displays asmain. Review this diff against #198's head82105d3: 55 files, +18,955/−3,968.Implemented against
docs/custody-state-machine.mdper the go-ahead. Integration, activation, migration, and release stay with the maintainer.Behaviour
Mode
Fallback sidecars are tombstoned by takeover:
{type:"oauth", access:"", refresh:"claustrum-tombstone:v1:anthropic", expires:0}.Empty access fails the vault sealer's shape gate by construction. Tombstones use a write/recognise/refuse split, with
refusal ⊋ recognitionpinned as one assertion. The refuse guard is in the innermost header builders: quota poll, prewarm, prime, profile, and send.Mode is committed last, with byte-identical rollback at every phase.
Serving
• Cold vault at boot is typed
FAIL_CLOSED: all OAuth routes hold until the next viable boot. A healthy fallback is held too; whether a per-handle cold main should instead degrade to fallback-only serving remains open in §12.4.• A main that goes cold after a warm boot returns typed
claustrum_main_unavailable. There is no sidecar fallback and no tombstone bearer.• Cold fallback is excluded per request.
• A vault credential whose
account_idmismatches persisted identity is refused. Main without identity evidence projectsunknown-identity.Exit
localleaves main in interactive re-login. A fallback's manifest binding clears only after a plugin-owned login completes and new material is observed. A stale in-flight refresh cannot resurrect real material over a tombstone. Re-enteringclaustrumfor that re-logged label refusesbinding_missinguntil a fresh operator import under--replace.Fresh install
A rostered OAuth account with no local material and a resolved binding is vault-owned. Loader, classifier, routing admission, and quota policy all use the same
hasNoLocalCredentialpredicate. A quota- or killswitch-exhausted vault row is still excluded.401s
A vault-served 401 reports
report_auth_failurewith the served record version andreporter_source. The report is suppressed when the resident cache has already advanced past that served version. Sidecar-served 401s never report.Commit shape
Verification
At
fce9a0c(tree byte-identical to the squashed history; final head5d42c6cadds the verb-neutral onboarding text and the cross-process quota-poll fence): opencode 1790/0 · core 177/0 · e2e 35 pass / 1 todo / 0 fail · typecheck 0 · lint and format clean.check:claustrum-goldenis IDENTICAL (0e9dee7). Pack dry-runs include custody modules and exclude tests, golden files, and captures.aft_inspectreports 0 diagnostics, dead code, unused exports, and cycles.New coverage includes the 36-row reconcile matrix and takeover rollback, hermetic fresh-install coverage (10 tests, 5 scenarios), and a real
opencode runprocess against a fake Claustrum daemon on the subc wire (packages/e2e-tests/src/mock-claustrum.ts), covering vault-served main and fallback, boot-cold hold, and served-version 401 reporting.Every production fix carries a red-first test and a named production mutation. Remaining open review threads are P3 test-hygiene items, declined; the two feed-schema threads are moot after the #198 merge and closed.
Merge-order dependencies
v1.22.0and does not contain them; on its own, those paths still read the sidecar token, which under custody is a tombstone — the header-builder guard refuses locally, so custody accounts lose cache keepalive and priming until fix(prime): send with the vault credential for a vault-served fallback #197/fix: prewarm and profile hydration use the vault credential for a vault-served fallback #199 are on top. No conflict expected (disjoint hunks); they were merged together in the live dev tree.credential.get— ifrecord_versionadvanced, retry the same account once; otherwise report. Lands with thecredential_iddecode in a follow-up before fleet activation.Open questions
• §12.4: should a per-handle cold MAIN at boot degrade to fallback-only serving instead of the global hold? Not decided here.
• The late-cold main e2e row is a
test.todo: a real process has no clock seam to expire the resident cache deterministically.• cortexkit/claustrum#37:
credential_idis omitted fromcredential.getby design; identity comparison relies onaccount_id.• The main import verb on the Claustrum side (see Dependency above) — this PR names no verb; the exact command gets pinned in the README once it ships.
Authoritative docs:
docs/custody-state-machine.mddefines every matrix letter in §3. README custody sections now use global-mode vocabulary only.