Skip to content

AGENTS.md

Latest commit

 

History

History
201 lines (127 loc) · 45.1 KB

AGENTS.md

File metadata and controls

201 lines (127 loc) · 45.1 KB
Raw

Inherits: @~/.cc-rules/AGENTS.md

OCP — Open Claude Proxy — Agent Guidelines

Scope: the dtzp555-max/ocp repository. Audience: any AI coding agent (Claude Code / Cursor / OpenCode / Copilot / Codex / Gemini) touching OCP source.

What this project is

OCP (Open Claude Proxy) is an open-source HTTP gateway that sits between the Claude Code CLI (cli.js) and Anthropic's public API. It forwards, observes, and multiplexes traffic that cli.js already emits — it is explicitly not an extension layer. A secondary role: registering OCP as a local provider inside OpenClaw (a sibling IDE-agnostic tool), so that users running OpenClaw against OCP see the same model list as native Claude Code.

Runtime: Node.js (ESM, .mjs throughout). No build step. No bundler. server.mjs is the single executable entrypoint; ocp and ocp-connect are CLI wrappers.

Stack

  • Node.js 22.13+ (or 23.4+ on the 23 line), native ESM modules — keys.mjs imports node:sqlite at module load and nothing on any launch path passes --experimental-sqlite; see scripts/lib/node-floor.mjs
  • http/https built-ins for the proxy core (no Express, no Fastify)
  • models.json as the single source of truth for model metadata
  • GitHub Actions for CI (alignment.yml, release.yml)
  • gh CLI assumed for PR creation and release automation
  • No TypeScript. No test framework beyond test-features.mjs (run via npm test; CI workflow .github/workflows/test.yml). Keep dependencies minimal.

Key files to know

  • server.mjs — the proxy itself; every request path lives here. Governed by ALIGNMENT.md.
  • models.json — single source of truth for model IDs, aliases, and context windows. See ADR 0003.
  • models.schema.json — the schema models.json declares in its $schema. CI validates the SPOT against it (test-features.mjs) using the repo's own validateJsonSchema, so a malformed entry fails the build instead of surfacing downstream in OpenClaw.
  • setup.mjs — first-time installer; reads models.json to derive bootstrap config.
  • scripts/sync-openclaw.mjs — idempotent OpenClaw registry sync invoked by ocp update. See ADR 0004.
  • ocp — user-facing CLI (install, update, start, stop, status, logs, etc.).
  • scripts/b2-key-snapshot.mjs + docs/governance/b2-response-keys.json — the per-release record of every grandfathered Class B.2 endpoint's response key set, read from the wire. npm test boots a real server.mjs, probes every B.2 endpoint+method pair in ALIGNMENT.md's inventory, and fails on any key-path difference from the snapshot. If you add a field to a B.2 response, the suite goes red until you regenerate the snapshot (node scripts/b2-key-snapshot.mjs --write) — and regenerating is not authorization: ADR 0012 condition 5 still requires the field names in the PR body and the CHANGELOG. A removed or renamed key is not covered by ADR 0012 at all and needs its own ADR. Introduced by #346 to replace a CHANGELOG grep that could only ever see additions whose author wrote the marker. Since #357 there are TWO configuration profiles (scripts/b2-key-snapshot.mjs § B2_PROFILES), each with its own snapshot block, its own two-boot stability check and its own ALIGNMENT.md coverage check: probes (the default fleet configuration) and probesTuiPool (CLAUDE_TUI_MODE=true, OCP_TUI_POOL_SIZE=1, CLAUDE_SKIP_PERMISSIONS=true), which is what guards /health's tui.pool counter bag. Neither profile ever runs a real claude pane: OCP_TUI_TMUX_BIN points at a stub tmux that logs its argv and exits 1, and the suite asserts from that log that the only invocation is list-sessions — established by feeding a fake tmux to the real reapStaleTuiSessions: with the real tmux a TUI-mode boot can kill-session a live legacy-named ocp-tui-<8hex> session, and reaches kill-server only when no ordinary session is present (lib/tui/session.mjs's !othersRemain gate). The stub is justified by the reachable kill-session, not by the stronger claim. The snapshot's own notCovered block states what it cannot see; read it before treating a green run as coverage.
  • scripts/lib/install-dir.mjs — the single answer to "where is this OCP installed?", resolved from the code's own location (fileURLToPath(import.meta.url)) rather than $HOME. doctor.mjs and upgrade.mjs each used to answer it with join(homedir(), "ocp"), which is wrong on any install not at $HOME/ocp — and the one host that isn't is the hardened one, relocated to /opt under an unprivileged user to close #328's escalation chain, so hardening moved it off the only path the updater could see. Named resolveInstallDir, not resolveOcpDir: keys.mjs already owns that name for the ~/.ocp data directory. Introduced by #350 (#348).
  • lib/spawn-token.mjs — the spawn-token decision extracted from server.mjs's resolveSpawnToken (#429/#343): makeResolveSpawnToken({ isExpiring, now }) applies the 5-minute expiry gate to CACHED creds on every spawn (the #146 forever-stale-token shape). The gate is mutation-proven by a fixed-now contrast test; production binds no injection, so behaviour is identical to the inline original.
  • scripts/release-notes.mjs + scripts/lib/release-notes.mjs — what goes in a GitHub Release body, decided in a place npm test can reach. .github/workflows/release.yml used to pipe the CHANGELOG section verbatim into gh release create --notes-file; GitHub caps a body at 125 000 characters, v3.29.3's section is 169 670 bytes by wc -c on the awk's output (169 669 through Buffer.byteLength on what the extractor now returns — the awk appended one trailing newline, and knowing which instrument produced a figure is the difference between the two numbers you will see in this commit), so the API answered 422 … body is too long and v3.29.3 shipped with no Release (#441). The body is now the section when it fits — byte-identical, so nothing changes for a normal release — and otherwise a prefix cut at a block boundary (^#{1,6} or a top-level list marker) plus a pointer to CHANGELOG.md at the tag; assertWithinLimit then refuses loudly rather than letting the API be the only size check. The budget is counted in UTF-8 bytes although the cap is in characters, because a byte count is never smaller than a character count, so passing in bytes passes under either meaning — it costs headroom, never correctness. The workflow's run: body deliberately contains no ${{ }}: test-features.mjs slices it out between the #441 anchors and executes it verbatim, which is the only thing that can catch release.yml no longer calling any of this — a release step is otherwise untestable except by cutting a tag, and a tag is not re-cuttable.
  • ALIGNMENT.md — the constitution. Binding for any server.mjs change. See ADR 0002.
  • .github/workflows/alignment.yml — CI blacklist grep; fails the build on known-hallucinated tokens.
  • CLAUDE.md — Claude-Code-specific session instructions + release_kit overlay (Iron Rule 5.5).
  • docs/adr/ — Architecture Decision Records. Read these before proposing governance or SPOT changes. See docs/adr/README.md for the index.
  • docs/superpowers/plans/ — active spec-kit plans. docs/superpowers/plans/shipped/ archives plans that have been delivered (don't propose changes against shipped plans — they're history). docs/superpowers/specs/ holds long-lived design documents that other code references (e.g., the SSE heartbeat design referenced from server.mjs).
  • memory/constitution.md — spec-kit's project constitution (its standard memory/ location). Distinct from ~/.cc-rules/memory/ (cross-machine personal memory) and from this repo's ALIGNMENT.md (the OCP code-level constitution).

Project-specific constraints

  • ALIGNMENT.md is binding — and its five Rules are scoped to Class A. ALIGNMENT.md:17 limits Rules 1–5 to the cli.js-mirror surface. Every PR touching server.mjs declares its endpoint class and cites that class's authority in the commit body and PR description: cli.js:NNNN (or cli.js vE4 <functionName>) for Class A, the OpenAI spec section + ADR 0006 for B.1, the authorizing ADR for B.2. Rule 2 is a prohibition, not an authorization — it is the wrong authority for a Class B endpoint, and citing it as justification is a category error — recorded in the #193 thread as an independent-review finding the author accepted, which held that PR until the citation was corrected. Classification is a lookup in ALIGNMENT.md § "Current Class B inventory" plus the Hybrid note after it, not an argument. See CLAUDE.md § "Classify the change first" and ADR 0006. ADR 0002 records the constitution's provenance but predates the Class A/B split, so its universal framing of the citation rule is narrowed by ADR 0006 and by ALIGNMENT.md:17.
  • Alignment CI is not suppressible. The alignment.yml workflow greps server.mjs for known-hallucinated tokens (currently blocking api.anthropic.com/api/oauth/usage). Adding new tokens is done via PR amendment to alignment.yml; removing entries requires an ALIGNMENT.md amendment PR.
  • No self-approval. Implementation author cannot merge their own PR (Iron Rule 10). A fresh-context reviewer must independently confirm the declared class against the ALIGNMENT.md inventory, then open the reference that class demands — cli.js at the cited lines (A), the cited OpenAI spec section (B.1), or the authorizing ADR (B.2) — and name it in the review comment.
  • models.json is the only place to add/edit models. Do not touch MODEL_MAP or MODELS arrays directly in server.mjs or setup.mjs. See ADR 0003.
  • OpenClaw boundary. scripts/sync-openclaw.mjs only writes models.providers["claude-local"].models and agents.defaults.models["claude-local/*"] in ~/.openclaw/openclaw.json. Do not expand scope. See ADR 0004.

Testing: reaching faults inside server.mjs

test-features.mjs cannot import server.mjs (it calls server.listen() at top level), and that has twice led to the wrong conclusion that a class of bug is untestable. It isn't. Read this before writing "no regression test is possible here".

There is a real live-server fixture. ltBoot(env, dir, nodeArgs) (around test-features.mjs:990) spawns the actual server.mjs as a child with a fake claude binary, so integration tests cost no quota. ltPost / ltPostStatus / ltWait / ltFreePort round it out. It already covers boot gates, cache-epoch invalidation across two boots sharing one SQLite store, and system-prompt capture.

ltBoot pins tmux, and you do not get to opt out (#384). lib/tui/session.mjs:54 resolves process.env.OCP_TUI_TMUX_BIN || "tmux" at module load, so a CLAUDE_TUI_MODE=true boot with that variable unset runs whatever tmux PATH provides — on a workstation, the operator's real one. Its boot reap issues kill-session for every session matching this port's prefix or the legacy ocp-tui-<8hex> shape, which is reached even with ordinary foreign sessions present — that is the reachable harm and the one that justifies the pin. kill-server is gated on !othersRemain && sparedLive === 0 (lib/tui/session.mjs:154), so it fires only when the operator has only legacy-shaped sessions, none at all, or just this instance's own — never alongside an ordinary foreign session. Two tests did exactly that until #384, invisibly, because CI has no tmux. ltBoot now writes a refusing stub into the test's own scratch dir and pins OCP_TUI_TMUX_BIN at it after the caller's ...env spread, unconditionally — a per-site pin is a rule the next TUI test has to remember, and a pin gated on CLAUDE_TUI_MODE is the same hole one level down. A test may supply its own stub, but only a file inside its own scratch dir (ltTmuxStub(dir, name)); anything else throws rather than being silently ignored. Read what the harness invoked with ltTmuxCalls(dir) — that log is how the guard is asserted behaviourally rather than by grepping ltBoot's source. The override lane has a live consumer: the #346 B.2 key-set test calls ltBootFresh(fx.env, fx.dir) with a fixture that already pins OCP_TUI_TMUX_BIN to <fx.dir>/bin/tmux (#382), so the containment check runs once per profile on every suite run — move that bin/ outside fx.dir and the snapshot test starts throwing from ltBoot. Do not make the stub succeed: a list-sessions that exits 0 with no output reads as "server up, zero sessions" and fires kill-server (lib/tui/session.mjs:154), i.e. the permissive shape reproduces the hazard instead of removing it.

--stack-size is a fault lever. ltBoot's third argument passes V8 flags to the child, which puts recursion- and argument-count-limited failures in reach at a much smaller input. #193 needed a synchronous throw deep inside spawnClaudeProcess; buildCliArgs does args.push("--allowedTools", ...ALLOWED_TOOLS), and under --stack-size=200 that spread throws at ~24k elements instead of ~124k — which is what brings the trigger under Linux's MAX_ARG_STRLEN (131072 bytes for a single env string) so the test runs in CI rather than skipping. No production fault hook was needed.

Three rules that made it hold up, all learned the hard way:

  • Discover the threshold in a child under the same flags, never in the test process — the parent's stack is not the one that matters.
  • Assert that the fault actually fired, not just that the outcome looks right. #193 asserts HTTP 500 and that the body carries call stack size exceeded; a control mutation (trigger neutered, bug still present) proves the test fails rather than passing vacuously.
  • Wait for the thing you are about to assert, not a proxy for it. Waiting on listening on and then asserting a different line is a race (#199); waiting for the process to exit and then reading its stderr is another, because a terminated child's pipes may still hold unread data (#203 — wait for the stdio to close, not for the exit).

Allocate ports with ltFreePort(). Fixed ports have caused at least one flake here.

Wiring-pin ledger (issue #343)

The #343 sweep looked for the #339 shape — a correct, well-tested helper whose CALL SITE silently drops or bypasses it. This is the first-pass inventory of every lib/ boundary transform and whether its wiring is pinned by a test that reddens when the call site stops consulting it. "Performance-only" rows are recorded, not pinned: a silent drop costs latency, never a wrong answer.

helper call site(s) wiring pinned evidence / why-not
applyRequestVerdictTtl server.mjs:1233 (/health via effectiveAuthStatus) YES #341
scrubInboundAuthEnv server.mjs:167/174/1259/1614/2890, lib/tui/session.mjs:442/677, setup.mjs:71/153/177 PARTLY #328 pins the child env at the server/setup sites; the TUI pane alias site (lib/tui/session.mjs:442) is unpinned — its own consequence
orderLabelsLastGoodFirst server.mjs:2886 (keychain label order) NO performance-only — a drop costs one extra security exec, never a wrong answer
createTtlCache server.mjs:2879 (keychain read cache) NO performance-only — re-reading the keychain per spawn is slower, never wrong
createSerialMutex server.mjs:673 (real-HOME fallback serialization) NO a drop would race two fallbacks — low blast radius, recorded not pinned
isTokenExpiring server.mjs:664 delegation → lib/spawn-token.mjs:24 YES via this PR — makeResolveSpawnToken + fixed-now contrast test, mutation-proven
image helpers (hasImageContent / buildImageBlocks / buildStreamJsonInput) handleChatCompletions image path YES #110 / #154
classifyToolRequest handleChatCompletions YES mutation-proven (#311 / #370-style)
structured-output helpers (detectStructuredOutput / extractJsonPayload / structuredSystemInstruction) structured-output path YES #153 / #181
isLoopbackBind boot gates YES #370
parsePositiveInt server.mjs:1406 (parseIntEnv, 6 numeric caps); lib/prompt.mjs:89 (resolveGlobalPromptCharOverride) PARTLY lib/prompt.mjs:89 pinned behaviorally (test-features.mjs:2491-2528); server.mjs:1406 parseIntEnv site unpinned — not importable and no live-server test feeds a garbage cap value to assert the fail-closed default

Testing discipline: what counts as a test

Enforced as a review condition on recent test PRs (#204, #205, #208, #216, #218, #221), never written down. Its only written form was inside #210: "This is behavioral, so a mutation to the table or the fallback fails it. A source-grep test would not — and per this repo's standing rule, a test that greps source is not a test." Written down now (#223). Line numbers below are test-features.mjs unless noted.

  • --only <substring> is for mutation campaigns and reviewer spot-checks; the FULL suite remains the final gate (implementer final run + CI).

  • Behavioral, not textual. A test asserting on the source text of the thing it tests is not a test — it passes when the code is deleted and re-added wrong, and breaks on reformatting. Assert on behavior: call the function, run the process, read the output. The ocp-connect section (#210, #218, ~5690) exec()s the real model_meta/get_model_meta() sliced from ocp-connect's source instead of grepping for a number. Exception this suite relies on: a textual assertion is fine for a premise of the harness or a slice boundary, never the behavior under test — see the heredoc-quoting check (:5992) and the kept os.makedirs/open(config_path anchor-drift guards (:5792-5793).

  • A mutation must have LANDED and must MEAN something — those are two claims, and the second has no cheap check. (#440)

    String.replace with a pattern that does not occur returns the string unchanged and does not throw, so asserting the anchor matched is not enough: #439's driver reported six greens from six no-ops. The driver must assert mutated !== original.

    That is necessary and not sufficient, measured on 2026-08-24. A row meant to break a YAML anchor replaced id: notes with id: notesX. The file changed, so mutated !== original held — and the assertion under test does indexOf("id: notes"), which still matches the longer string. The row reported zero red, and a row with no red reads exactly like "this assertion is redundant". It was caught only because the red was expected and did not arrive.

    So ask a third question, on the CONSUMER side: how does the code under test read this string? Any includes / indexOf / startsWith / substring-regex consumer is unaffected whenever the replacement is a superstring of the original. One rule: the replacement must not contain the original — otherwise delete or replace the whole line.

    And the sharpest version of the same point: ask whether RUNNING the row performs the act the guard prevents. On 2026-08-24 a row that removed a test's containment — in order to prove the containment was load-bearing — let the child run a real installer through to launchctl bootout/bootstrap and redefined the operator's live service, for the second time in one change. A mutation that disables a safety guard to demonstrate the guard matters performs the very act the guard prevents. Replace it with assertions that are each other's control (two calls that must stop at different points, so either going vacuous fails the other) — which re-proves the property on every run instead of once per campaign.

  • Mutation-prove every test you add or change in a PR. Break the code it guards, confirm an actionable failure, restore, confirm green — but restore from a file backup, diffed byte-identical against the pre-mutation copy, never git checkout -- <file>. Checkout restores the file's last committed state, not its pre-mutation state: if the file holds the uncommitted test you just wrote, checkout silently discards it along with the mutation; a file backup can't. #218's mutation table (mutation / file / result before / result after) is the format to copy; #221 uses a different shape for the same protocol.

  • A measurement requires exclusivity, and a red you cannot attribute is not a result. New prescription, not existing practice. Every rule above says how to write a test; none said the number you read back has to have been produced alone, and this repo now has two runs where it wasn't. The rule deliberately rests on the observation and not on its cause. #374 recorded, on a host running two concurrent full suites, teardown waits of 75 514 ms and 111 203 ms against a 5 000 ms budget; a clean run on the same host is green, measured as a control, so contention is necessary — whether it is sufficient is exactly what #374 leaves open, and that issue carries a framing correction saying so in as many words. The rule does not need it. Whatever produced the stall, the run was unusable as a measurement: the harness reported the serialization claim UNPROVEN and named the offender, and the agent that hit it correctly discarded the row rather than recording it as a mutation effect. Separately, a mutation campaign's first baseline came back exit 143 with no === Results: line because another agent killed it (see the next bullet); it was voided, re-run, and the baseline then confirmed twice. Do not restate #374's open question as a settled cause — the temptation to write "concurrency causes this" into a rules document is itself an instance of the discipline this section exists to enforce.

    So: take a mutex before every suite run, and do not hand-roll it. mkdir is atomic, so until mkdir "$LOCK" 2>/dev/null; do sleep <short>; done is a real lock rather than a check-then-act, and releasing it in a trap whose last statement is exit is mandatory because bash otherwise carries on through the rest of the loop. Those two facts are the whole of what belongs in prose. The interval is deliberately not specified here. A mkdir lock has no queue, so it offers no ordering guarantee and starvation is reachable. This entry originally said sleep 10, and that literal was measured to produce zero acquisitions in 22 minutes for an agent that was following it correctly, while a sibling re-acquired within its own poll window on essentially every release. Both agents were polling at the same interval — so what the run demonstrates is the missing queue, not that a faster poller beats a slower one, which was asserted in an earlier revision of this entry and is not what was measured. The loser was unblocked by shortening its own interval, which is a workaround for the absent queue rather than evidence about frequency. Poll frequently in the meantime; the interval and the fairness discipline are properties of the implementation, and #416 owns both. The rest of the protocol does not live here — it lives in code with tests: the suite takes its own lock now (suiteLockAcquire in test-features.mjs, #416), and the five failed prescriptions above are its mutation rows.

    That is not tidiness. This bullet tried to specify the protocol and five successive prescriptions in it failed measurement (the fifth being the poll interval this bullet used to name): kill -0 (succeeds on a zombie), [ "$st" = "Z" ] (macOS pads the column, so it is false on a real zombie), the cwd comparison (measured to reject a valid owner, because a driver that cds in a subshell keeps the cwd it was launched with), and ps -o lstart= (a local-time rendering, so an owner and a claimant in different TZs false-mismatch and break a live lock). A further gap has no prescription at all: mkdir succeeds a moment before the owner file is written, and a claimant polling in that window reads nothing — break races a lock taken microseconds ago, wait makes an owner that died in the window unbreakable.

    Every one of those was found by executing the rule, and none by reading it — including across four rounds of review by someone hunting for exactly this. That is a property of the medium, not of the reviewers. A line of prose is an adjective a reader can believe; a line of code is something a mutation can redden. Where a rule needs more than two facts to follow, the extra facts are a program.

    The transferable knowledge stays, because it is what a reader needs even without the protocol:

    • kill -0 / process.kill(pid, 0) succeeds on a zombie (#374), measured — so it cannot answer "is the owner gone".
    • ps output is column-padded on macOS. Measured three times in one session, on Z , SN , and an lstart with embedded spaces. Unquoted or equality comparisons against it fail on the very cases they name, and they would pass on Linux — a platform-dependent guard, right on CI and wrong on the workstation.
    • A process's cwd is where it is, not which tree it serves. It can veto a signal; it can never authorise one.
    • Ask of any clearing condition not only what it is, but whether it is reachable. This one is the third: #324 composed two individually-correct rules into a latch only a conclusive success could clear, while a reliably-timing-out probe never produced one — it ran in production for hours, and its only symptom was ocp update refusing while /health reported ok. ADR 0014 records the shape twice in one design; search it for "It was unreachable under the default configuration" and "One inconclusive probe disarmed it permanently". #416 records all three alongside that further gap, which is what produced this entry.

    And there are three verdicts, not two. State the base rule here, because until this entry it existed only in an issue thread and a rule that lives only in a comment cannot be followed by someone reading the repo: a verdict comes from the suite's own === Results: line, never from an exit code, and a run with no results line is VOID rather than red. Satisfying it is necessary and not sufficient — a run can complete, print a results line, and still carry an environmentally-caused red, which a driver that accepts any run with a results line will record as a mutation effect. The discriminator is attribution: when a mutation's reach is known, any red outside the expected set is environmental or unexpected reach, and both need a human look rather than silent recording. Generalised: a mutation's red set must be attributable; an unattributable red is not a result.

    Two independent measurements that agree are the cheapest validity check available, and the cheapest form needs no second run at all. The day this was written, one agent read 1205 / 0 / 2 on origin/main and another, in a different worktree, read 1207 / 0 / 2 on PR #413's head. The reconciliation was then checked from the diff rather than by re-running anything — git diff origin/main...<head> -- test-features.mjs | grep -cE '^\+\s*(test|ltTest|testAsync)\(' returns 2, with none removed, and ltTest delegates to test so it is not a second bucket. THE PATTERN IS A CROSS-CHECK, NOT THE AUTHORITY, and widening it keeps failing (#440). Measured on 2026-08-27 against four registrations — one flat test, one indented test, one indented ltTest, one flat testAsync: the form this file shipped for months, ^\+(test|ltTest)\(, counts 1; adding testAsync alone counts 2; #440's proposed ^\+\s*(test|ltTest)\( counts 3 — it fixes indentation and still misses testAsync; only ^\+\s*(test|ltTest|testAsync)\( counts 4. That is three successive widenings, each short. A grep over a diff cannot see a registration a conditional adds at all, so no pattern closes this. The authoritative derivation is behavioural: run --only "<filter>" against base and head and compare what actually registers. Use the count to notice disagreement, never to settle it — the same ceiling the ADR 0012 marker grep hit before #346 replaced the mechanism rather than the pattern. Prefer that form: a count derived from the diff is publicly re-checkable, whereas a total is only as good as the tree and the host it was taken on.

  • Attribute a PID by its cwd before killing it — killing by PID is necessary and not sufficient. New prescription, not existing practice. The existing prohibition is on pattern kills (pkill/killall), and it was fully satisfied by the incident that produced this rule: PR #413's independent reviewer, diagnosing two concurrent suite runs, sent SIGTERM to four PIDs chosen individually and by explicit PID — and one of them belonged to another agent's worktree, invalidating that agent's baseline run. Compliance with the old rule is what makes this worth writing down; it is not the case the old rule was aimed at, and no amount of care about how you select a PID tells you whose it is. Note that the bullet above is the structural form of this one: the incident happened while diagnosing two concurrent runs, so a mutex removes the situation in which anyone reaches for a kill at all. This bullet is what remains once that is granted, and it is advice rather than construction — stated plainly because the section's own standing rule prefers construction and a reader should be able to see which this is.

    Before signalling anything, resolve its working directory — /usr/sbin/lsof -a -p <pid> -d cwd -Fn — and act only on what sits under your own worktree. Use the absolute path and do not swallow stderr. Measured here: /usr/sbin is not on the agent shell's PATH, so a bare lsof fails with command not found, and lsof … 2>/dev/null turns that failure into an empty result that reads exactly like "no such process" — a command that never ran, indistinguishable from one that found nothing. That mistake was made three times in one session before it was caught, each time producing a confident negative conclusion. Note also what this does and does not tell you: cwd locates the process, not the tree it operates on (a driver that cds in a subshell keeps the cwd it was launched with), so a cwd outside your worktree is a reason not to signal, never a proof that the process is unrelated. This matters here specifically because the normal working shape of this repo is several agents in several worktrees running the same suite, and because a real OCP instance may be serving on the documented default port on the very host you are testing on — the suite's own children and somebody's live proxy are both node processes owned by you. The reviewer disclosed its own error unprompted, which is the behaviour to copy: a review that hides its own mistake is not worth much.

  • A control mutation must prove the test CAN fail — already stated above (#193); here's the specific way it's been missed. #218's py_compile/exec harnesses slice source between two anchors; found out of order, the slice is silently '', and checking an empty string trivially "succeeds" (:5790, "anchor drift"). Assert the slice is non-empty before trusting anything downstream of it.

  • Anchor drift has a second form that a length floor makes WORSE, not better (#347). The note above prescribes "assert the slice is non-empty", which defends the '' form correctly. It does not defend the inverted form, and the natural guard against one is actively wrong against the other. String.indexOf returns -1 when a marker is absent, and slice(start, -1) is not an error and not empty — it runs to one before the end of the string. Measured, in this repo, on the #347 G2 test: with the end anchor intact the slice was 109 chars; with the end anchor broken it was 185. The guard in place was chain.length > 40 && chain.includes("restart") — written specifically to catch anchor drift — and it passed, so the control mutation came back green and the test it was meant to prove was never proven. An empty slice is conspicuous; a longer, richer-looking slice reads as healthier than the correct one, which is why this form survives the very check added to catch its sibling. Prescription: assert both anchors by INDEX (start > -1 && end > start) before slicing at all. A length floor, a substring check, or any assertion on the resulting slice is not a substitute — those are assertions about the output of a computation whose inputs you have not established. Found by a control mutation doing its job; recorded because the guard that failed was itself the anti-drift guard.

  • An assertion that never EXECUTED is indistinguishable from one that passed (#405). Both are silence. The entry above and :72's "wait for the thing you are about to assert" are instances; this is the general rule, and it applies outside the suite too — it showed up three times in one day in three different systems, only one of which was the test suite. Throughout, the observation is the lookup or execution the assertion depends on, never the assertion's output: #347's slice did happen, what was absent was the anchor find.

    Three ways a claim goes unproven inside a run that looks green, with the remedy each needs. They were being treated as one thing and the remedies are not interchangeable:

    • Asymmetric — an earlier assertion in the same body catches the mutation aimed at a later one, which then never runs. Fixed by ORDERING: put the narrow assertion first, or delete it when a broader one implies it. #405's T1 is the worked example — a whole-argv deepEqual replaced !argv.includes("--tools") rather than joining it, because deepEqual strictly implies the narrower check and keeping both would have left the broader one permanently unprovable.
    • Mutual — one mutation breaks two claims in the same body. The first assert to throw ends that body, the second claim never executes, and it ships unproven. Ordering cannot help: both are broken, so no order makes both report — only separate test() bodies do. Measured on #405's M3 (drop shq() from the --tools value): with the two claims in separate tests, 1193 passed / 2 failed against that branch's 1195/0 baseline, reddening exactly the two tests whose names carry "stays ONE argv element, not word-split" and "cannot break the shell string" — quoted as the fragments that survive the cosmetic rename #405 applied after that run, so they match both the log and the tree. Co-located, one row is the most that could ever have appeared.
    • Process death — the body does not throw; the process goes away, and nothing reports at all. Separation into test() calls is NOT a remedy here. test(name, fn) (:67) invokes fn() immediately inside its own try/catch, so bodies run at call time in one process: a genuine kill takes every later test() call with it, and a throw is always reported rather than silent. Only a separate child process isolates this.

    Do not diagnose from the totals — establish execution. For each claim, name the mutation row that reddens that named test; a claim with no row of its own is unproven however green the suite is. If two claims share a body and one mutation kills both, only one row can ever exist, and that missing row is the signal.

    Two cautions from getting this wrong while writing it down. A loud failure is not evidence of the category — M3's `(' shell error is emitted by sh in a child (spawnSync) and reaches the suite as an ordinary AssertionError in the parent, which is why the run completed at all; had the second claim failed with a plain diff, co-location would have had the identical consequence. And what forces separation is that one mutation breaks both claims, not what the failure looked like.

    The same shape one level up, in the toolchain rather than the suite, which is why this is a general rule and not a testing quirk:

    • A required check that never RAN reads exactly like one that passed. #405's merge was refused with Required status check "gitleaks scan (hard fail)" is expected while all five checks were green on the head SHA. Observed: ruleset 20181348 carries exactly that one required context and strict_required_status_checks_policy: true, and a PR had landed on main in the interval — which alone accounts for the refusal. (That protection evaluates a recomputed refs/pull/N/merge ref is the inference that fits; the check-runs in this repo attach to head SHAs.) gh pr checks showing five passes was true and irrelevant.
    • A wait loop can exit before the thing it waits for exists. #404's until ! gh pr checks | grep -q pending returned immediately — not because the checks had finished but because they had not been created yet, so its negative predicate was satisfied by an empty world.

    Prescription: require a POSITIVE count before trusting a negative predicate. Wait for ≥N checks AND 0 pending, never for no pending. Assert argv.length > 0 and that a known-present element is there before asserting any element is absent. Derive N from what applies to THIS change, not from a previous PR — alignment.yml is path-filtered, so a docs-only PR gets 2 checks where a lib/** PR gets 5; the PR that added this entry hung its own wait loop on ≥5 for exactly that reason. Err high: too high hangs visibly, too low exits silently. An empty slice, an empty check list and an unexecuted assertion are the same bug, and each satisfies the guard written to catch it.

  • Guards on dynamic execution must bound capability, not scan text. #218 took three rounds, npm test writing a real file while staying green each time: a narrow two-marker denylist was shown insufficient by the author's own mutation-proof of their own fix (open("<path>", "w"), matching neither marker); the blanket denylist that replaced it was bypassed too, by pathlib.Path(...).write_text(...). Claiming "this code cannot do X" while the implementation is "its text doesn't contain Y" is false. Fix: restrict __builtins__ in the exec() namespace to only the names each slice calls — a drift guard, not an adversarial sandbox (deliberate dunder traversal still escapes, by design). Full narrative, and the harness that shipped with no guard at all (harness 3, _OC_PROVIDER_PY): :5725-5782.

  • A claim of guaranteed behaviour must cite the mutation that proves it — and a NAME is a claim. Any sentence of the form "X is pinned by test", "X holds by construction", "X cannot happen", or "this is a faithful port of Y" is an assertion about the code, and assertions about the code are the thing this repo keeps shipping wrong. Cite the mutation row that kills the test, in the comment, the PR body, or both. Written down after a single day in which nine review findings across two PRs were all of this shape and none was a behavioural bug: the code did what it should; the prose about the code did not. Five of the nine would have been stopped here.

    The failures ran deepest at the two places nobody thinks of as prose:

    • A name is a claim. classifyPostFlightProbeFailure stamps kind: "version-mismatch" on any post-body rejection, including status: "degraded" with the version completely correct. A remediation whose stated method was "re-key every cell on the classification so no sentence asserts more than was measured" applied it correctly to four kinds and wrote the fifth's semantics from its name — in the comment whose subject was that discipline (#371 round 3). Read what assigns the value, never what it is called.
    • A predicate is a claim. The recurring root cause across three rounds was cells firing on a negation instead of on positive evidence: lastSeen !== target is satisfied by a missing operand, so an unreadable snapshot produced SERVING THE WRONG VERSION (3.10.0, expected ). Ask of every branch: does this fire because something was observed, or because something was absent? Note that splitting the classifier — the obvious structural fix — would not have caught this one, because the defect was in the other operand.

    Two corollaries learned the same day. Numbers are claims too: a comment carried "a quiet loop finishes at 1.02x nominal and a stalled one at 7.53x", figures that appeared nowhere else in the repo and contradicted a comment 2,600 lines earlier in the same commit. And a mutation table measured under a superseded predicate must be re-measured, not carried forward — reusing rows whose predicate changed is this defect one document up.

  • A citation into ANOTHER file goes stale exactly when your own file does not change (#393). A merge-forward that leaves your region byte-identical is the case with no prompt to re-check, and it is the case where server.mjs:NNNN in your comment silently starts pointing at the wrong line — #393's review found four such citations off by ~139 lines, with the reasoning still correct. Byte-identity of the region was the argument offered for the merge being safe; it is the condition under which this fails. Lead with the greppable anchor and treat the number as decorative, or pin it to a stated SHA so a reader knows which tree to check it against. The same applies to a mutation table: rows keyed on another file's line numbers cannot be re-derived later.

  • Constraints must be unreachable by construction, not stated as prohibitions. #217's review took production OCP down: a cmd_restart stub defined before source-ing the real ocp script was silently overwritten once sourced, and it ran for real against a live host (bootout ok, bootstrap failed, nohup fallback wrote to a scratch dir — proxy down until noticed). Define stubs after sourcing, never before. New prescription, not existing practice: a bash harness sourcing ocp needs its own scratch $HOME, and any command that can mutate a running service (launchctl/systemctl/pkill/nohup) should be a stub that fails loudly by default.

  • Testing the RUNNER itself: use a second instance, never a miniature copy (#402). test() / testAsync() / testSkipped() / skipRemainingTest() live inside createHarness({ log }), and the suite runs on one instance created at the top of the file. To test a runner verdict, create another instance: it has its own counters, its own passedNames / failedNames ledgers, its own pendingAsync and its own AsyncLocalStorage, so a fixture that must be reported as a failure does not make npm test exit 1, and its synthetic ✗ lines do not reach stdout (pass a collecting log — .github/workflows/flake-hunt.yml:179 histograms stdout ✗ lines and would read a deliberate fixture as a flake). Re-implementing test() in miniature inside the test is the shape the "behavioral, not textual" rule forbids for the same reason a source grep is: it passes while the real runner is broken. Two invariants keep the ledgers usable as evidence: every passed++ goes through recordPass() and every failed++ through recordFailure(). What makes them hold is structural, not vigilance — passed and failed are closure-private lets inside createHarness, so no code outside the factory can reach them and there is exactly one increment site for each. (Two tests also assert passedNames.length === passed and failedNames.length === failed, but those are positional reads and do not cover every arm; the closure is what makes the invariant total.) The same discipline applies to _recordSkip — it is the single funnel every skip helper reaches, which is why the swallowed-abort counter lives there rather than in either public helper.

Release protocol

OCP follows the machine-readable release_kit: overlay in CLAUDE.md (Iron Rule 5.5). Before any version bump or tag push, re-read that YAML block and walk every item in new_feature_doc_expectations and bootstrap_quirk_policy.

And AFTER the tag push, walk release_channel.post_release_checks — the walk does not end at the tag. Tag push triggers .github/workflows/release.yml, which creates the GitHub Release automatically. That is what it is supposed to do, not a fact you may assume it did: for v3.29.3 the job failed on the release-body length, no release was created, and nothing in this repo noticed for a day because every other mechanism reads package.json rather than Releases (#441). So confirm the Release exists for the tag, and confirm the job's conclusion — gh run list prints failure while exiting 0, so read its output, not its exit code.

Do not create the release manually as the normal path — the automation owns it. The one exception is a recovery when that automation has already failed, and #441 is the worked example: say on the release itself that it was a recovery, and note that re-running the failed job does not help, because the workflow file is read from the tag ref and a fix on main only reaches the next tag.

Version is sourced from package.json; changelog from CHANGELOG.md; user-facing docs from README.md.

Handoff expectations

A fresh session picking up OCP work should read, in order:

  1. This file (AGENTS.md).
  2. ALIGNMENT.md — constitution; non-optional.
  3. CLAUDE.md — tool-specific instructions and release_kit overlay.
  4. docs/adr/ — most recent ADRs first; they explain why the current structure exists.
  5. Any active plan under docs/superpowers/plans/ (excluding shipped/ which is the archive).
  6. ~/.cc-rules/memory/auto/MEMORY.md — cross-machine memory index.

Only after these should the session touch code.