Compare commits

...

2 Commits

Author SHA1 Message Date
Michal
ff0e71da05 Merge 'fix(opencode): guard state parsing, lint the .tsx, correct an overstated doc claim' into main
Some checks failed
CI/CD / lint (push) Successful in 1m17s
CI/CD / test (push) Successful in 1m28s
CI/CD / typecheck (push) Successful in 2m58s
CI/CD / build (push) Successful in 2m21s
CI/CD / smoke (push) Failing after 3m8s
CI/CD / publish (push) Has been skipped
2026-08-09 20:47:45 +01:00
Michal
cbd3b95d97 fix(opencode): guard state parsing, lint the .tsx, correct an overstated doc claim
Three findings from a cross-branch review of the competing opencode
implementations, all of which are fair.

1. `readState` type-guards the parsed JSON now. A bare try/catch does not
   cover it: `JSON.parse('null')` succeeds and returns null, so the catch never
   fires and the next `state.project` throws a TypeError that takes the plugin
   down. Verified the crash before fixing; a test pins the guard in the embedded
   copies. Credit to the competing 'opencode-mine' branch, which had this right.

2. eslint now covers `src/opencode-ext/*.tsx`. The glob was `*.ts` only, so the
   300-line TUI plugin — the largest file in the addon — was linted by nothing.
   It was typechecked, which is why this went unnoticed. Confirmed the rules
   actually fire on it rather than the file being silently skipped. The
   'abhishek' branch was the only entry that got this right.

3. docs/opencode-extension.md overstated the security argument. "The token would
   sit in a 0644 opencode.json" is not a point against a `type: local` stdio
   bridge, which needs no token at all because it reads your own credentials.
   That reason is a consequence of having picked the HTTP gateway, not a
   justification for it. The docs now lead with the real reason — live
   re-pointing without a restart — and state the trade honestly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVwuCjuMoA13gmzYEfcrNP
2026-08-09 20:47:45 +01:00
6 changed files with 47 additions and 11 deletions

View File

@@ -41,13 +41,26 @@ with no restart.
### The state file, not `opencode.json` ### The state file, not `opencode.json`
Two reasons the project does not live in opencode's own config: 1. **The restart.** A config file is read at startup. Re-pointing the mount
through the running server's MCP API is what makes `/mcpctl` instant. This is
the load-bearing reason.
2. **The token.** Having chosen the gateway, we need
`Authorization: Bearer <mcpctl PAT>` somewhere. `opencode.json` is a mode-0644
file people paste into bug reports; `~/.mcpctl/opencode-state.json` is 0600,
like every other mcpctl credential.
1. **The token.** The gateway needs `Authorization: Bearer <mcpctl PAT>`. > **Reason 2 is not an argument for this design over the alternative.** A
`opencode.json` is a mode-0644 file people paste into bug reports; > `type: "local"` entry running `mcpctl mcp -p <project>` — the same stdio
`~/.mcpctl/opencode-state.json` is 0600, like every other mcpctl credential. > bridge `config claude` uses — needs no bearer token at all, because the bridge
2. **The restart.** A config file is read at startup. Re-pointing the mount > reads your own `~/.mcpctl/credentials`. So "no secret in a 0644 file" is not a
through the running server's MCP API is what makes `/mcpctl` instant. > point against that approach; it is just a consequence of having picked the
> HTTP gateway.
>
> The honest trade is: the gateway works against a remote mcpctl with no local
> `mcplocal` daemon, and mounts through an API that can be re-pointed live. The
> stdio bridge is simpler and credential-free, but requires `mcpctl` and a
> reachable mcplocal on the same machine. Both are defensible; this one was
> chosen for the remote case and for the live re-point, not for the token.
Tokens are kept **per project**, so switching back to a project you have Tokens are kept **per project**, so switching back to a project you have
already used needs no new mint — and a failed mint for project B cannot cost already used needs no new mint — and a failed mint for project B cannot cost

View File

@@ -3,7 +3,7 @@ import tsparser from '@typescript-eslint/parser';
export default [ export default [
{ {
files: ['src/*/src/**/*.ts', 'src/pi-ext/*.ts', 'src/opencode-ext/*.ts', 'src/prime-agent-ext/*.ts'], files: ['src/*/src/**/*.ts', 'src/pi-ext/*.ts', 'src/opencode-ext/*.ts', 'src/opencode-ext/*.tsx', 'src/prime-agent-ext/*.ts'],
languageOptions: { languageOptions: {
parser: tsparser, parser: tsparser,
parserOptions: { parserOptions: {

File diff suppressed because one or more lines are too long

View File

@@ -88,3 +88,16 @@ describe('embedded opencode plugins', () => {
expect(OPENCODE_TUI_PLUGIN_SOURCE).toContain("'--skip-marker'"); expect(OPENCODE_TUI_PLUGIN_SOURCE).toContain("'--skip-marker'");
}); });
}); });
describe('embedded opencode plugins — state parsing', () => {
it('type-guard the parsed state, not just try/catch', () => {
// `JSON.parse('null')` succeeds and returns null, so a bare try/catch lets
// it through and the next `state.project` throws a TypeError that takes the
// plugin down. A hand-edited or truncated state file must degrade to "no
// project", never to a broken opencode.
for (const src of [OPENCODE_SERVER_PLUGIN_SOURCE, OPENCODE_TUI_PLUGIN_SOURCE]) {
expect(src).toContain("typeof parsed === 'object' && parsed !== null");
expect(src).not.toMatch(/return JSON\.parse\(await readFile\([^)]*\)\) as OpencodeState;/);
}
});
});

View File

@@ -57,7 +57,12 @@ function statePath(): string {
async function readState(): Promise<OpencodeState> { async function readState(): Promise<OpencodeState> {
try { try {
return JSON.parse(await readFile(statePath(), 'utf-8')) as OpencodeState; const parsed: unknown = JSON.parse(await readFile(statePath(), 'utf-8'));
// Type-guard, not just try/catch: `JSON.parse('null')` succeeds and returns
// null, so the catch never fires and the next `state.project` throws a
// TypeError that takes the plugin down. A truncated or hand-edited state
// file must degrade to "no project", never to a broken opencode.
return typeof parsed === 'object' && parsed !== null ? (parsed as OpencodeState) : {};
} catch { } catch {
return {}; return {};
} }

View File

@@ -40,7 +40,12 @@ function statePath(): string {
async function readState(): Promise<OpencodeState> { async function readState(): Promise<OpencodeState> {
try { try {
return JSON.parse(await readFile(statePath(), 'utf-8')) as OpencodeState; const parsed: unknown = JSON.parse(await readFile(statePath(), 'utf-8'));
// Type-guard, not just try/catch: `JSON.parse('null')` succeeds and returns
// null, so the catch never fires and the next `state.project` throws a
// TypeError that takes the plugin down. A truncated or hand-edited state
// file must degrade to "no project", never to a broken opencode.
return typeof parsed === 'object' && parsed !== null ? (parsed as OpencodeState) : {};
} catch { } catch {
return {}; return {};
} }