opencode: persist custody telemetry to a bounded file, and say so when serving - #35
opencode: persist custody telemetry to a bounded file, and say so when serving#35iceteaSA wants to merge 4 commits into
Conversation
…n serving Custody logs previously went to the OpenCode pty and were not persisted. Add an on-by-default JSONL file sink with the configured or XDG state path, private permissions, one-generation 5 MiB rotation, and fail-open telemetry degradation. Configuration decisions and the first successful serve per provider are logged with bounded structured fields, while the secret-absence canary proves parser error text cannot enter the file.
![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 6 files
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/log.ts">
<violation number="1" location="packages/opencode/src/log.ts:97">
P2: When the file is near 5 MiB, this append can push it over the limit and leave it oversized until another event occurs. Rotate based on existing size plus the next line's byte length before appending.</violation>
</file>
<file name="packages/opencode/src/plugin.ts">
<violation number="1" location="packages/opencode/src/plugin.ts:396">
P2: A split provider (custody handle present with a real local credential) is logged with both `state: "split"` and `state: "unmanaged"`. The split branch never continues, so control falls through to the `unmanaged` line after setting the throwing fetch. These are contradictory states and this also breaks the PR's stated "one info line per provider" contract. Only log `unmanaged` for providers that are not split, e.g. move `log.info({ provider, state: "unmanaged" })` into an `else` of the `owner === OUR_PLUGIN_ID` branch (or `continue` at the end of that branch).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| initialized = true; | ||
| } | ||
| rotateIfNeeded(); | ||
| appendFileSync(path, `${JSON.stringify(fileEntry(entry))}\n`, { mode: 0o600 }); |
There was a problem hiding this comment.
P2: When the file is near 5 MiB, this append can push it over the limit and leave it oversized until another event occurs. Rotate based on existing size plus the next line's byte length before appending.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/log.ts, line 97:
<comment>When the file is near 5 MiB, this append can push it over the limit and leave it oversized until another event occurs. Rotate based on existing size plus the next line's byte length before appending.</comment>
<file context>
@@ -32,12 +41,78 @@ function defaultSink(entry: CustodyLogEntry): void {
+ initialized = true;
+ }
+ rotateIfNeeded();
+ appendFileSync(path, `${JSON.stringify(fileEntry(entry))}\n`, { mode: 0o600 });
+ chmodSync(path, 0o600);
+ } catch {
</file context>
| fetch: async () => { throw error; }, | ||
| }; | ||
| } | ||
| log.info({ provider, state: "unmanaged" }); |
There was a problem hiding this comment.
P2: A split provider (custody handle present with a real local credential) is logged with both state: "split" and state: "unmanaged". The split branch never continues, so control falls through to the unmanaged line after setting the throwing fetch. These are contradictory states and this also breaks the PR's stated "one info line per provider" contract. Only log unmanaged for providers that are not split, e.g. move log.info({ provider, state: "unmanaged" }) into an else of the owner === OUR_PLUGIN_ID branch (or continue at the end of that branch).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/plugin.ts, line 396:
<comment>A split provider (custody handle present with a real local credential) is logged with both `state: "split"` and `state: "unmanaged"`. The split branch never continues, so control falls through to the `unmanaged` line after setting the throwing fetch. These are contradictory states and this also breaks the PR's stated "one info line per provider" contract. Only log `unmanaged` for providers that are not split, e.g. move `log.info({ provider, state: "unmanaged" })` into an `else` of the `owner === OUR_PLUGIN_ID` branch (or `continue` at the end of that branch).</comment>
<file context>
@@ -369,36 +371,42 @@ export function createOpencodeClaustrumPlugin(dependencies: ConfigHookDependenci
fetch: async () => { throw error; },
};
}
+ log.info({ provider, state: "unmanaged" });
continue;
}
</file context>
The file sink landed alongside a console sink that still carried every level, and the new info lines were the plugin's first happy-path output ever — so they surfaced straight into the OpenCode TUI (three "serving" lines per boot). The console was quiet before by accident, not design. Console now carries warn/error only; info/debug are file-only. If the file is unavailable the one-shot warning says so and those levels are dropped rather than redirected to the screen. Pinned by inverting the test that had documented the old routing; mutation (info back to console.log) is RED.
|
Pushed Now: console carries Pinned by inverting the test that had documented the old routing ( Verified on the built bundle: exercising the config hook with stdout and stderr captured separately gives 0 stdout lines, 0 stderr lines, 3 file lines ( |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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/tests/log.test.ts">
<violation number="1" location="packages/opencode/src/tests/log.test.ts:40">
P3: This test calls `createLogger()` with no sink, so it constructs a real `createFileLogSink()` that writes every logged record - including the `state:"serving"` line - to the process's actual default path (`$XDG_STATE_HOME` or `~/.local/state/cortexkit/opencode-plugin/custody.jsonl`). The test is not hermetic: it pollutes the developer's real custody log and silently depends on that path being writable. In an environment where the default path is unwritable (e.g. a read-only HOME), the file sink's fail-open path emits a console.error warning on the first write, changing `errorLines` to length 3 and making `expect(errorLines).toHaveLength(2)` plus `errorLines[0]`/`errorLines[1]` assertions fail. Since this test only exercises console routing, route the records explicitly instead of the default logger so the real filesystem is not touched.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| test("the console sink carries only faults: info and debug never reach stdout or stderr", () => { | ||
| // The console is the OpenCode TUI's screen. Happy-path telemetry surfacing | ||
| // there is the defect this pins (2026-09-05: three "serving" lines per boot in the TUI). | ||
| const real = createLogger(); |
There was a problem hiding this comment.
P3: This test calls createLogger() with no sink, so it constructs a real createFileLogSink() that writes every logged record - including the state:"serving" line - to the process's actual default path ($XDG_STATE_HOME or ~/.local/state/cortexkit/opencode-plugin/custody.jsonl). The test is not hermetic: it pollutes the developer's real custody log and silently depends on that path being writable. In an environment where the default path is unwritable (e.g. a read-only HOME), the file sink's fail-open path emits a console.error warning on the first write, changing errorLines to length 3 and making expect(errorLines).toHaveLength(2) plus errorLines[0]/errorLines[1] assertions fail. Since this test only exercises console routing, route the records explicitly instead of the default logger so the real filesystem is not touched.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/log.test.ts, line 40:
<comment>This test calls `createLogger()` with no sink, so it constructs a real `createFileLogSink()` that writes every logged record - including the `state:"serving"` line - to the process's actual default path (`$XDG_STATE_HOME` or `~/.local/state/cortexkit/opencode-plugin/custody.jsonl`). The test is not hermetic: it pollutes the developer's real custody log and silently depends on that path being writable. In an environment where the default path is unwritable (e.g. a read-only HOME), the file sink's fail-open path emits a console.error warning on the first write, changing `errorLines` to length 3 and making `expect(errorLines).toHaveLength(2)` plus `errorLines[0]`/`errorLines[1]` assertions fail. Since this test only exercises console routing, route the records explicitly instead of the default logger so the real filesystem is not touched.</comment>
<file context>
@@ -34,20 +34,23 @@ describe("custody logger", () => {
+ test("the console sink carries only faults: info and debug never reach stdout or stderr", () => {
+ // The console is the OpenCode TUI's screen. Happy-path telemetry surfacing
+ // there is the defect this pins (2026-09-05: three "serving" lines per boot in the TUI).
+ const real = createLogger();
+ real.debug({ provider: "deepseek", state: "available" });
+ real.info({ provider: "deepseek", state: "serving" });
</file context>
|
Gated green at The diagnosis is right and worth stating plainly, because it is the part that makes the rest necessary: telemetry going to a pty that nothing persists is telemetry that does not exist, and a plugin silent on the happy path cannot be distinguished from a plugin that is not running. I reviewed the security claim rather than the feature, since that is my half. One thing
|
… the only door The secret-absence canary proved the sink drops errorMessage, not that a real SyntaxError on the handle-file path cannot reach an allowlisted field (a caller writing errorClass: String(error) would leak Bun's token-quoting message and every test would stay green). The canary now feeds a malformed handle file through the plugin's own config hook and asserts no handle or key reaches disk; the sink validates field shapes so a caller routing an error message into an allowlisted field cannot leak; mutation on the sink is RED. ts/pid join FILE_FIELDS so nothing is appended after the filter; existing dirs/files/rotations are chmod'd to 0700/0600; the off-switch test asserts the file never appears.
…rings; every file field has a named rule An object routed into an allowlisted field serialised whole, message and all, past the string-shape checks. Non-strings are now number|boolean only; level and ts get closed rules; the identifier validator is imported from handles.ts rather than copied a third time.
|
Both landed, two commits: The canary as you specified it cannot go RED on this branch — and that is a finding, not a dodge. The implementer drove a real malformed handle file ( So the protection is now a mechanism at the sink, which is what you asked for.
Your other three, as asked: Hermetic 158/158, |
There was a problem hiding this comment.
5 issues found across 4 files (changes from recent commits).
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/tests/log-leak.test.ts">
<violation number="1" location="packages/opencode/src/tests/log-leak.test.ts:42">
P3: The integration arm writes a fresh `custody-log-canary-<uuid>` tree under /tmp/opencode on every run and never removes it, accumulating directories in system temp. Tear it down (e.g. in a finally / afterEach) or track and remove the created dir.</violation>
<violation number="2" location="packages/opencode/src/tests/log-leak.test.ts:63">
P3: This canary is titled/claimed to prove the allowlist is "the only door," but it never parses the written JSONL record and asserts on its keys. It only checks that raw secret text is absent, so a regression that adds `errorMessage` (or any non-allowlisted field containing non-secret text) to written records would pass silently, defeating the PR's core errorMessage-exclusion invariant. Parse the line(s) and assert the record's keys stay within the allowlist (and that `errorMessage` is absent).</violation>
<violation number="3" location="packages/opencode/src/tests/log-leak.test.ts:64">
P3: The `sk-fake-secret-key` canary is vacuous: the value is only assigned (line 48) and asserted absent (line 64), but it is never written into any file or input the plugin reads, so `expect(contents).not.toContain(key)` is trivially true. Unlike `handle` (which is placed in the malformed handle file), `key` cannot leak through this path. The test therefore overstates what it guards. Either make the key reach a real input (e.g. include it in the malformed handle file or auth source) or drop the redundant assertion.</violation>
</file>
<file name="packages/opencode/src/log.ts">
<violation number="1" location="packages/opencode/src/log.ts:40">
P2: The custody-log allowlist claims "fake keys do not appear in the file," but the ERROR_CODE character-class cadmit s any string of letters/digits/`_.-` verbatim, including real secret shapes. I verified `ERROR_CODE.test("sk-fake-secret-key") === true` and a full 43-char `ckh_` handle also matches, so such a value routed into `errorCode` would be persisted unmodified. The new canary test masks this: it appends a trailing space (`errorCode: \`${key} \``) solely to force rejection, and the log-leak integration arm routes the real fault path where `errorCode` is undefined, so neither test proves a safe-char-key is rejected. This weakens the PR's core secret-leakage guarantee for a field that admits secrets composed entirely of permitted characters.</violation>
<violation number="2" location="packages/opencode/src/log.ts:45">
P2: When a credential returns `auth_required`, the file sink records `state:"invalid_shape"` instead of `state:"reauth"`. Add `reauth` to the state allowlist so recovery telemetry remains accurate.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const ISO_TIMESTAMP = /^\d{4}-\d{2}-\d{2}T[\d:.]+Z$/; | ||
| const STATES = new Set([ | ||
| "available", "transient", "cooldown", "other_owner", "orphan", "split", "unmanaged", | ||
| "refusing", "serving", "served", "gone", |
There was a problem hiding this comment.
P2: When a credential returns auth_required, the file sink records state:"invalid_shape" instead of state:"reauth". Add reauth to the state allowlist so recovery telemetry remains accurate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/log.ts, line 45:
<comment>When a credential returns `auth_required`, the file sink records `state:"invalid_shape"` instead of `state:"reauth"`. Add `reauth` to the state allowlist so recovery telemetry remains accurate.</comment>
<file context>
@@ -27,10 +31,19 @@ export type CustodyLogger = {
+const ISO_TIMESTAMP = /^\d{4}-\d{2}-\d{2}T[\d:.]+Z$/;
+const STATES = new Set([
+ "available", "transient", "cooldown", "other_owner", "orphan", "split", "unmanaged",
+ "refusing", "serving", "served", "gone",
+]);
</file context>
| "refusing", "serving", "served", "gone", | |
| "refusing", "serving", "served", "gone", "reauth", |
| ]; | ||
| const CREDENTIAL_ID = /^[A-Za-z0-9._:-]{1,128}$/; | ||
| const ERROR_CLASS = /^[A-Za-z][A-Za-z0-9_]{0,63}$/; | ||
| const ERROR_CODE = /^[A-Za-z0-9_.-]{1,64}$/; |
There was a problem hiding this comment.
P2: The custody-log allowlist claims "fake keys do not appear in the file," but the ERROR_CODE character-class cadmit s any string of letters/digits/_.- verbatim, including real secret shapes. I verified ERROR_CODE.test("sk-fake-secret-key") === true and a full 43-char ckh_ handle also matches, so such a value routed into errorCode would be persisted unmodified. The new canary test masks this: it appends a trailing space (errorCode: \${key} `) solely to force rejection, and the log-leak integration arm routes the real fault path where errorCode` is undefined, so neither test proves a safe-char-key is rejected. This weakens the PR's core secret-leakage guarantee for a field that admits secrets composed entirely of permitted characters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/log.ts, line 40:
<comment>The custody-log allowlist claims "fake keys do not appear in the file," but the ERROR_CODE character-class cadmit s any string of letters/digits/`_.-` verbatim, including real secret shapes. I verified `ERROR_CODE.test("sk-fake-secret-key") === true` and a full 43-char `ckh_` handle also matches, so such a value routed into `errorCode` would be persisted unmodified. The new canary test masks this: it appends a trailing space (`errorCode: \`${key} \``) solely to force rejection, and the log-leak integration arm routes the real fault path where `errorCode` is undefined, so neither test proves a safe-char-key is rejected. This weakens the PR's core secret-leakage guarantee for a field that admits secrets composed entirely of permitted characters.</comment>
<file context>
@@ -27,10 +31,19 @@ export type CustodyLogger = {
];
+const CREDENTIAL_ID = /^[A-Za-z0-9._:-]{1,128}$/;
+const ERROR_CLASS = /^[A-Za-z][A-Za-z0-9_]{0,63}$/;
+const ERROR_CODE = /^[A-Za-z0-9_.-]{1,64}$/;
+const LEVELS = new Set(["debug", "info", "warn", "error"]);
+const ISO_TIMESTAMP = /^\d{4}-\d{2}-\d{2}T[\d:.]+Z$/;
</file context>
| test("real malformed handle file fault path never writes handle or key", async () => { | ||
| // This integration arm proves the real config-hook path writes a fault without secrets; | ||
| // handles.ts -> parseSecretJson applies the fixed-message SecretJsonParseError upstream. | ||
| const root = join("/tmp/opencode", `custody-log-canary-${crypto.randomUUID()}`); |
There was a problem hiding this comment.
P3: The integration arm writes a fresh custody-log-canary-<uuid> tree under /tmp/opencode on every run and never removes it, accumulating directories in system temp. Tear it down (e.g. in a finally / afterEach) or track and remove the created dir.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/log-leak.test.ts, line 42:
<comment>The integration arm writes a fresh `custody-log-canary-<uuid>` tree under /tmp/opencode on every run and never removes it, accumulating directories in system temp. Tear it down (e.g. in a finally / afterEach) or track and remove the created dir.</comment>
<file context>
@@ -0,0 +1,66 @@
+ test("real malformed handle file fault path never writes handle or key", async () => {
+ // This integration arm proves the real config-hook path writes a fault without secrets;
+ // handles.ts -> parseSecretJson applies the fixed-message SecretJsonParseError upstream.
+ const root = join("/tmp/opencode", `custody-log-canary-${crypto.randomUUID()}`);
+ const config = join(root, "config");
+ const data = join(root, "data");
</file context>
| const contents = readFileSync(custody, "utf8"); | ||
| expect(contents.trim().length).toBeGreaterThan(0); | ||
| expect(contents).not.toContain(handle); | ||
| expect(contents).not.toContain(key); |
There was a problem hiding this comment.
P3: The sk-fake-secret-key canary is vacuous: the value is only assigned (line 48) and asserted absent (line 64), but it is never written into any file or input the plugin reads, so expect(contents).not.toContain(key) is trivially true. Unlike handle (which is placed in the malformed handle file), key cannot leak through this path. The test therefore overstates what it guards. Either make the key reach a real input (e.g. include it in the malformed handle file or auth source) or drop the redundant assertion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/log-leak.test.ts, line 64:
<comment>The `sk-fake-secret-key` canary is vacuous: the value is only assigned (line 48) and asserted absent (line 64), but it is never written into any file or input the plugin reads, so `expect(contents).not.toContain(key)` is trivially true. Unlike `handle` (which is placed in the malformed handle file), `key` cannot leak through this path. The test therefore overstates what it guards. Either make the key reach a real input (e.g. include it in the malformed handle file or auth source) or drop the redundant assertion.</comment>
<file context>
@@ -0,0 +1,66 @@
+ const contents = readFileSync(custody, "utf8");
+ expect(contents.trim().length).toBeGreaterThan(0);
+ expect(contents).not.toContain(handle);
+ expect(contents).not.toContain(key);
+ });
+});
</file context>
|
|
||
| const contents = readFileSync(custody, "utf8"); | ||
| expect(contents.trim().length).toBeGreaterThan(0); | ||
| expect(contents).not.toContain(handle); |
There was a problem hiding this comment.
P3: This canary is titled/claimed to prove the allowlist is "the only door," but it never parses the written JSONL record and asserts on its keys. It only checks that raw secret text is absent, so a regression that adds errorMessage (or any non-allowlisted field containing non-secret text) to written records would pass silently, defeating the PR's core errorMessage-exclusion invariant. Parse the line(s) and assert the record's keys stay within the allowlist (and that errorMessage is absent).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/tests/log-leak.test.ts, line 63:
<comment>This canary is titled/claimed to prove the allowlist is "the only door," but it never parses the written JSONL record and asserts on its keys. It only checks that raw secret text is absent, so a regression that adds `errorMessage` (or any non-allowlisted field containing non-secret text) to written records would pass silently, defeating the PR's core errorMessage-exclusion invariant. Parse the line(s) and assert the record's keys stay within the allowlist (and that `errorMessage` is absent).</comment>
<file context>
@@ -0,0 +1,66 @@
+
+ const contents = readFileSync(custody, "utf8");
+ expect(contents.trim().length).toBeGreaterThan(0);
+ expect(contents).not.toContain(handle);
+ expect(contents).not.toContain(key);
+ });
</file context>
1 participant
The custody plugin's telemetry was going nowhere. It logs structured JSON via
console.log/console.error; under the OpenCode TUI that is the pty, which nothing persists —opencode.loghad 0 such lines, the daemon journal had 0. And the plugin was silent on the happy path (1 debug / 0 info / 2 warn / 2 error call sites), so "no news" was indistinguishable from "not running". The only witness that it was serving at all was the vault's audit chain, which only works when one operator owns both ends.What this adds
A bounded JSONL file sink, on by default, alongside the console sink.
$CLAUSTRUM_CUSTODY_LOGif set, else${XDG_STATE_HOME:-~/.local/state}/cortexkit/opencode-plugin/custody.jsonl. Dir 0700, file 0600.<path>.1past 5 MiB; one generation kept.CLAUSTRUM_CUSTODY_LOG=off|0|false|nodisables it.Two happy-path lines, both bounded.
confighook, oneinfoper provider with the cell decision in the plugin's existing vocabulary (serving/ refusal states).infowith{provider, label, credentialId, recordVersion, state:"served"}. Never per request.The property that matters: the file cannot carry a secret
Entries written to the file pass through an explicit allowlist (
FILE_FIELDS: level, provider, label, credentialId, recordVersion, state, httpStatus, cooldownUntil, errorClass, errorCode).errorMessageis not in it — Bun'sJSON.parsequotes adjacent tokens into its error message, which is how a hand-edited handle file leaks a bearer handle (fixed once already in #28). No free-text field reaches disk.Pinned by a canary that drives a fault path whose error text contains a fake 47-char
ckh_handle and a fake key, then asserts neither appears in the file. Mutation-proved independently of the implementer: addingerrorMessageto the written record (exactly one site) turns it red —— byte-identical restore, 7 pass.
Verified on the installed bundle
Exercised the built bundle's config hook in-process against a scratch
XDG_STATE_HOMEon the live box: wrote{"level":"info","provider":"minimax-coding-plan","state":"serving","ts":…,"pid":…}(the one provider under custody here), file 600 / dir 700, secret-shape scan clean. Bundle shape unchanged: single{id, server}default export, Node builtins only.Hermetic 152/152, gate green, exit census updated (plugin 33/41, serve 13/15).
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Custody telemetry previously went to the OpenCode pty and was lost; the plugin was also silent on the happy path. This persists custody logs to a bounded JSONL file (on by default), logs configuration decisions plus the first successful serve per provider, and routes the console to warn/error only so happy-path lines no longer surface in the TUI.
File sink
$CLAUSTRUM_CUSTODY_LOGif set, else~/.local/state/cortexkit/opencode-plugin/custody.jsonl; dir 0700, file 0600.<path>.1past 5 MiB, keeping one generation;CLAUSTRUM_CUSTODY_LOG=off|0|false|nodisables it.Log lines and secret safety
serving,orphan,split,unmanaged,refusing,other_owner); the first successful serve per provider per process logs{provider, label, credentialId, recordVersion, state:"served"}— never per request.errorMessageis excluded so error text from malformed handle files cannot leak a bearer handle to disk.invalid_shape; a canary drives a malformed handle file through the real config hook to pin this.Written for commit ee4807e. Summary will update on new commits.