diff --git a/AGENTS.md b/AGENTS.md index 709adcd6..3f6ad59d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -358,8 +358,9 @@ several decisions below. The reference deployment's `ai.opencode.serve` LaunchAgent runs a binary built from `fix/subagent-effective-deny-inheritance` in the local opencode checkout (source version 1.18.23; `dev` is an ancestor ~4,145 commits behind at 1.4.7 and is not a rebuild source). - `~/.opencode/bin/opencode-1.18.23-dca-taskmodel` is the currently pinned binary; the old - `opencode-1.18.22-dca` filename is the rollback artifact, never a branch or rebuild source. + `~/.opencode/bin/opencode-1.18.23-dca.2` is the currently pinned binary; the earlier + `opencode-1.18.23-dca-taskmodel` and `opencode-1.18.22-dca` files are rollback artifacts, + never a branch or rebuild source. Verified live: a parent with appended `[bash deny, bash allow]` spawned a task child with no inherited bash deny that ran bash successfully. Session-level Plan enforcement is still required regardless — the resolved Plan agent is not read-only after project merges. @@ -370,6 +371,23 @@ several decisions below. silently move work to an expensive model; earlier ungated attempts (#26535, #29447) were closed as superseded. This fork has no such gate, so adopting that binary accepts unbounded agent-chosen model cost. Do not propose the fork's version upstream as a competing PR. + **A fork build must say so in its version string.** Build it as + `OPENCODE_VERSION=+dca. OPENCODE_CHANNEL=prod`, where `` counts + the fork patch set — today `1.18.23+dca.2` (1: the deny fix; 2: the task `model` parameter). + That string is what `/global/health`, `--version`, the LLM `User-Agent`, MCP `clientInfo` and + the durable per-session `version` field all report, so a plain `1.18.23` would attribute + fork-only behaviour — a `task.model` parameter stock 1.18.23 does not have — to upstream. + Use SemVer **build metadata (`+`), never a prerelease (`-`)**: OpenCode gates plugin loading on + `semver.satisfies`, and a prerelease sorts *below* `1.18.23` and fails ordinary ranges like + `>=1.18.0`, while build metadata is stripped before comparison and behaves as the release does. + Both env vars are mandatory: without `OPENCODE_VERSION` the build stamps `0.0.0--`, + whose major of 0 silently disables the plugin engine check, and without `OPENCODE_CHANNEL=prod` + the channel is inferred and can change database and websocket behaviour. Name the executable + after the version with `+`→`-`; the filename is a convenience label, never the source of truth. + `EXPECTED_SERVER_VERSION` pins the full string including `+dca.` and is compared exactly, so + an accidental fallback to a stock binary — which reintroduces the #75 bug — is visible rather + than tolerated. Bumping `` means updating this list, the pin, and the deterministic fixtures + together. 20. **A file reference is data the server verified, never a URL the client trusted.** The client contract is `WorkspaceTarget { path, startLine?, endLine? }`, not a route: following a reference must not change the browser location, because the drawer is a diff --git a/client/simulator/publicSimulator.ts b/client/simulator/publicSimulator.ts index 0d038b3c..7a36d830 100644 --- a/client/simulator/publicSimulator.ts +++ b/client/simulator/publicSimulator.ts @@ -148,7 +148,7 @@ export function createPublicSimulator(): typeof fetch { const body = bodyOf(init); const path = url.pathname; - if (path === "/api/health") return response({ healthy: true, upstream: { url: "simulator://opencode", reachable: true, version: "1.18.23", expected: "1.18.23", versionMatches: true }, events: { connected: true } }); + if (path === "/api/health") return response({ healthy: true, upstream: { url: "simulator://opencode", reachable: true, version: "1.18.23+dca.2", expected: "1.18.23+dca.2", versionMatches: true }, events: { connected: true } }); if (path === "/api/app-config") return response({ publicAppUrl: null }); if (path === "/api/projects") return response({ root: "/tmp", projects: [{ name: "mock-project", relativePath: "mock-project", directory: SIMULATOR_DIRECTORY, kind: "repository" }, { name: "mock-second-project", relativePath: "mock-second-project", directory: SECOND_DIRECTORY, kind: "repository" }] }); if (path === "/api/project-pins") { diff --git a/docs/subagents.md b/docs/subagents.md index a0af7d86..b8583443 100644 --- a/docs/subagents.md +++ b/docs/subagents.md @@ -294,8 +294,9 @@ children. ## Model selection for delegated work Managed Children accept an explicit, validated model at launch, and the ledger shows the -requested model as provenance. The reference deployment's **forked** OpenCode 1.18.23 binary -also adds an optional `model` parameter to native `task` calls (issue #90): +requested model as provenance. The reference deployment's **forked** OpenCode binary, which +reports `1.18.23+dca.2`, also adds an optional `model` parameter to native `task` calls +(issue #90): ```text explicit task model > subagent configured model > invoking parent model @@ -326,6 +327,32 @@ The deployed fork exposes the raw parameter because the operator chose immediate over a deny-by-default cost gate. Revisit that choice when adopting an upstream implementation; do not claim that the fork's model parameter has landed in a stock release. +### Why the version string carries `+dca.` + +The fork build reports `+dca.`, where `` counts the fork patch +set. That suffix is SemVer **build metadata**, deliberately not a prerelease tag: + +| Form | Plugin `engines` ranges | Ordering vs stock | Honest about the fork | +|---|---|---|---| +| `1.18.23` | passes | equal | **no** | +| `1.18.23-dca.1` | **fails `>=1.18.0`** | **sorts lower** | yes | +| `1.18.23+dca.2` | passes | equal | yes | + +A prerelease would be rejected by `semver.satisfies`, so any plugin declaring an `engines.opencode` +range would refuse to load. Build metadata is stripped before comparison, so the build behaves +exactly like the release it is based on while still naming itself honestly in `/global/health`, +`--version`, the LLM `User-Agent`, MCP `clientInfo`, and the durable per-session `version` field. + +Rebuild with both variables set explicitly: + +```bash +OPENCODE_VERSION=1.18.23+dca.2 OPENCODE_CHANNEL=prod bun run build --single +``` + +Omitting `OPENCODE_VERSION` stamps `0.0.0--`, and a major of `0` silently +disables the plugin engine check. Omitting `OPENCODE_CHANNEL=prod` lets the channel be inferred, +which can change database selection and enable experimental websockets. + ```mermaid flowchart TD Task[Native task call] --> Requested{Explicit model supplied?} @@ -440,7 +467,7 @@ terminal Bash denies even after Build made the parent's own tools available agai allows that superseded them. A fork-only patch copies only denies that are still the parent's effective action for their exact permission and pattern; deployments running a build with that patch verified live (the reference deployment runs -`opencode-1.18.23-dca-taskmodel`, built from the +`opencode-1.18.23-dca.2`, built from the `fix/subagent-effective-deny-inheritance` branch; the binary filename is not a branch) no longer need the fresh-parent workaround. On a build without it, failed preflight means stop; do not weaken policy or silently replace the native child with an independent root session. diff --git a/scripts/dev.sh b/scripts/dev.sh index 1803a2f3..c77c6768 100755 --- a/scripts/dev.sh +++ b/scripts/dev.sh @@ -39,7 +39,11 @@ echo " ✓ ${health}" # Warn early on version skew — it is the first thing to suspect when a # response shape looks wrong. -expected=$(grep -oE 'EXPECTED_SERVER_VERSION = "[^"]+"' server/opencode/client.ts | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' || true) +# Capture the pin verbatim, including any SemVer build metadata (`+dca.`). +# A bare MAJOR.MINOR.PATCH match would drop that suffix and then disagree with +# the server's full string on every start — a warning that always fires is +# worse than none. +expected=$(grep -oE 'EXPECTED_SERVER_VERSION = "[^"]+"' server/opencode/client.ts | sed -E 's/.*"([^"]+)"/\1/' || true) actual=$(printf '%s' "$health" | grep -oE '"version":"[^"]+"' | cut -d'"' -f4 || true) if [ -n "$expected" ] && [ -n "$actual" ] && [ "$expected" != "$actual" ]; then echo " ! version skew: server ${actual}, client pinned to ${expected}" >&2 diff --git a/server/opencode/client.ts b/server/opencode/client.ts index 49d45064..73af1f95 100644 --- a/server/opencode/client.ts +++ b/server/opencode/client.ts @@ -33,8 +33,20 @@ // defeats the point of types. We own a thin typed layer over fetch instead and // treat the live `GET /doc` as the source of truth. -/** Pinned server version these types were written against. */ -export const EXPECTED_SERVER_VERSION = "1.18.23"; +/** + * Pinned server version these types were written against. + * + * The deployed server is a fork build, so this carries SemVer *build metadata* + * (`+dca.`), never a prerelease tag. Build metadata compares equal to the + * upstream release for SemVer range checks, while a prerelease would sort BELOW + * it and fail `engines`-style ranges. `` counts the fork patch set; see + * AGENTS.md decision 19 for what each one is. + * + * The comparison in `checkHealth` is deliberately exact: reverting to a stock + * binary silently reintroduces the #75 deny-inheritance bug, so that skew must + * be visible rather than tolerated. + */ +export const EXPECTED_SERVER_VERSION = "1.18.23+dca.2"; export interface OpencodeConfig { /** Base URL of the running `opencode serve` / `opencode web` instance. */ diff --git a/tests/dev-script.test.ts b/tests/dev-script.test.ts index 032d20cb..23683100 100644 --- a/tests/dev-script.test.ts +++ b/tests/dev-script.test.ts @@ -26,14 +26,30 @@ async function availablePort(): Promise { return address.port; } -async function runPreflight(password?: string): Promise<{ authorization?: string; requests: number; stdout: string; stderr: string }> { +interface PreflightOptions { + password?: string; + /** Value written into the fake `EXPECTED_SERVER_VERSION` pin. */ + pinVersion?: string; + /** Value the fake OpenCode health endpoint reports. */ + serverVersion?: string; + /** Set when the run is expected to warn on stderr. */ + allowStderr?: boolean; +} + +async function runPreflight( + passwordOrOptions?: string | PreflightOptions, +): Promise<{ authorization?: string; requests: number; stdout: string; stderr: string }> { + const options: PreflightOptions = typeof passwordOrOptions === "string" + ? { password: passwordOrOptions } + : passwordOrOptions ?? {}; + const { password, pinVersion = "1.18.21", serverVersion = "1.18.21", allowStderr = false } = options; let authorization: string | undefined; let requests = 0; const server = createServer((req, res) => { requests += 1; authorization = req.headers.authorization; res.writeHead(200, { "Content-Type": "application/json" }); - res.end(JSON.stringify({ healthy: true, version: "1.18.21" })); + res.end(JSON.stringify({ healthy: true, version: serverVersion })); }); servers.push(server); server.listen(0, "127.0.0.1"); @@ -46,7 +62,7 @@ async function runPreflight(password?: string): Promise<{ authorization?: string await mkdir(path.join(tempRoot, "scripts")); await mkdir(path.join(tempRoot, "server", "opencode"), { recursive: true }); await copyFile(path.join(root, "scripts", "dev.sh"), path.join(tempRoot, "scripts", "dev.sh")); - await writeFile(path.join(tempRoot, "server", "opencode", "client.ts"), 'const EXPECTED_SERVER_VERSION = "1.18.21";\n'); + await writeFile(path.join(tempRoot, "server", "opencode", "client.ts"), `const EXPECTED_SERVER_VERSION = "${pinVersion}";\n`); await writeFile(path.join(tempRoot, ".env"), "OPENCODE_URL=http://127.0.0.1:1\nOPENCODE_SERVER_PASSWORD=conflicting\n"); const env = { ...process.env }; @@ -67,7 +83,7 @@ async function runPreflight(password?: string): Promise<{ authorization?: string child.stdout.setEncoding("utf8").on("data", (chunk) => { stdout += chunk; }); child.stderr.setEncoding("utf8").on("data", (chunk) => { stderr += chunk; }); const [code] = await once(child, "close") as [number | null]; - expect({ code, stderr }).toEqual({ code: 0, stderr: "" }); + expect(allowStderr ? { code } : { code, stderr }).toEqual(allowStderr ? { code: 0 } : { code: 0, stderr: "" }); return { authorization, requests, stdout, stderr }; } @@ -84,4 +100,20 @@ describe("scripts/dev.sh health preflight", () => { expect(result.requests).toBe(1); expect(result.authorization).toBe(`Basic ${Buffer.from("tester:s3cret").toString("base64")}`); }); + + // The fork binary reports SemVer build metadata. Comparing only + // MAJOR.MINOR.PATCH would drop `+dca.` from the pin and warn on every + // start, training the reader to ignore the one signal that catches an + // accidental fallback to a stock binary. + it("does not report skew when the pin and server agree on build metadata", async () => { + const result = await runPreflight({ pinVersion: "1.18.23+dca.2", serverVersion: "1.18.23+dca.2" }); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain('"version":"1.18.23+dca.2"'); + }); + + it("still reports skew when only the build metadata differs", async () => { + const result = await runPreflight({ pinVersion: "1.18.23+dca.2", serverVersion: "1.18.23", allowStderr: true }); + expect(result.stderr).toContain("version skew"); + expect(result.stderr).toContain("1.18.23+dca.2"); + }); }); diff --git a/tests/e2e/mock-opencode.ts b/tests/e2e/mock-opencode.ts index 61cd784e..b0e11c91 100644 --- a/tests/e2e/mock-opencode.ts +++ b/tests/e2e/mock-opencode.ts @@ -1087,7 +1087,7 @@ function handle(req: IncomingMessage, res: ServerResponse): void { const directory = url.searchParams.get("directory"); if (pathname === "/global/health") { - return json(res, 200, { healthy: true, version: "1.18.23" }); + return json(res, 200, { healthy: true, version: "1.18.23+dca.2" }); } if (pathname === "/global/event") { diff --git a/tests/e2e/smoke.api.spec.ts b/tests/e2e/smoke.api.spec.ts index 9699dd5d..e1745b41 100644 --- a/tests/e2e/smoke.api.spec.ts +++ b/tests/e2e/smoke.api.spec.ts @@ -63,7 +63,7 @@ test.describe("health", () => { const body = await res.json(); expect(body.healthy).toBe(true); expect(body.upstream.reachable).toBe(true); - expect(body.upstream.version).toBe("1.18.23"); + expect(body.upstream.version).toBe("1.18.23+dca.2"); expect(body.upstream.versionMatches).toBe(true); }); diff --git a/tests/e2e/smoke.ui.spec.ts b/tests/e2e/smoke.ui.spec.ts index f8b30aaa..3547a356 100644 --- a/tests/e2e/smoke.ui.spec.ts +++ b/tests/e2e/smoke.ui.spec.ts @@ -75,7 +75,7 @@ test.describe("hub", () => { test("reports the upstream agent version", async ({ page }) => { await page.goto(hub); - await expect(page.getByTestId("opencode-upstream-badge")).toContainText("1.18.23"); + await expect(page.getByTestId("opencode-upstream-badge")).toContainText("1.18.23+dca.2"); }); test("shows compact directory-wide auto permissions controls", async ({ page }) => { diff --git a/tests/opencode-client.test.ts b/tests/opencode-client.test.ts index 3215ffd0..9d10add6 100644 --- a/tests/opencode-client.test.ts +++ b/tests/opencode-client.test.ts @@ -71,6 +71,6 @@ describe("eventStreamUrl", () => { describe("version pin", () => { it("matches the deliberate deployed server pin", () => { // Bump deliberately after re-auditing the live GET /doc contract. - expect(EXPECTED_SERVER_VERSION).toBe("1.18.23"); + expect(EXPECTED_SERVER_VERSION).toBe("1.18.23+dca.2"); }); }); diff --git a/tests/preview-e2e/public-simulator.spec.ts b/tests/preview-e2e/public-simulator.spec.ts index 80bd1aa5..987c91f9 100644 --- a/tests/preview-e2e/public-simulator.spec.ts +++ b/tests/preview-e2e/public-simulator.spec.ts @@ -7,7 +7,7 @@ test("serves an interactive, credential-free PR simulator", async ({ page }) => await page.waitForLoadState("networkidle"); expect(pageErrors).toEqual([]); await expect(page.getByTestId("opencode-public-simulator-banner")).toContainText("fixture data only"); - await expect(page.getByTestId("opencode-upstream-badge")).toContainText("1.18.23"); + await expect(page.getByTestId("opencode-upstream-badge")).toContainText("1.18.23+dca.2"); await expect(page.getByTestId("opencode-session-list")).toContainText("Build the PR preview pipeline"); await page.getByTestId("opencode-session-list").getByText("Build the PR preview pipeline").click();