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 8 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 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>
|
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
|
|
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.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
All five, in Field rules now match what the producers emit, not a character class:
P3s: the integration arm removes its Hermetic 160/160, |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Both, in Rules now: The population is now mechanical rather than typed: the source-scan test collects every Secret table (all rejected by both rules, checked outside the suite as well): The integration file now has the arm you asked for: through the real logger and file sink, Hermetic 161/161, |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Correct observation, and the answer is a ruling rather than a fix:
Hermetic 162/162, |
|
Re-reviewed at The caller gap is closed, and the second test is what closes itMy earlier point was that the protection was a convention held at four call sites: the canary hand-built its entry, so it proved the sink drops That one hand-builds too, and that is correct now. Before, the sink dropped one field and the caller decided what went in the others, so only a real fault path could tell you what a caller does. Now every field passes a rule, so the mechanism is the rule and a hand-built input is the right way to exercise it. The distinction is worth stating because "hand-built" was my criticism and it stopped being one when the thing under test changed.
My finding survives, moved fields, and my example expiredI said "every secret shape fails both" overshoots because But Reaching it still needs a caller to put a hex string in one of those fields, and no producer does — the wire classes are If you want it closed for free: reject an all-hex body. Every real value on both sides has a non-hex letter or an underscore, so the rule costs nothing and removes the class rather than narrowing it. Your call. And the general shape is worth more than the instance: a length bound is not a shape bound. Tightening 32 to 24 makes the window smaller and leaves it open, while widening a sibling rule to accept a new legitimate form can re-open it on another field with nothing to notice — which is what happened between two commits here. The gate fails on a stale lockfile again, not on your changeThree waves since your branch point, on a PR that touches no Rust. Same as last time — a rebase clears it. Worth knowing that One thing I would still change before merge
|
…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.
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.
… 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.
…d from producers ERROR_CODE and ERROR_CLASS admitted hyphen/underscore/long-token shapes that real keys and handles satisfy, and the canary forced rejection with a trailing space instead of a real value. Rules now match what the producers emit (PascalCase error names; upper- or lower-snake codes, max 32, one case class); the canary uses realistic key/handle shapes with no dodge. STATES is the exact producer set, pinned by a source scan. Integration arm parses records, asserts keys within FILE_FIELDS, and cleans its temp tree.
…ield-rule population pinned by source scan ERROR_CLASS admitted only Error.name values, so the freshness and ownership paths (credential_warm, transient, auth_required, other_owner) were written as invalid_shape — the rule matched half the producers. Both errorClass and errorCode now accept a PascalCase name or a lower-snake token (max 24; a 32-char hex key no longer fits), and a source scan pins every literal producer plus the wire ErrorClass set against the rules so the population cannot drift again.
…al pinned A short lowercase underscore token (sk_fake_secret) has the same shape as not_found and no rule admitting codes can reject it. The canary now claims realistic credential shapes, and a separate test pins the residual as written-verbatim so a future rule that rejects it also visibly rejects real codes.
…is process-generated only A length bound is not a shape bound: capping the snake arms at 24 moved the hex-token window, and admitting the snake wire classes in errorClass reopened it on a second field. Both rules now reject an all-hex (or all-digit) body outright — no real class or code is hex — which removes the class instead of narrowing it. The pre-filter spread in fileEntry is documented as process-generated values only.
9efc44d to
c117b21
Compare
|
Rebased onto Hex class: closed, your way. Both rules now reject an all-hex (or all-digit) body outright — Your general shape is the thing I am keeping: a length bound is not a shape bound. The hex window was moved by one commit and re-opened on a sibling field by the next, and nothing in the suite could see it because the tests were checking lengths. A shape predicate closes the class; a cap only makes it smaller. Pre-filter spread: one line at the site — the additions before the filter are process-generated ( Rebase: the branch was three lock waves stale, as you said. Rebased with siblings pinned to what master's lock declares ( Hermetic 162/162, |
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, with existing dirs and files tightened to those modes on first write.<path>.1past 5 MiB, keeping one generation;CLAUSTRUM_CUSTODY_LOG=off|0|false|nodisables it.tsandpidmetadata.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.errorClassPascalCase or lower-snake,errorCodeupper- or lower-snake,statepinned by a source scan of producers); non-string values must be numbers or booleans, all-hex bodies are rejected, and anything else is written asinvalid_shape. A canary drives a malformed handle file through the real config hook to pin this.sk_fake_secret) are written as-is because they are indistinguishable from real error codes.Written for commit c117b21. Summary will update on new commits.