From 0c84769231bf025cf5325eec08f08243d93c0089 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 08:22:37 +0000 Subject: [PATCH] =?UTF-8?q?docs(agents):=20principles-only=20AGENTS.md=20?= =?UTF-8?q?=E2=80=94=20lessons=20distilled=20in=20place,=20no=20issue-ID?= =?UTF-8?q?=20citations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite per the same three-way sorting rule as the SKILL.md / os-dev.md rewrites: mechanically-enforced rules shrink to principle + script pointer (the script header is the authority on detail); incident narratives and procedural walkthroughs are distilled in place as self-contained lessons (failure mode + discipline + boundary) with zero issue-ID citations; operational lookups (commands, tables, markers) stay as data. All 15 Prime Directives survive whole; maintainer rulings keep date + verbatim quote; section names, the numbered discipline list and its section anchors, and the RELEASE-OWNED guardrail row that check:docs-audit-scope anchors on are preserved. 1050 -> 818 lines, 84.7KB -> 66.9KB, 123 -> 0 issue-ID citations. CLAUDE.md's mirrored rules updated in the same PR (two ID-carrying passages distilled to match). check:pm-skill-id-lint's scan set extended to AGENTS.md — no waiver needed since the rewrite and the scan-set extension travel together; the dead os-dev.md legacy waiver entry (which would have re-admitted exactly 81 IDs) is deleted and the self-test now exercises the waiver mechanism with a synthetic entry. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012Gg6rMAti8ZaueWb6BRsDn --- AGENTS.md | 1144 +++++++++++----------------- CLAUDE.md | 6 +- scripts/pm/check-skill-id-lint.mjs | 49 +- 3 files changed, 486 insertions(+), 713 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d2ce70e1b1..7903aa3be9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,9 +1,19 @@ # ObjectStack — AGENTS.md -Primary AI instruction file for this repo. Read natively by Claude Code, GitHub Copilot (coding agent + CLI, since Aug 2025), and other agents — no separate `.github/copilot-instructions.md` mirror needed. +Primary AI instruction file for this repo — and the human contributors' source of truth. +Read natively by Claude Code, GitHub Copilot (coding agent + CLI), and other agents — no +separate `.github/copilot-instructions.md` mirror needed. When any other instruction file +in this repo (including `.claude/skills/**`) conflicts with this one, **AGENTS.md wins**. > **v5.0 breaking rename: `project` → `environment`** everywhere (CLI `-e`, `/api/v1/environments/:id`, header `X-Environment-Id`, `OS_ENVIRONMENT_ID`, DB column `environment_id`). No aliases. See ADR-0006. "Project" now only means the npm/monorepo sense. +This file carries principles, binding rules and lookup tables. Lessons from past +incidents are distilled in place — failure mode, discipline, boundary — without +issue-number citations (maintainer ruling, 2026-08-12: 「处理 issue 时犯的错应该总结成 +经验,保留 issue id没有意义」); maintainer rulings keep their date and verbatim quote. +Where a hook or CI gate enforces a rule mechanically, the rule is stated once here and +the script's own header is the authority on detail. + --- ## Communication @@ -13,17 +23,19 @@ Primary AI instruction file for this repo. Read natively by Claude Code, GitHub - **在 Claude Code 中与维护者对话一律使用中文**(对话回复、轮次报告等聊天通道里的内容)。 - **GitHub 产物一律使用英文**:issue 与 PR 的标题、正文、评论。维护者裁决 - (2026-08-08,#6692),原文引用、未翻译: + (2026-08-08),原文引用、未翻译: > issue 和 PR 必须用英文，在 claude code 中和我讨论可以用中文。 - 这条裁决推翻的是本节此前的写法——它把上面两件事并成一句,并把「PR/issue 讨论中的 - 解释性文字」一并划给中文。代价实测过:同一天、同一身份下的 PR 正文一半中文一半英文 - (#6556 中文 / #6691 英文),因为 agent 手里同时握着两条相反的指令(#6692)。 - **引用中文裁决时保持原文、不翻译**,即使承载它的 issue/PR 正文通篇是英文——改写引文 就是改写裁决。上面那段引用即是一例:它是维护者的原话,一个字未动。 - 代码、标识符、提交信息(commit messages)、ADR/文档正文等仓库产物保持现有语言惯例(以英文为主),不要因本节而改写。 +The rules are split per channel because a merged rule was measured to fail: an agent +holding one instruction that claimed explanatory PR text for Chinese and another +demanding English produced half-Chinese, half-English PR bodies on the same day. One +rule per channel, no overlap. + --- ## Build & Test @@ -33,56 +45,38 @@ pnpm install # deps pnpm setup # first-time: install + build spec pnpm build # turbo build (excludes docs) pnpm test # turbo test -pnpm typecheck # turbo typecheck — per-package `tsc --noEmit`; tsup/vitest never type-check (#4311) +pnpm typecheck # turbo typecheck — per-package `tsc --noEmit`; tsup/vitest never type-check pnpm docs:dev # docs site ``` -Type-check coverage is ratcheted (`pnpm check:type-check-coverage`, CI-gated): every -workspace package declares a `typecheck` script or carries a measured DEBT/EXEMPT entry -in `scripts/check-type-check-coverage.mjs`. New packages must arrive covered; a package -that graduates deletes its ledger entry in the same PR. - -The ledger numbers are ratcheted too (`pnpm check:type-check-debt`, run in the same CI -job after its build step): every DEBT/TEST_DEBT count is re-run through `tsc --noEmit`, -and a count ABOVE its recorded number fails. Below is only an informational -"can be lowered" line — improvements never owe CI a bookkeeping edit. Before #5278 the -gate asserted only that *some* positive number was written down, so the real counts had -drifted up to 2.25x while it reported success. When a re-measure makes you raise an -entry, rewrite its `note` as well: the composition drifts too, and a note that still -names only the old errors reads as "nearly graduated" to the next author. - -**Do not `exclude` `*.test.ts` / `*.spec.ts` from a package's `tsconfig.json`.** `tsc ---noEmit` reads that config, so an exclusion there hides the tests from the check the -`typecheck` script advertises — a green gate over source nothing read, which is the -#4311 defect itself. The ratchet's `TESTS_COVERED` invariant fails on any new exclusion; -the packages that already had one carry a measured `TEST_DEBT` entry and graduate by -dropping the exclusion — or, when the build config must keep the exclusion (ci.yml gates -that no test file reaches the published artifact), by adding a **sibling -`tsconfig.test.json` and naming it in the `typecheck` script**, which is what -`packages/spec` does since #5286. The sibling may carry its own *module* semantics to -match how vitest executes the files (`module: esnext`, `moduleResolution: bundler`) — -never its own *strictness*: `strict` and friends are inherited, untouched. - -**A `@ts-expect-error` in a file no tsc program compiles is a phantom check** — the -`PINS_CHECKED` invariant of the same ratchet, repo-wide. `@ts-expect-error` is the -"tsc is the best sweeper" channel the spec-property-retirement playbook leans on: the -directive is meant to go red the day a removed key comes back. Outside a program it -evaluates never, and *deleting the directive leaves every gate just as green* — which is -how spec's 17 retirement pins across 5 files were found (#5286), and the repo-wide sweep -that followed found the eighteenth in `packages/client` (#5449). Before writing one, -check the file is compiled. A package whose test layer still carries residue holds it in -a per-file, exactly-measured, shrink-only ledger next to its `tsconfig.test.json` -(`/test-typecheck-debt.json`, regenerated with -`pnpm --filter gen:test-typecheck-debt`): a file not listed there may have no -type errors at all. The gate behind both is one shared script, -`scripts/check-test-typecheck.mts --package ` — onboard a package by wiring its -`typecheck` script to it, never by copying it. - -One trap worth knowing before you read any of these counts: under `moduleResolution: -NodeNext` a relative import missing its `.js` extension does not resolve, every symbol it -names becomes `any`, and the callbacks over those symbols then report TS7006 "implicitly -any". A pile of TS7006 is usually one broken import upstream, not a package that needs -type annotations — fix the extension first and re-measure. +Type-check coverage and its debt counts are ratcheted in CI +(`pnpm check:type-check-coverage`, `pnpm check:type-check-debt`; the script headers are +the authority on detail): every package declares a `typecheck` script or carries a +measured, shrink-only DEBT/EXEMPT ledger entry; new packages arrive covered; a package +that graduates deletes its entry in the same PR; when a re-measure forces a count up, +rewrite the entry's `note` too — a note naming only the old errors reads as "nearly +graduated" to the next author. + +Three principles the ratchet's invariants encode, worth knowing before you fight them: + +- **Never `exclude` `*.test.ts` / `*.spec.ts` from a package's `tsconfig.json`** — + `tsc --noEmit` reads that config, so the exclusion hides the tests from the very + check the `typecheck` script advertises (a green gate over source nothing read). When + the build config must keep the exclusion, add a sibling `tsconfig.test.json` and name + it in the `typecheck` script (the `packages/spec` pattern); the sibling may carry its + own *module* semantics to match vitest, never its own *strictness*. +- **A `@ts-expect-error` in a file no tsc program compiles is a phantom check** — it + evaluates never, and deleting it leaves every gate just as green (a repo-wide sweep + once found seventeen retirement pins in that state in one package alone). Before + writing one, check the file is compiled. Test-layer residue lives in the per-file, + shrink-only `/test-typecheck-debt.json` (regenerate with + `pnpm --filter gen:test-typecheck-debt`); the shared gate is + `scripts/check-test-typecheck.mts --package ` — onboard by wiring, never by + copying. +- **A pile of TS7006 "implicitly any" is usually one broken import upstream**, not a + package that needs annotations: under `moduleResolution: NodeNext` a relative import + missing its `.js` extension does not resolve and every symbol it names becomes + `any`. Fix the extension first and re-measure. ### Running the dev server @@ -106,7 +100,7 @@ This repo ships **backend only**. All Studio/Console UI work happens in `../obje Other scripts: `objectui:bump` (pull only), `objectui:build`, `objectui:clean`. ⚠️ Never hand-edit `packages/console/dist/` or `.cache/objectui-*/` — regenerated. -**Moving the pin has a second half: `pnpm sdui:manifest`.** ADR-0082 D4's spec↔registry declaration-parity ratchet reads objectui's `sdui.manifest.json`, which changes only when `.objectui-sha` moves — so the pin bump is the ratchet's trigger, and its only one. It is an **on-demand gate by decision** (#5960), never a CI job; `objectui:bump` and `objectui:refresh` both print the reminder. Needs Playwright chromium. Full procedure: `docs/releases-maintenance.md` → "After the pin moves". +**Moving the pin has a second half: `pnpm sdui:manifest`.** ADR-0082 D4's spec↔registry declaration-parity ratchet reads objectui's `sdui.manifest.json`, which changes only when `.objectui-sha` moves — so the pin bump is the ratchet's trigger, and its only one. It is an **on-demand gate by decision**, never a CI job; `objectui:bump` and `objectui:refresh` both print the reminder. Needs Playwright chromium. Full procedure: `docs/releases-maintenance.md` → "After the pin moves". **Fast iteration on `../objectui` src (no commit/refresh loop):** run objectui's own console dev server — `cd ../objectui && pnpm --filter @object-ui/console dev` (Vite on **:5180**, HMR). Its `/api` proxy targets `DEV_PROXY_TARGET || http://localhost:3000`, so **run the backend you're testing on :3000** (`PORT=3000 pnpm dev` for showcase) and browse `:5180`. Note `:3001/_console` (or whatever the backend serves) is the **published** console, not your `../objectui` src — only `:5180` reflects local UI edits. See `../objectui/AGENTS.md` for the app-id / localStorage / auth gotchas. @@ -134,61 +128,51 @@ Other scripts: `objectui:bump` (pull only), `objectui:build`, `objectui:clean`. - **Opt-out** → `OS_SKIP_{X}` / `OS_DISABLE_{X}`. **Test/CI-only** → `OS_TEST_*` / `OS_EXPECT_*`. - Pre-existing vars that don't fit (`OS_METADATA_WRITABLE`, `OS_EAGER_SCHEMAS`, `OS_SERVER_TIMING`) are **debt, not precedent** — new vars follow this rule; rename old ones via the deprecation helper below when touched. - When renaming a legacy var, use `readEnvWithDeprecation('OS_NEW', 'LEGACY')` from `@objectstack/types` (keeps legacy working one release). Third-party exceptions kept as-is: `NODE_ENV`, `HOME`, `OPENAI_API_KEY`, `TURSO_*`, OAuth `*_CLIENT_ID/SECRET`, `RESEND_API_KEY`, `POSTMARK_TOKEN`, `AI_GATEWAY_*`, `SMTP_*`. See #1382. -10. **File issues for out-of-scope findings — don't silently expand scope or leave them buried.** When you hit a bug, gap, or unenforced capability that's unrelated to the current task, or too large to fix in scope, open a GitHub issue (`gh issue create`) with a clear repro/decision and link it from your PR. Corollary: **never advertise or demo a capability the runtime doesn't actually deliver** (declared ≠ enforced) — fix it, trim it, or file an issue, but don't fake coverage. Example: the spec once declared 9 validation-rule types while the write-path validator enforced only 3 (`state_machine`/`script`/`cross_field`); the gap was filed as #1475 rather than demoed in the showcase, then closed by **trimming** what could never be enforced (`unique`/`async`/`custom`) and **implementing** the rest — the spec now declares 6 and `rule-validator.ts` handles all 6. Note how narrow that claim stayed even so: the evaluator was wired into insert and single-id update only, so a bulk `updateMany` silently skipped every rule — a second `declared ≠ enforced` gap one layer down, at the **call site** rather than the `switch`; filed as #3106 and closed by evaluating the bulk match set per row. A `case` label is not enforcement; check the **call site**. -11. **Worktree-first — never edit on the shared `main` checkout.** This repo is edited by **multiple agents at once**; the shared `main` tree has its HEAD switched and reset *under you*, silently clobbering uncommitted work. Before your **first file edit**, you MUST be in a dedicated worktree on a feature branch: `git worktree add ../objectstack- -b main && cd ../objectstack- && pnpm install`. Two PreToolUse hooks **enforce** this — `.claude/hooks/guard-main-checkout.sh` blocks `Edit`/`Write`/`NotebookEdit`, and `.claude/hooks/guard-main-checkout-bash.sh` blocks the identical write arriving through **Bash** (`>`/`>>` redirection, `sed -i`, `perl -i`, `tee`, `cp`, `mv`, `rm`, `touch`) — unless the target is in a dedicated **worktree** — a feature branch on the *shared* checkout is **not** enough (it still gets switched under you) — and both check the **target file's own repo**, so sibling repos (`objectui`/`cloud`) you touch are covered too (override for a deliberate non-task fix with `OS_ALLOW_MAIN_EDITS=1`, one switch for both). The Bash guard is precision-first: it never blocks reads, and any shape it cannot resolve with confidence (`bash -c …`, `xargs`, `node -e`, a `$VAR`/glob target) is allowed through — the rule still outranks the hook. **The one thing a worktree does *not* isolate is the stash**: `refs/stash` lives in the **common** `.git`, so a bare `git stash push`/`pop` operates on a LIFO stack shared with every other worktree — objectui#3430 swapped two agents' in-flight changes through it, silently. A third hook (`guard-shared-stash.sh`, `OS_ALLOW_STASH=1`) blocks the mutating forms; the collision-free replacements are in the discipline section below. Full playbook below. -12. **Contract-first — fix the metadata, not the runtime.** This is a metadata-driven framework: `packages/spec` is the one contract between metadata *producers* and the runtime/renderers that *consume* it. When a piece of metadata "doesn't work," ask **first**: *is it spec-compliant? is this the long-term-correct direction?* If the metadata is wrong, fix it at the **producer** and **reject it at authoring/publish** (validation / lint) so the error surfaces loudly — do **not** add a lenient alias or `??` fallback in the consumer (a node executor, the REST layer, a renderer) to tolerate off-spec input. A tolerant fallback fossilizes the wrong convention into a second de-facto contract, dilutes the spec, and hides the producer's bug — one strict contract beats N dialects. This is an **internal** contract (we own both ends), so "be liberal in what you accept" (Postel) does **not** apply — that's for untrusted boundaries. Change the **spec** only when the spec itself is genuinely wrong, and then deliberately (edit the Zod schema + migrate), never by accreting consumer-side fallbacks. The `cfg.filter ?? cfg.filters` / `cfg.objectName ?? cfg.object` fallbacks the flow executors once carried are **debt to pay down, not a pattern to copy** — and the way they are being paid down is the pattern to copy. `filters` → `filter` has **graduated** into the ADR-0087 D2 conversion layer (`flow-node-crud-filter-alias`): rewritten to the canonical key at load, including the `AutomationEngine.registerFlow` rehydration seam, so the CRUD executors read `cfg.filter` directly and no consumer-side fallback survives. `object` → `objectName` and the six open-coded stragglers #3796 tracked (notify `to`/`subject`/`body`/`url`, script `functionName`/`input`) graduated the same way at protocol 17 (`flow-node-crud-object-alias`, `flow-node-notify-config-aliases`, `flow-node-script-config-aliases`), emptying the `readAliasedConfig` executor shim — deleted with them. When you must tolerate an alias at all, declare it as a conversion-layer entry (never a bare `??`, and no new executor shims) so it is declared, loud, tested, and *removable on a schedule*. Stored `sys_metadata` rows (data at rest) are covered from the other side: every rehydration seam replays the **full** conversion chain — retired entries included — via `applyConversionsToStoredItem` (#3903, ADR-0087 addendum), so a consumer never needs its own accommodation for a legacy stored shape either. *Worked example:* an AI-authored `create_record` used `fieldValues` / `today()` / `{{trigger.record.id}}` while the executor reads `fields` / `{TODAY()}` / `{record.id}` → the fix was correcting the authoring skill + a publish-gate lint that rejects the wrong shape (cloud#688), **not** a `cfg.fields ?? cfg.fieldValues` runtime alias (framework#2419, rejected). Strengthens #5. -13. **An accepted ADR binds until a superseding ADR says otherwise.** Reversing a recorded decision is itself a decision: it needs a **new ADR** (or an amended status line on the old one), not a changeset that quietly does the opposite. Before changing behaviour in `docs/adr/`-governed territory, **grep the ADRs for the surface you are touching** — the decision is often older and broader than the code comment in front of you. *Worked example:* three accepted ADRs said `sys_member.role` must never carry RBAC authority (ADR-0057 D4 "never as the authority for RBAC", ADR-0090 D3's word ban "distribution = `position`", ADR-0095 D3 "no enforcement-time code path may consult the better-auth role"). A patch-level changeset made app-declared names storable there anyway; a follow-up made it automatic in every host; the reversal held for a day and the tracking issue was closed, reopened and rewritten three times while the cause moved (#3723 → ADR-0108). The mechanism was not carelessness — **the file being edited never named the ADRs that governed it**, so the author could not have known. Hence the corollary: when you implement an ADR's decision, **leave its id in the code**, and anchor load-bearing spots in `scripts/adr-anchors/` (`pnpm check:adr-anchors`) — **one new JSON file per anchor, named for the path it anchors; there is no index to register it in** (#6957) — so the next author is told which decision they are standing on. A decision nobody can find is a decision that will be reversed. -14. **⛔ An ADR is confirmed and merged by the maintainer, by hand — no AI seat merges, queues, or arms auto-merge on a `docs/adr/**` PR.** Maintainer ruling, 2026-08-08 (#6741), verbatim and untranslated: + When renaming a legacy var, use `readEnvWithDeprecation('OS_NEW', 'LEGACY')` from `@objectstack/types` (keeps legacy working one release). Third-party exceptions kept as-is: `NODE_ENV`, `HOME`, `OPENAI_API_KEY`, `TURSO_*`, OAuth `*_CLIENT_ID/SECRET`, `RESEND_API_KEY`, `POSTMARK_TOKEN`, `AI_GATEWAY_*`, `SMTP_*`. +10. **File issues for out-of-scope findings — don't silently expand scope or leave them buried.** When you hit a bug, gap, or unenforced capability that's unrelated to the current task, or too large to fix in scope, open a GitHub issue (`gh issue create`) with a clear repro/decision and link it from your PR. Corollary: **never advertise or demo a capability the runtime doesn't actually deliver** (declared ≠ enforced) — fix it, trim it, or file an issue, but don't fake coverage. The recurring shape: a spec declaring more rule types than the write-path validator enforced was closed by **trimming** what could never be enforced and **implementing** the rest — and even then the claim had to stay narrow, because the evaluator was wired into insert and single-id update while bulk update silently skipped every rule: the same declared-≠-enforced gap one layer down, at the **call site** rather than the `switch`. A `case` label is not enforcement; check the **call site**. +11. **Worktree-first — never edit on the shared `main` checkout.** This repo is edited by **multiple agents at once**; the shared tree has its HEAD switched and reset *under you*, silently clobbering uncommitted work — a feature branch on the *shared* checkout is **not** enough (it still gets switched under you). Before your **first file edit**, be in a dedicated worktree on a feature branch: `git worktree add ../objectstack- -b main && cd ../objectstack- && pnpm install`. Two PreToolUse hooks **enforce** this — `.claude/hooks/guard-main-checkout.sh` blocks `Edit`/`Write`/`NotebookEdit`, and `.claude/hooks/guard-main-checkout-bash.sh` blocks the identical write arriving through **Bash** (`>`/`>>` redirection, `sed -i`, `perl -i`, `tee`, `cp`, `mv`, `rm`, `touch`) — and both check the **target file's own repo**, so sibling repos (`objectui`/`cloud`) you touch are covered too (deliberate non-task override: `OS_ALLOW_MAIN_EDITS=1`, one switch for both). The Bash guard is precision-first: it never blocks reads, and any shape it cannot resolve with confidence (`bash -c …`, `xargs`, `node -e`, a `$VAR`/glob target) is allowed through — the rule still outranks the hook. **The one thing a worktree does *not* isolate is the stash** — `refs/stash` lives in the **common** `.git`, shared by every worktree; a third hook (`guard-shared-stash.sh`, `OS_ALLOW_STASH=1`) blocks the mutating forms, and the collision-free replacements are in Multi-agent discipline below. +12. **Contract-first — fix the metadata, not the runtime.** This is a metadata-driven framework: `packages/spec` is the one contract between metadata *producers* and the runtime/renderers that *consume* it. When a piece of metadata "doesn't work," ask **first**: *is it spec-compliant? is this the long-term-correct direction?* If the metadata is wrong, fix it at the **producer** and **reject it at authoring/publish** (validation / lint) so the error surfaces loudly — do **not** add a lenient alias or `??` fallback in a consumer (a node executor, the REST layer, a renderer) to tolerate off-spec input. A tolerant fallback fossilizes the wrong convention into a second de-facto contract, dilutes the spec, and hides the producer's bug — one strict contract beats N dialects. This is an **internal** contract (we own both ends), so "be liberal in what you accept" (Postel) does **not** apply — that's for untrusted boundaries. Change the **spec** only when the spec itself is genuinely wrong, and then deliberately (edit the Zod schema + migrate), never by accreting consumer-side fallbacks. When an alias must be tolerated at all, declare it as an **ADR-0087 conversion-layer entry** (never a bare `??`, and no executor shims) so it is declared, loud, tested, and *removable on a schedule* — the `cfg.filter ?? cfg.filters`-style fallbacks the flow executors once carried were all paid down exactly that way, emptying and deleting the executor shim that read them. Stored `sys_metadata` rows (data at rest) are covered from the other side: every rehydration seam replays the **full** conversion chain — retired entries included — via `applyConversionsToStoredItem` (ADR-0087 addendum), so a consumer never needs its own accommodation for a legacy stored shape either. *Worked example:* an AI-authored flow node used wrong key names and template syntax for what the executor reads → the fix was correcting the authoring skill + a publish-gate lint that rejects the wrong shape, **not** a runtime alias in the executor (that alias was proposed and rejected). Strengthens #5. +13. **An accepted ADR binds until a superseding ADR says otherwise.** Reversing a recorded decision is itself a decision: it needs a **new ADR** (or an amended status line on the old one), not a changeset that quietly does the opposite. Before changing behaviour in `docs/adr/`-governed territory, **grep the ADRs for the surface you are touching** — the decision is often older and broader than the code comment in front of you. A reversal of three accepted ADRs once landed as a patch-level changeset and held for a day; the mechanism was not carelessness — **the file being edited never named the ADRs that governed it**, so the author could not have known. Hence the corollary: when you implement an ADR's decision, **leave its id in the code**, and anchor load-bearing spots in `scripts/adr-anchors/` (`pnpm check:adr-anchors`) — **one new JSON file per anchor, named for the path it anchors; there is no index to register it in** — so the next author is told which decision they are standing on. A decision nobody can find is a decision that will be reversed. +14. **⛔ An ADR is confirmed and merged by the maintainer, by hand — no AI seat merges, queues, or arms auto-merge on a `docs/adr/**` PR.** Maintainer ruling, 2026-08-08, verbatim and untranslated: > **adr 只能由维护者自己确认,人工合并,ai 不得擅自合并。** - **Authoring stays open to every seat.** Drafting an ADR, pushing the branch, opening the PR, revising it under review — all permitted, and none of it is what this directive touches. What is reserved is the **landing**: on any PR whose diff touches `docs/adr/**`, ⛔ never merge it, ⛔ never add it to the merge queue, ⛔ never call `enable_pr_auto_merge`. Judge it on the PR's **file list**, not on its description, and a **mixed diff is not a proportion question** — one path hit is enough; if the rest needs to land, split the ADR into its own PR. **Reviewed + approved + fully green does not override this.** Under #13 an accepted ADR *is* the decision, so merging one is the act of adopting a governance position — the one class of change about which "CI is green" carries no information at all. *Worked example:* #6668 — a thorough, fully-green, correctly-measured ADR draft for a capability **nobody had asked for**, closed by the maintainer on demand grounds no gate could have evaluated. Structurally identical to the version-release prohibition (maintainer 2026-08-07, #6170): in both, the existence of a mechanical path — a queue button, an `auto_merge` call — is not authorization to use it. - - **Already armed or queued when you read this?** ⚠️ Converting the PR back to **draft** is the only action that reliably removes it from the merge queue; `disable_pr_auto_merge` alone drops the arming but **not** queue membership. Do both, then confirm from the remote that it is in neither the queue nor `origin/main` (§7's third re-arm situation, run backwards). - - ⚠️ **And do not read draft as a barrier.** Measured on **#6732**: `draft: true` on the PR that was nevertheless merged at 14:38:56Z on 2026-08-08 — *after* exactly that disable-plus-draft reversal. Draft is a speed bump; the barrier is machine enforcement (**#6785** — `docs/adr/` in CODEOWNERS plus a required check that stays red unless the maintainer's own account has approved). Why that gate exists is this directive's own failure record: within one hour of the ruling, two **different** AI seats merged ADR PRs — #6671 at 14:23:32Z by `os-zhuang`, #6732 at 14:38:56Z by `os-project-manager`. The maintainer confirmed neither was theirs and ratified both retroactively — those two only, explicitly setting no precedent. So a seat that has read this far is not thereby licensed to judge an exception; the rule has no exception to judge. + **Authoring stays open to every seat** — drafting an ADR, pushing the branch, opening the PR, revising it under review. What is reserved is the **landing**: on any PR whose diff touches `docs/adr/**`, ⛔ never merge it, ⛔ never add it to the merge queue, ⛔ never call `enable_pr_auto_merge`. Judge it on the PR's **file list**, not on its description, and a **mixed diff is not a proportion question** — one path hit is enough; if the rest needs to land, split the ADR into its own PR. **Reviewed + approved + fully green does not override this.** Under #13 an accepted ADR *is* the decision, so merging one is the act of adopting a governance position — the one class of change about which "CI is green" carries no information at all (a thorough, fully-green ADR draft has been closed by the maintainer on demand grounds no gate could evaluate). **Already armed or queued when you read this?** ⚠️ Converting the PR back to **draft** is the only action that reliably removes it from the merge queue; `disable_pr_auto_merge` alone drops the arming but **not** queue membership. Do both, then confirm from the remote that it is in neither the queue nor `origin/main` (§7's draft-flip re-arm note, run backwards). ⚠️ **And do not read draft as a barrier that holds by itself** — a drafted ADR PR has nevertheless been merged, twice, by two different AI seats within one hour of the ruling above (both ratified retroactively, explicitly setting no precedent). The barrier is machine enforcement — `docs/adr/` in CODEOWNERS plus a required check that stays red unless the maintainer's own account has approved; this directive is the part that binds the seat reading it. A seat that has read this far is not thereby licensed to judge an exception; the rule has no exception to judge. -15. **⛔ A version release is performed by the maintainer, by hand — no AI seat publishes, tags, cuts a Release, or triggers a release workflow, and none merges the Version Packages PR.** Maintainer ruling, 2026-08-07 (#6170), verbatim and untranslated: +15. **⛔ A version release is performed by the maintainer, by hand — no AI seat publishes, tags, cuts a Release, or triggers a release workflow, and none merges the Version Packages PR.** Maintainer ruling, 2026-08-07, verbatim and untranslated: > **刚才我也没提出要求,是哪个ai自己替我发了 rc.4,版本发布必须是人工的。这个要写入规范。** - Its last sentence is this directive's warrant: 「这个要写入规范」. Until #6830 it had not been — the ruling lived only in `.claude/skills/pm-dispatch/SKILL.md`, a file exactly one lane loads, while its own text binds 「任何 AI 座位(PM / dev / Routine / 队列管家)」. A rule that binds every seat has to be readable by every seat; that is why it is here and not only there. + Its last sentence is this directive's warrant: 「这个要写入规范」. A rule that binds every seat has to be readable by every seat — which is why it lives here, not only in a lane-specific skill file. **Release-adjacent work stays open to every seat.** The release board, `.objectui-sha` pin bumps, version reconciliation, writing changesets, compiling release notes when asked, and *verifying* release state (`npm view`, `git ls-remote --tags`) are ordinary tasks. What is reserved is the **release act itself**: ⛔ running `changeset publish` / `pnpm run release`, ⛔ pushing a version tag, ⛔ cutting a GitHub Release, ⛔ pushing a runtime image, ⛔ `workflow_dispatch`-ing `release.yml` or any other publish-capable workflow, and ⛔ merging — or queueing, or arming auto-merge on — the **Version Packages** PR (`chore: version packages`). That PR is bot-authored and standing-open by design: it is regenerated on every push to `main`, so "green, current, and nobody has objected" is its permanent resting state, not a signal that it is due. When you find a publish nobody ordered — a tag or an npm version that simply appeared — ⛔ do not "repair" it with a counter-publish: file it as an incident for the maintainer. - **Release-adjacent work stays open to every seat.** The release board, `.objectui-sha` pin bumps, version reconciliation (#6149's field-level pass, #6169's merge-back), writing changesets, compiling release notes when asked, and *verifying* release state (`npm view`, `git ls-remote --tags`) are ordinary tasks — none of them is what this directive touches. What is reserved is the **release act itself**: ⛔ running `changeset publish` / `pnpm run release`, ⛔ pushing a version tag, ⛔ cutting a GitHub Release, ⛔ pushing a runtime image, ⛔ `workflow_dispatch`-ing `release.yml` or any other publish-capable workflow, and ⛔ merging — or queueing, or arming auto-merge on — the **Version Packages** PR (`chore: version packages`, today **#6208**). That PR is bot-authored and standing-open by design: it is regenerated on every push to `main`, so "green, current, and nobody has objected" is its permanent resting state, not a signal that it is due. And when you find a publish nobody ordered — a tag or an npm version that simply appeared — ⛔ do not "repair" it with a counter-publish: file it as an incident for the maintainer, which is how #6169 was handled. - - **The precedent is that the mechanical channel fires with nobody deciding to use it.** On 2026-08-07 `release.yml`'s `on: push` lane shipped **17.0.0-rc.4** end to end — 69 packages to npm, 69 tags at `a10cbc77`, GitHub Releases, and the runtime image — from run `31146224227`, event `push`, actor `github-merge-queue[bot]`: **no human, no dispatch, no seat clicked anything** (#6169 measured it; #6170 diagnosed it). The same mechanism had already shipped rc.3 four days earlier (#6135). Both publishes also skipped `check:objectui-pin-fresh`, and neither version commit ever reached `main`. So the existence of a path to a release is not authorization to walk it — the same sentence #14 makes about the queue button, one rule over. - - ⚠️ **And do not read "the human lane" as a barrier that holds against you.** #6172 closed the on-push hole: the publish job is now `if: github.event_name == 'workflow_dispatch'` behind `environment: release`, and the file's own comment calls `workflow_dispatch` the guarantee because "no push, no merge queue landing, no bot token and no schedule can synthesise this event". True for those four — **an authenticated seat calling the Actions API is not among them.** A `workflow_dispatch` is precisely the event an agent *can* synthesise, and whether that environment actually carries required reviewers is a repo-Settings fact this file cannot assert (the one-time setup is the maintainer's, #6170). The YAML stops the machine; this directive is the part that stops you. + **The precedent is that the mechanical channel fires with nobody deciding to use it.** The release workflow's `on: push` lane once shipped a full release candidate end to end — 69 packages to npm, tags, GitHub Releases, runtime image — with **no human, no dispatch, no seat clicking anything**, twice in one week. So the existence of a path to a release is not authorization to walk it — the same sentence #14 makes about the queue button. ⚠️ **And do not read "the human lane" as a barrier that holds against you.** The publish job is now gated on `workflow_dispatch` behind `environment: release`, and the file's own comment calls that the guarantee because no push, merge-queue landing, bot token or schedule can synthesise the event. True for those four — **an authenticated seat calling the Actions API is not among them.** A `workflow_dispatch` is precisely the event an agent *can* synthesise, and whether the environment carries required reviewers is a repo-Settings fact this file cannot assert. The YAML stops the machine; this directive is the part that stops you. --- ## Multi-agent working discipline -This repo is worked on by **multiple agents in parallel**. **Use one git -worktree per agent/task** (`git worktree add ../objectstack- -b `; -run `pnpm install` in the new tree) so file systems are physically isolated — -this is mandatory, not a preference (Prime Directive #11), and a PreToolUse hook -blocks edits made while on the shared `main` branch. Working in the shared `main` -checkout is *not* a supported fallback: branches get switched and shared files — -including ones you just wrote — get reset *under you* mid-task (a full session's -work was silently reverted twice before this rule was enforced). +This repo is worked on by **multiple agents in parallel**. **Use one git worktree per +agent/task** (`git worktree add ../objectstack- -b `; run `pnpm install` +in the new tree) so file systems are physically isolated — mandatory, not a preference +(Prime Directive #11), and hook-enforced. Working in the shared `main` checkout is *not* +a supported fallback: branches get switched and shared files — including ones you just +wrote — get reset *under you* mid-task (full sessions of work were silently reverted +this way before the rule was enforced). **⛔ `git stash` is the one thing the worktree does NOT isolate — never run a bare -`git stash push`/`pop`.** The worktree gives you your own working tree and your own -HEAD; it does **not** give you your own stash. `refs/stash` and its reflog live in the -**common** `.git` directory, so every linked worktree pushes onto and pops off **one -shared LIFO stack**. Two agents stashing at the same time swap entries: A's `pop` -restores B's changes into A's worktree, A's own work stays on the stack for B to take, -and **`pop` reports success** — the only symptom is another agent's files appearing in -your `git status`, after which a `git add -A` commits their half-finished work into your -PR. Not hypothetical: objectui#3430 (2026-08-06) did exactly this to two parallel dev -agents mid reverse-verification, and both changesets survived only as unreachable commits -whose SHAs happened to still be in scrollback — once the stack empties, `refs/stash` and -`logs/refs/stash` are gone (`git reflog refs/stash` → `fatal: ambiguous argument`) and a -`git gc` in between makes the loss permanent. Reverse verification ("revert the fix, watch -the diagnostics") is the workflow every dev agent runs, which is exactly why the collision -window is wide. Use one of these instead — no shared state, all inside your own worktree: +`git stash push`/`pop`.** `refs/stash` lives in the **common** `.git` directory, so +every linked worktree pushes onto and pops off **one shared LIFO stack**. Two agents +stashing at the same time swap entries — your `pop` restores the other agent's changes, +your own work stays on the stack for them to take, and **`pop` reports success**; the +only symptom is another agent's files in your `git status`, after which a `git add -A` +commits their half-finished work into your PR. It has really happened to two parallel +agents mid reverse-verification, both changesets recoverable only as unreachable +commits. A PreToolUse hook (`.claude/hooks/guard-shared-stash.sh`; details and the +`OS_ALLOW_STASH=1` escape in its header, self-test alongside) blocks the mutating forms +— including `stash@{N}`, a *position* in a stack you don't own — and allows +`list`/`show`/`create` and `apply`/`store` pinned to a literal hex object id. It fails +open on shapes it cannot parse, so the rule still outranks the hook. The collision-free +replacements, all inside your own worktree: ``` git diff > /tmp/wip.patch && git checkout -- # then: git apply /tmp/wip.patch @@ -196,219 +180,137 @@ git commit -am wip # then: git reset --soft HEAD~1 git worktree add ../objectstack--cmp # a second tree to compare against ``` -**Doing reverse verification? Commit the fix FIRST.** Once it is committed, restoring is -`git checkout -- ` — pulling the file back out of a commit that really -exists. Running the same deletion against an **uncommitted** edit -(`git checkout origin/main -- `) leaves you no restore point at all: the working tree -is the only copy, `git stash` is banned by the rule above, and `git checkout -- ` -discarding local modifications is a normal, silent, exit-0 operation — no warning, no -conflict, no "would be overwritten" refusal. The change is simply gone. Landed twice in one -day in `objectui` (#4278 / PR #4293, #4243 / PR #4299) and once here (#7739 / PR #7791); -every recovery depended on the change still being in the session transcript, so an -incomplete transcript is a net loss. If you ever do retype a lost change, prove it is -identical with `git diff` against a saved patch or `git hash-object ` — a matching -`git diff --stat` insertion count is **not** byte-identity, and this failure mode is silent -enough that only the strong check is worth anything. Then re-run the reverse verification -from the committed state, so the red/green numbers you report are trustworthy. - -A third PreToolUse hook (`.claude/hooks/guard-shared-stash.sh`, mirrored from objectui -after that incident — #5742) enforces this on the `Bash` matcher: it blocks the mutating -forms (`push`/`pop`/`save`/`drop`/`clear`/`branch`, including `stash@{N}` positions, which -are positions in a stack you don't own) and allows what cannot take another agent's entry -— `git stash list`/`show`/`create`, and `apply`/`store` pinned to a **literal hex object -id**. It fails open on shapes it cannot parse (`bash -c …`, `xargs`), so the rule still -outranks the hook. Deliberate exception when the stack really is yours alone: -`OS_ALLOW_STASH=1`. Changing the hook? Re-run `.claude/hooks/guard-shared-stash.selftest.sh`. +**Doing reverse verification ("revert the fix, watch the diagnostics")? Commit the fix +FIRST.** Committed, restoring is `git checkout -- ` — pulling the +file back out of a commit that really exists. Against an **uncommitted** edit, +`git checkout origin/main -- ` leaves no restore point at all: the working tree +is the only copy, the stash is banned above, and discarding local modifications is a +normal, silent, exit-0 operation. The change is simply gone — it has happened +repeatedly across the sibling repos, and every recovery depended on the change still +being in the session transcript. If you ever retype a lost change, prove identity with +`git diff` against a saved patch or `git hash-object ` — a matching `--stat` +insertion count is **not** byte-identity — then re-run the reverse verification from +the committed state, so the red/green numbers you report are trustworthy. **Claim the issue BEFORE you write any code.** Assign it to yourself -(`gh issue edit --add-assignee @me`, or the `issue_write` MCP tool with -`assignees`) as the *first* action of the task — before the worktree, before the -first read. An unassigned issue reads as an open invitation, and several agents -work this repo at once: two that both start on it burn the same hours twice and -then race to land conflicting shapes for the same problem, which is worse than -either one alone. If it is already assigned to someone else it is taken — pick -another, or say so and ask; never reassign it to yourself. - -Because every agent here shares one GitHub identity, the assignee field alone -cannot answer "is this claim *mine*?" — seeing your own shared name on an issue -is exactly what another session's claim looks like. So a claim is two acts, not -one: assign, **and leave a claim comment carrying your session ID and branch -name** (`claude/issue--`). Before writing code, re-read the issue's -comments; an earlier claim comment with a different session ID or branch means -the issue is taken no matter what the assignee field seems to say. Skipping -this read is how #4551 got implemented twice in one morning (#4555 and #4559 — -post-mortem in #4588), and misreading shared-identity state is also how a -maintainer's manual ready-flip got reverted by an agent that assumed its own -write had failed. +(`gh issue edit --add-assignee @me`, or `issue_write` with `assignees`) as the +*first* action of the task — before the worktree, before the first read. An unassigned +issue reads as an open invitation: two agents that both start on it burn the same hours +twice, then race to land conflicting shapes for one problem. Already assigned to +someone else? It is taken — pick another or ask; never reassign it to yourself. And +because every agent here shares one GitHub identity, the assignee field alone cannot +answer "is this claim *mine*?" — a claim is two acts: assign, **and a claim comment +carrying your session ID and branch name** (`claude/issue--`). Before writing +code, re-read the issue's comments; an earlier claim with a different session ID or +branch means the issue is taken no matter what the assignee field seems to say — +skipping that read is how one issue got implemented twice in one morning. The claim is +also what makes the *finding* rule (Prime Directive #10) safe: once findings become +issues, the list is a real queue other agents read — file unassigned when merely +recording; assign at the moment you actually start. **State on your PR that you did not set belongs to another actor — ask, never -"correct" it.** That is the same misread one step on, and the more expensive -half. Under one shared identity every other participant's write arrives +"correct" it.** Under one shared identity every other participant's write arrives unsigned: the PM flipping your draft to ready and arming auto-merge, a bot -re-labelling, the platform rewriting your body. #6567 is the worked example — -a dev read its own PR body back, found the trailing `Generated by [Claude -Code]` footer in a form it had never typed, correctly concluded *something is -rewriting my PR*, then carried that conclusion to the **draft flag** and -flipped the PM's ready PR back to draft. That drops auto-merge and queue -membership at once (§7's third re-arm situation), and `pull_request_read` -reports neither, so the agent could not see what it had destroyed. The -observation was right; the second inference did not follow from it — body -rewriting is a known platform behaviour and is **evidence of nothing else**. -So when state you did not write changes under you: read the timeline event's -actor, or ask the PM. Undo it only once you know who set it and why. - -**Write the attribution footer in its session-URL form** — that is the half of -the above you can act on directly. Measured on PR #6556 and recorded in #6567: +re-labelling, the platform rewriting your body. The worked failure: a dev found its PR +footer in a form it had never typed, correctly concluded *something is rewriting my +PR*, then extended that to the **draft flag** and flipped the PM's ready PR back to +draft — destroying auto-merge and queue membership at once (§7's draft-flip re-arm +note), invisibly (`pull_request_read` reports neither). The observation was right; +the inference did not follow — body rewriting is a known platform behaviour and is +evidence of nothing else. When state you did not write changes under you: read the +timeline event's actor, or ask. Undo it only once you know who set it and why. + +**Write the attribution footer in its session-URL form** — the half of the above you +can act on directly: ```text _Generated by [Claude Code](https://claude.ai/code)_ ← stripped on edit _Generated by [Claude Code](https://claude.ai/code/session_)_ ← survives ``` -A body ending in the bare form loses the **whole** footer, `---` separator -included, on every `update_pull_request` edit (reproduced twice, including with -a blank line before the separator), while the session-URL form survives both -write paths — #6556 still carries it. `create_pull_request` does not -strip; it *rewrites* the bare form into the session form, which is precisely -how a body comes back in a shape nobody typed. **Which layer does this is -unknown** — platform sanitizer, MCP tool layer, or a workflow's body -post-processing — and the guidance does not depend on the answer, so nobody -should spend a session establishing it. Comments are a different path and are -unaffected: the bare form survives untouched in issue and PR comments, -including the two on #6567 itself. - -The claim is also what makes the *finding* rule (Prime Directive #10) safe to -follow. Once out-of-scope discoveries become issues, the issue list is a real -queue other agents read, and a claim is the only thing separating "someone is on -this" from "nobody has looked yet". File it unassigned when you are merely -recording a finding; assign it at the moment you actually start. +A body ending in the bare form loses the **whole** footer, `---` separator included, on +every `update_pull_request` edit; the session-URL form survives both write paths. +`create_pull_request` does not strip; it *rewrites* the bare form into the session form +— which is precisely how a body comes back in a shape nobody typed. Which layer does +this is unknown, and the guidance does not depend on the answer — do not spend a +session establishing it. Comments are a different path and unaffected. Even inside your own worktree, operate defensively: -1. **Only touch the files your task needs.** Don't "fix" unrelated diffs, - reverts, or other agents' in-flight edits, and don't try to manage the whole - working tree. If a file you didn't change shows as modified, leave it. -2. **One feature branch + one PR per task.** Branch off `main`. **Never commit - task work straight to `main`.** Name the branch after the issue it fixes: - `claude/issue--`. The issue number in the name is what makes - in-flight work *discoverable* — `git ls-remote --heads origin | grep - issue-` is a one-command pre-check, and the Duplicate Fix Guard workflow - warns on fix PRs whose branch names no declared issue. The #4555/#4559 - duplicate (#4588) stayed invisible partly because one branch carried the - issue number and the other didn't. +1. **Only touch the files your task needs.** Don't "fix" unrelated diffs, reverts, or + other agents' in-flight edits, and don't try to manage the whole working tree. If a + file you didn't change shows as modified, leave it. +2. **One feature branch + one PR per task.** Branch off `main`. **Never commit task + work straight to `main`.** Name the branch after the issue it fixes: + `claude/issue--`. The issue number in the name is what makes in-flight work + *discoverable* — `git ls-remote --heads origin | grep issue-` is a one-command + pre-check, and the Duplicate Fix Guard workflow warns on fix PRs whose branch names + no declared issue. A duplicated implementation once stayed invisible partly because + one branch carried the issue number and the other didn't. 3. **Never `git push --force` / `--force-with-lease`, and never push `main`.** A - force-push can clobber a parallel agent's work; `main` is shared — land - everything via PR. + force-push can clobber a parallel agent's work; `main` is shared — land everything + via PR. 4. **Verify the current branch before every commit/push** - (`git rev-parse --abbrev-ref HEAD`). HEAD may have been switched by another - agent — if it isn't your feature branch, stop and re-checkout before pushing. -5. **Shared files (barrels/registries like `builtin/index.ts`): edit → `git add` - → commit atomically, then confirm the commit really contains your lines** - (`git show HEAD: | grep `). A concurrent edit can revert - your working-tree change between the edit and the commit. On a real conflict, - re-apply only *your* lines and let the PR merge integrate the rest. + (`git rev-parse --abbrev-ref HEAD`). HEAD may have been switched by another agent — + if it isn't your feature branch, stop and re-checkout before pushing. +5. **Shared files (barrels/registries like `builtin/index.ts`): edit → `git add` → + commit atomically, then confirm the commit really contains your lines** + (`git show HEAD: | grep `). A concurrent edit can revert your + working-tree change between the edit and the commit. On a real conflict, re-apply + only *your* lines and let the PR merge integrate the rest. 6. **Don't rebase or force-update shared branches** to tidy other agents' commits. -7. **Land through the merge queue: arm auto-merge on a PR that is already - green, accepted and non-draft, then let the queue merge it.** Arming is how - you enter the queue, and the queue is what makes arming safe — it rebuilds - your PR *as merged onto the current `main`*, re-runs the subscribing - workflows on that rebuilt generation, and lands it only if the required ones - pass. That is the §10 re-verification, done by the platform, race-free. - - ⛔ **Two classes of PR never enter this path, however green:** (a) a diff - that touches `docs/adr/**` (**Prime Directive #14**); (b) the **Version - Packages** PR — `chore: version packages`, today #6208 — or any other PR - whose merge performs a release (**Prime Directive #15**). Do not merge them, - do not queue them, do not arm them — read the PR's file list (`get_files`) - **and its author** before you arm anything, and see those two directives for - the rulings, for why "accepted and green" is not an exception, and for how - to get an already-queued one back out. - - **What "the queue validates" means here, measured** (`origin/main`, - 2026-08-07): three of this repo's 22 workflows carry an `on: merge_group:` - trigger — `ci.yml`, `lint.yml`, `spec-liveness-check.yml` — and the Actions - API reports **2742** `merge_group` runs, the most recent 30 all on - `gh-readonly-queue/main/pr--` refs, all three workflows, all green. - A fourth workflow, `merge-queue-triage.yml`, is *not* a subscriber: it - watches those runs through `workflow_run` and comments the diagnosis on the - PR when a queue build goes red (#4859). - - **This supersedes the older "never `gh pr merge --auto`" ban.** Its premise — - auto-merge lands a still-red PR on shared `main` (#1475) — is inverted by - rebuild-then-land, and the ban forbade what is now the sanctioned path. Its - *true half* survives, as a precondition rather than a prohibition: **arm only - what is already green and accepted**, where green means the gate-carrying - jobs' `conclusion` is `success`, not "no failure yet" — `in_progress` is not - a pass. Arming a red PR does not queue it, it hides it: #4852 sat armed from - 10:15 for **100 minutes** without ever entering the queue, every poll - misreading "not on `main` yet" as "queued". So always read *two* things when - checking on a landing: the queue branch **and** `origin/main`. - - **The queue enforces only the required set; everything else is advisory and - rides through.** #6067's final queue generation - (`gh-readonly-queue/main/pr-6067-db0d53c2…`) had `Lint & Type Check` at - `completed/failure` — run 31136745851, concluded 01:12:11Z — and merged at - 01:13Z regardless; the `check:slot-lookup` red it carried then rode `main`'s - merge ref into every following PR's ESLint job until it was stanched (#6100, - the same shape as #5584 → #5601 → hot-fix #5615). Governance half: **#5617**, - under which the maintainer on 2026-08-07 added **ESLint** and **TypeScript - Type Check** to both `main`'s required-status-check set and the queue's check - set, so those two now block — the audit archived on that issue also lists - which other jobs can and cannot safely join them. A gate outside that set - stops nothing, which is why "arm only on green" is a rule and not a - formality. - - **Three re-arm situations this repo has actually hit.** None of them is a - reason to avoid the queue; all are reasons to confirm a PR is still *in* it: - - **A red queue build ejects your entry and drops the auto-merge.** The - failure is often in a package your PR never touched, because the queue runs - the *full* suite while the PR ran affected-only. #6059 was ejected at - 01:03:02Z on a known flaky (`datasource-pool-support.test.ts`, #6044), - diagnosed against the triage comment, re-armed at 01:04:15Z and landed at - 01:25:00Z. Recognise the signature first, then re-arm once — never re-queue - reflexively. - - **Collateral eviction is silent by design.** `merge-queue-triage.yml` - comments only on `conclusion == failure`; an entry cancelled because - something *ahead* of it failed gets nothing, since that outcome says - nothing about your PR. A PR that is neither on `main` nor in the queue was - dropped — re-arm it. - - **Flipping back to draft drops auto-merge and queue membership at once, and - neither returns by itself.** The order is therefore fixed: ready *first*, - arm *second*. (This one is the repo's standing operating note — - `.claude/skills/pm-dispatch/SKILL.md` note 1 — not an API measurement.) - - And one non-fix: **a stale red does not clear by re-running.** - `rerun_failed_jobs` reuses the original run's commit and merge ref, so it - cannot see a fix that landed on `main` since. Compare the fix's merge time - against the run's creation time; if the fix is later, only a new commit - (`git merge origin/main`) helps — #4852's red was byte-identical across a - rerun until #4856 landed. - - **Not measured here:** whether a direct, non-auto `gh pr merge` is refused - with `405 Changes must be made through the merge queue`. Establishing that - would mean actually attempting a merge on a live PR, which is not an - experiment worth running. objectui returns 405; that is **not** extrapolated - to this repo (objectui#3243) — separate rulesets, and #5617's cross-repo - audit found the two configured differently. - - **Fallback, when the queue is unavailable:** the old manual protocol — merge - serially, only after remote CI is fully green, rebasing other open branches - before merging the next one. It is a fallback because it loses under load: - `main` can land a PR every few minutes at peak while a manual merge–reverify - loop takes ~25 minutes, so one PR went three full green cycles without - managing to land. That is a livelock, not a discipline failure — and it is - why the queue is the default path rather than an optimisation. +7. **Land through the merge queue: arm auto-merge on a PR that is already green, + accepted and non-draft, then let the queue merge it.** The queue rebuilds your PR + *as merged onto the current `main`*, re-runs the subscribing workflows on that + rebuilt generation, and lands it only if the required ones pass — the §10 + re-verification, done by the platform, race-free. This sanctioned path supersedes + the older blanket auto-merge ban; the ban's true half survives as the precondition: + **arm only what is already green and accepted.** + + ⛔ **Two classes of PR never enter this path, however green:** (a) a diff touching + `docs/adr/**` (**Prime Directive #14**); (b) the **Version Packages** PR, or any PR + whose merge performs a release (**Prime Directive #15**). Read the PR's file list + (`get_files`) **and its author** before you arm anything. + + **Green means the gate-carrying jobs' `conclusion` is `success`** — not "no failure + yet"; `in_progress` is not a pass. Arming a red PR does not queue it, it hides it: + every poll then misreads "not on `main` yet" as "queued". Always read *two* things + when checking a landing: the queue branch **and** `origin/main`. And **the queue + enforces only the required set** — **ESLint** and **TypeScript Type Check** block by + maintainer decision (2026-08-07); everything else is advisory and rides through, and + an advisory red that lands rides `main`'s merge ref into every following PR until + stanched. That is why "arm only on green" is a rule, not a formality. + + **Re-arm awareness** — none of these is a reason to avoid the queue; all are reasons + to confirm a PR is still *in* it: a red queue build **ejects** your entry and drops + auto-merge, often on a package your PR never touched (the queue runs the full suite; + the PR ran affected-only) — diagnose against `merge-queue-triage.yml`'s comment, + recognise a known-flaky signature, then re-arm once, never reflexively; **collateral + eviction is silent** (triage comments only on `failure`, so an entry cancelled + because something *ahead* failed gets nothing) — neither on `main` nor in the queue + means dropped, re-arm; **flipping back to draft drops auto-merge and queue + membership at once**, and neither returns by itself — ready *first*, arm *second*. + One non-fix: **a stale red does not clear by re-running** — `rerun_failed_jobs` + reuses the original run's commit and merge ref, so a fix that landed on `main` since + is invisible to it; only a new commit (`git merge origin/main`) helps. Whether a + direct `gh pr merge` is refused here is deliberately unmeasured — do not establish + it by attempting one. + + **Fallback, when the queue is unavailable:** merge serially, only after remote CI is + fully green, rebasing other open branches before merging the next. It loses under + load (`main` lands a PR every few minutes at peak; a manual merge–reverify loop + takes ~25) — which is why the queue is the default path, not an optimisation. 8. **Testing needs a server? Start your own temporary one — never stop someone - else's.** A running dev server you didn't start probably belongs to another - agent or the user; killing it (or its port) breaks their in-flight work. Spin - up your own instance on a random high port (`pnpm dev -- --fresh -p `) - and **shut it down yourself when the task is done** - (`kill $(lsof -ti tcp:)`). Don't leave orphan servers behind. -9. **After pulling `main` into a long-lived worktree, refresh its build state - before you trust a single test or gate.** A worktree that has been open across - several merges accumulates artefacts that are stale relative to the source, and - every one of them fails **as if your change broke something** — naming other - people's exports, other packages' files, or config you never touched: + else's.** A running dev server you didn't start probably belongs to another agent or + the user; killing it (or its port) breaks their in-flight work. Spin up your own + instance on a random high port (`pnpm dev -- --fresh -p `) and **shut it + down yourself when the task is done** (`kill $(lsof -ti tcp:)`). Don't leave + orphan servers behind. +9. **After pulling `main` into a long-lived worktree, refresh its build state before + you trust a single test or gate.** A worktree that has been open across several + merges accumulates artefacts that are stale relative to the source, and every one of + them fails **as if your change broke something** — naming other people's exports, + other packages' files, or config you never touched: | stale artefact | how it presents | why it lies | |---|---|---| @@ -419,100 +321,62 @@ Even inside your own worktree, operate defensively: So after any `git merge origin/main`: `pnpm install --frozen-lockfile && pnpm build && rm -rf packages/runtime/.objectstack` - (add `rm -rf .cache` if you have run the console build). Note `OS_SKIP_DTS=1` - keeps a build fast but leaves no `.d.ts`, so `gen:api-surface` cannot run at - all under it — that one needs a real build. - - None of this is CI-visible: CI checks out fresh and installs clean. It costs - only *your* time, which is exactly why it is worth recognising in one step - rather than re-diagnosing per gate. - - **One row of that table now has a gate, and only one** (#5864). `pnpm dev` - refuses to boot on a stale `packages/spec/dist`: its build stamps a hash of - its own inputs into `dist/.build-input-hash`, and `check:dev-prereqs` - recomputes and compares. Content, never mtime — so a checkout, a `touch` or - a clock skew cannot false-red it, and `git worktree add` is invisible to it. - For every other row this prescription is still the whole remedy: the gate's - pass line says "existence, not freshness" about the other ~66 packages - precisely so its green cannot be read as vouching for them. -10. **A clean merge is not a working merge — but scope the re-check to the - overlap.** Git conflicts on overlapping lines; nothing warns you when two - changes are individually fine and jointly wrong. Real examples from one - branch's lifetime: a test asserting a response body's exact shape landed - while that shape was being changed elsewhere (merged clean, failed CI); a - domain file was deleted while another agent's guard still declared it. - **Before opening a PR, pull `main`, refresh build state (§9), and run the - full suite once.** For the *subsequent* pre-merge merges of `main` — the - ones you do only because `main` moved again while CI ran — the full suite is - usually re-proving what three identical runs already proved, at ~15 minutes - per lap while `main` lands a PR every few. Scope it instead: - - **Always:** rebuild what the merge touched, and if `packages/spec` moved - on either side, `pnpm --filter @objectstack/spec build && pnpm --filter - @objectstack/spec check:generated` — generated snapshots (`api-surface`, - baselines) are the classic jointly-wrong artifact, and only a rebuild of - the merged source can validate them (never trust git's textual merge of a - generated file). Then assert your branch's *delta vs `main`* is still - exactly what your PR intends (e.g. "N removed / 0 added"). - - **Full `pnpm typecheck && pnpm test` again only when** the incoming - commits touch the same packages or the same behavior your diff does, or a - conflict occurred outside trivially-mechanical files. - - CI on the PR, and then the merge queue on its rebuilt generation (§7), - validates the merge commit itself — that second CI round is where joint - breakage surfaces, and - the guards in `scripts/check-*.mjs` exist largely because this class of + (add `rm -rf .cache` if you have run the console build). Note `OS_SKIP_DTS=1` keeps + a build fast but leaves no `.d.ts`, so `gen:api-surface` cannot run at all under it — + that one needs a real build. None of this is CI-visible (CI checks out fresh), which + is exactly why it is worth recognising in one step rather than re-diagnosing per + gate. Only the first row has a gate: `check:dev-prereqs` refuses `pnpm dev` on a + stale `packages/spec/dist` (content-hash, never mtime); for every other row the + prescription above is the whole remedy — the gate's own pass line says "existence, + not freshness" so its green cannot be read as vouching for them. +10. **A clean merge is not a working merge — but scope the re-check to the overlap.** + Git conflicts on overlapping lines; nothing warns you when two changes are + individually fine and jointly wrong (a test asserting a response body's exact shape + has landed while that shape was being changed elsewhere — merged clean, failed CI; a + domain file has been deleted while another agent's guard still declared it). + **Before opening a PR, pull `main`, refresh build state (§9), and run the full suite + once.** For the *subsequent* pre-merge merges of `main` — the ones you do only + because `main` moved again while CI ran — the full suite is usually re-proving what + three identical runs already proved, at ~15 minutes per lap while `main` lands a PR + every few. Scope it instead: + - **Always:** rebuild what the merge touched, and if `packages/spec` moved on either + side, `pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec + check:generated` — generated snapshots (`api-surface`, baselines) are the classic + jointly-wrong artifact, and only a rebuild of the merged source can validate them + (never trust git's textual merge of a generated file). Then assert your branch's + *delta vs `main`* is still exactly what your PR intends (e.g. "N removed / 0 + added"). + - **Full `pnpm typecheck && pnpm test` again only when** the incoming commits touch + the same packages or the same behavior your diff does, or a conflict occurred + outside trivially-mechanical files. + - CI on the PR, and then the merge queue on its rebuilt generation (§7), validates + the merge commit itself — that second CI round is where joint breakage surfaces, + and the guards in `scripts/check-*.mjs` exist largely because this class of breakage is invisible to `git merge`. -11. **Generated artifacts don't text-merge — a driver defers them and - `pre-commit` collects the debt.** §10's "never trust git's textual merge of a - generated file" is now mechanical (#4675). `.gitattributes` routes the - generator-owned artifacts (`spec-changes.json`, `authorable-surface/**`, - `authorable-surface.base.json`, `api-surface/**`, - `api-surface-signatures.json`, `json-schema.manifest/**`, - `docs/protocol-upgrade-guide.md`, `content/docs/references/**`) to - `merge=os-regen`, so a merge that used to stop on conflicts across all of - them now stops only on the hand-written files that actually need you. - - **The driver is a LOCAL facility, and #5837 is where that bound showed.** The - GitHub merge queue rebuilds each PR server-side, where no custom merge driver - runs — so two PRs that both touched `authorable-surface.json` (a 310KB sorted - array every spec PR rewrites) were a plain textual conflict there and the - second was evicted, capping the spec lane at one PR at a time. The three - hottest artifacts are therefore **sharded**: `authorable-surface/.json`, - `json-schema.manifest/.json`, `api-surface/.json`. PRs - touching different categories now touch disjoint files, and the driver keeps - the residue (two PRs in the same category). Every gate reads the whole - directory as one set, so the ratchet semantics are unchanged — see - `packages/spec/scripts/lib/sharded-artifacts.ts`. Deliberately still single - files: `spec-changes.json` (keyed by version), `api-surface-signatures.json` - (1.3KB) and `authorable-surface.base.json` (written only by an explicit - `--update-base`, so never on the churn path). - - The driver does **not** regenerate. Git runs merge drivers *while* it merges, - in index order, so the worktree still holds pre-merge sources — a generator - run there would describe a half-merged tree and write a confidently wrong - artifact, which is strictly worse than the conflict it replaced. Instead it - records each path in `$GIT_DIR/os-regen-pending`, and `pre-commit` refuses the - commit until those artifacts check clean. So the sequence after a merge is - unchanged from §9 — rebuild, then `check:generated --fix` — you just cannot - forget it. - - Two things worth knowing: - - **Registration is per clone.** `pnpm install` does it (`prepare` → - `scripts/setup-git-hooks.mjs`). A clone where that never ran falls back to - git's default text merge — pre-#4675 behaviour, not breakage — so nothing - depends on every machine being set up. - - **The ratchets are deliberately excluded** - (`docs-import-surface.baseline.json`, `dual-source-exports.baseline.json`, - the hand-written `migrations`/`conversions` registries, `variant-docs.json`). - Recomputing a shrink-only ratchet can *widen* it, which would launder a new - exemption in as merge noise. Those conflicts are yours to read. See - `NOT_DRIVER_MANAGED` in `scripts/regen-artifacts.mjs` for why, per path. - - Related: `check:generated --fix` now **refuses** to run `gen:api-surface` on a - stale `dist` rather than warning about it (§9's trap, made unsurvivable on the - one path that writes). - - `pnpm check:merge-driver` reconciles `.gitattributes` against that table in - both directions and proves the driver end to end against real git. +11. **Generated artifacts don't text-merge — a driver defers them and `pre-commit` + collects the debt.** §10's "never trust git's textual merge of a generated file" is + mechanical: `.gitattributes` routes the generator-owned artifacts to + `merge=os-regen` (the list is the `.gitattributes` entries themselves; `pnpm + check:merge-driver` reconciles both directions), so a merge stops only on the + hand-written files that actually need you. The driver does **not** regenerate — git + runs merge drivers while the worktree still holds pre-merge sources, so a generator + run there would describe a half-merged tree; instead it records each path in + `$GIT_DIR/os-regen-pending` and `pre-commit` refuses the commit until those + artifacts check clean. Sequence after a merge unchanged from §9: rebuild, then + `check:generated --fix` — you just cannot forget it. Worth knowing: + - **The driver is a LOCAL facility** — the merge queue rebuilds server-side where no + custom driver runs, so the three hottest artifacts are **sharded** per + category/entry (`authorable-surface/`, `json-schema.manifest/`, `api-surface/`) to + keep parallel spec PRs textually disjoint; every gate reads the whole directory as + one set, so ratchet semantics are unchanged + (`packages/spec/scripts/lib/sharded-artifacts.ts`). + - **Registration is per clone** (`pnpm install` → `prepare` → + `scripts/setup-git-hooks.mjs`); an unregistered clone falls back to git's default + text merge — older behaviour, not breakage. + - **The ratchet baselines are deliberately excluded**: recomputing a shrink-only + ratchet can *widen* it, laundering a new exemption in as merge noise — those + conflicts are yours to read (`NOT_DRIVER_MANAGED` in `scripts/regen-artifacts.mjs` + says why, per path). --- @@ -593,13 +457,12 @@ Root also exports: `defineStack`, `composeStacks`, `defineView`, `defineApp`, `d ### Touched `packages/spec`? Regenerate its artifacts BEFORE pushing -`packages/spec` has **eight** checked-in generated artifacts, each with its own CI gate. -All of them live in one job — `TypeScript Type Check` in `lint.yml`, which is required and -has no paths filter, so no gate can go dormant on the PR that breaks it (#4291 retired the -filtered `Check Generated Artifacts` job for exactly that reason). That job runs its gates -**sequentially**, so the first stale artifact masks every one behind it, and you get one -red build per artifact instead of one for all of them. Match the change to the gate and -regenerate up front: +`packages/spec` has **eight** checked-in generated artifacts, each with its own CI +gate. All of them live in one job — `TypeScript Type Check` in `lint.yml`, which is +required and has no paths filter, so no gate can go dormant on the PR that breaks it. +That job runs its gates **sequentially**, so the first stale artifact masks every one +behind it, and you get one red build per artifact instead of one for all of them. Match +the change to the gate and regenerate up front: | You changed | Gate that fails | Regenerate with `pnpm --filter @objectstack/spec …` | |:---|:---|:---| @@ -610,12 +473,10 @@ regenerate up front: | A `SKILL.md` (frontmatter or body) | `check:skill-docs`, `check:skill-refs` | `gen:skill-docs`, `gen:skill-refs` | | The react-blocks contract | `check:react-blocks` | `gen:react-blocks` | -A `.describe()` string counts — it is not "just a comment", it lands in -`content/docs/references/`. Adding one export counts — it lands in `api-surface/`. -Both were learned the hard way in #4040: two separate red builds, neither a logic error. - -Don't match by hand — one command runs **every** gate and reports **all** stale -artifacts at once, which is precisely what CI cannot do: +A `.describe()` string counts — it lands in `content/docs/references/`. Adding one +export counts — it lands in `api-surface/`. Don't match by hand — one command runs +**every** gate and reports **all** stale artifacts at once, which is precisely what CI +cannot do: ```bash pnpm --filter @objectstack/spec build # REQUIRED first — see the dist caveat @@ -623,97 +484,54 @@ pnpm --filter @objectstack/spec check:generated # every gate; the first failur pnpm --filter @objectstack/spec check:generated --fix # regenerate ONLY the ones it proved stale ``` -`--fix` is deliberately narrow. Regenerating the whole set on principle destroys the -signal: it rewrites artifacts whose staleness you never saw, so a real semantic change -lands silently inside a mechanical diff. Let the check tell you which are stale, then -regenerate those. - -**No `check:` script regenerates anything — that is the point of the split, not an -oversight.** `check:docs` used to begin with `pnpm gen:schema`, which rewrites two -*tracked* files (`json-schema.manifest/`, `authorable-surface/`) whenever they -are behind: running the gate edited your working tree and reported nothing, so a -`check:generated` run on a stale manifest printed a red `check:authorable-surface` -over a file the gate two lines below had already quietly fixed (#4711, #4723). The -generation belongs to the **caller** now — `pnpm build`, or the -`check:authorable-surface` gate that runs before `check:docs` in both CI and -`check:generated`, whose `--check` mode writes the gitignored `json-schema/` tree and -refuses to touch a tracked one. Consequence for you: **`check:docs` is not -self-sufficient**. Run the `build` line above first (it is already required for the -`dist` caveat below) — `build-docs.ts` refuses on a missing or stale tree and names -the command, so the failure is loud, never a wrong verdict. - -The script carries its own ledger of gate → generator and **reconciles it against -`package.json` on every run**, in both directions. A new `check:`/`gen:` script that -nobody classified fails the run rather than quietly dropping out of coverage — the -failure mode a hardcoded list here would have had. (It caught its own `package.json` -entry on the very first run.) CI runs the same reconciliation on every PR -(`--reconcile-only`, in lint.yml's required typecheck job), so an unclassified script -fails its own PR instead of landing on `main` and turning this wrapper red for -everyone else — which happened twice before the CI step existed (#4203, #4232). - -⚠️ **`check:api-surface` reads the built `dist/*.d.ts`, not `src/`.** A stale `dist` -makes it report exports as **removed** — "N breaking (removed/narrowed)" — when nothing -was removed at all: the snapshot is simply newer than your build. Rebuild before you -believe it, and before you file a bug about `main` being red. (Two phantom "breaking -removals" this way while writing this section; `check:generated` now prints this caveat -inline when that gate is the one failing.) - -`check:liveness`, `check:empty-state`, `check:skill-examples`, `check:exported-any` and -`check:dual-source-exports` are -pure checks with no generator — a failure there is a real finding to fix, not an artifact -to regenerate. `check:generated` names them as deliberately not run, so its "all up to -date" never reads as "everything passed". The last one asks the third question about the -export surface (#4446): `api-surface/` shows a name on two entries but not whether -that is one declaration re-exported (fine) or two declarations sharing a name — the #4411 -trap, judged by symbol identity against the built dist, with the accepted cases in the -shrink-only `dual-source-exports.baseline.json` (hand-edited under review, never -generated: a `gen:` would admit a new dual-source via "run the fix command"). - -⚠️ **`check:react-declaration-parity` compares two DECLARATIONS, not a declaration against -an implementation.** Left: the props a block's spec zod schema declares. Right: the inputs -the objectui *registry config* declares. Both are declarations — `manifestFromConfigs` -copies `config.inputs` verbatim — so a prop **both sides declare and no renderer reads** -is, to this gate, perfect agreement. It was named `check:react-conformance` and opened by -claiming it confirmed the components "ACTUALLY implement" the spec props; it never could, -and #4413 shipped four dead blocks straight through a green run of it. Renamed and -re-scoped in #4472. The gate is still worth having (`spec-only`, `registry-only` and -`missing` are real signals) — just don't read it as proof anything renders. - -⚠️ **It is also the one gate `check:generated` cannot run at all**, and it says so in its -own bucket (`EXTERNAL_INPUT_REQUIRED`, "cannot run here") rather than beside the source -audits that are merely *deliberately* not run. Its right-hand side is objectui's -`sdui.manifest.json`, and nothing here can produce one: the registry is a browser app, so -the manifest exists only after `pnpm sdui:manifest` builds objectui at `.objectui-sha` and -enumerates it in a real browser — `packages/console/dist/` is gitignored, the console -build deliberately does not dump one, and the published `@objectstack/console` carries -none either. Until #4690 that combined with a manual run that printed `⚠ manifest -unavailable` and **exited 0**, so no path existed on which this gate could go red; it now -**exits 1** when it has no usable manifest, because "could not run" is a failure, not a -skip (Route & surface ownership §3, *Absence must be loud*). Run it the one way that -works: `pnpm sdui:manifest` (or `OBJECTUI_ROOT=../objectui pnpm objectui:build` first), -which dumps the manifest and runs the ratchet against it. Where the manifest comes from is -**settled** (#5960, maintainer ruling 2026-08-07): **not from CI**. It is an on-demand -gate whose trigger is the **objectui pin bump** — `.objectui-sha` is the only thing that -moves the manifest, so `scripts/bump-objectui.sh` and `scripts/build-console.sh` print the -`pnpm sdui:manifest` step and `docs/releases-maintenance.md` carries the procedure. -Producing it here was rejected outright: the sole producer drives Playwright chromium over -objectui's built console, so a CI-side dump means a full objectui build plus a browser -download on every matching PR. So: do not "fix" the red by re-adding a skip, and do not -"fix" it by wiring the gate into a workflow either — run it where it belongs, at the pin -bump. - -`check:exported-any` is the one of those that also reads the built `dist/*.d.ts`, so the -stale-`dist` caveat above applies to it too. It asks the other half of the -`api-surface/` question: that snapshot records an export *exists*, never what it -*resolves to*, which is how five exported symbols sat at `any` for a whole major with -every gate green (#4171). A recursive Zod schema needs an annotation to break its -circular inference, and `z.ZodType` compiles, validates correctly, and silently -throws the type away — annotate with the type instead (`QueryAST` in -`src/data/query.zod.ts` is the pattern). +Principles the wrapper encodes (its own output is the authority on detail): + +- **`--fix` is deliberately narrow.** Regenerating the whole set on principle rewrites + artifacts whose staleness you never saw, so a real semantic change lands silently + inside a mechanical diff. Let the check say which are stale, regenerate those. +- **No `check:` script regenerates anything** — a gate that regenerates edits your + working tree and reports nothing. Generation belongs to the **caller** (`pnpm build`, + or the `check:authorable-surface` gate whose `--check` mode writes only the + gitignored tree). Consequence: `check:docs` is not self-sufficient — run the `build` + line first; `build-docs.ts` refuses loudly on a missing or stale tree. +- **The gate → generator ledger self-reconciles against `package.json`** on every run + and on every PR (`--reconcile-only` in lint.yml's required typecheck job): a new + `check:`/`gen:` script nobody classified fails its own PR instead of quietly dropping + out of coverage. +- ⚠️ **`check:api-surface` and `check:exported-any` read the built `dist/*.d.ts`, not + `src/`.** A stale `dist` makes the first report *other people's* exports as + **removed** ("N breaking (removed/narrowed)") when nothing was removed at all — + rebuild before you believe it, and before you file a bug about `main` being red. +- **The pure source audits** (`check:liveness`, `check:empty-state`, + `check:skill-examples`, `check:exported-any`, `check:dual-source-exports`) have no + generator — a failure there is a real finding to fix, never an artifact to + regenerate; `check:generated` names them as deliberately not run, so its "all up to + date" never reads as "everything passed". `check:exported-any` exists because the + `api-surface/` snapshot records that an export *exists*, never what it *resolves to* + — a recursive Zod schema annotated `z.ZodType` compiles, validates, and silently + throws the type away; annotate with the real type (`QueryAST` in + `src/data/query.zod.ts` is the pattern). `check:dual-source-exports` asks whether a + name on two entries is one declaration re-exported (fine) or two declarations sharing + a name, with accepted cases in the shrink-only, hand-edited + `dual-source-exports.baseline.json`. + +⚠️ **`check:react-declaration-parity` compares two DECLARATIONS, not a declaration +against an implementation** — the props the spec zod schema declares vs the inputs the +objectui registry config declares. A prop both sides declare and no renderer reads is, +to this gate, perfect agreement (dead blocks have shipped through a green run under its +earlier, over-claiming name). Its `spec-only` / `registry-only` / `missing` signals are +real; just don't read it as proof anything renders. It is also the one gate +`check:generated` cannot run at all (`EXTERNAL_INPUT_REQUIRED`): its right-hand side is +objectui's `sdui.manifest.json`, produced only by `pnpm sdui:manifest` driving a real +browser over objectui built at `.objectui-sha`, and it **exits 1** with no usable +manifest — "could not run" is a failure, not a skip (Route & surface ownership §3). +Where the manifest comes from is **settled** (maintainer ruling 2026-08-07): **not from +CI** — it is an on-demand gate whose trigger is the **objectui pin bump** +(`docs/releases-maintenance.md` carries the procedure). Do not "fix" the red by +re-adding a skip, and do not wire the gate into a workflow either. Two generators have **no** gate at all — `gen:openapi` and `gen:sbom`. Nothing verifies -their output is current; the script reports that each run rather than staying silent -about it. +their output is current; the wrapper reports that each run rather than staying silent. --- @@ -769,8 +587,8 @@ export type Field = z.infer; ``` **Plugin** (the kernel contract is `init`/`start`/`destroy` — -`packages/core/src/types.ts`; the old `onInstall`/`onEnable`/`onDisable` -example described hooks nothing ever called, retired in #4212): +`packages/core/src/types.ts`; an older `onInstall`/`onEnable`/`onDisable` example +described hooks nothing ever called, and was retired for it): ```ts export class MyPlugin implements Plugin { name = 'plugin.my-feature'; @@ -784,251 +602,201 @@ export class MyPlugin implements Plugin { ## Route & surface ownership -Four rules, each paid for by a real bug. They matter more than usual here because -this repo is largely written by agents, and every one of them is a trap that -reads as reasonable code. - -**1. One route, one owner.** Never add a second implementation of a path that -another package already serves, however convenient. A shadowed duplicate is code -that `grep` finds and the runtime never runs — the exact input that makes an -agent (or a human) reason confidently from dead code. It also silently forks -every future invariant: the retired hono `/data` surface had to re-learn the -anonymous-deny gate (#2567), honest batch capability reporting (#3298) and -discovery accuracy (#4018), each after the fact, each because someone fixed the -real owner and never knew about the copy. Retired in #4073. - -**2. Explicit composition over default magic.** A capability that appears -because of a default nobody wrote down is invisible at every call site — and -call sites are the primary evidence an agent reasons from. #4073's own first -analysis checked *who passes the option* and missed *who relies on the default*; -the correction is in that issue's opening paragraph. If a host should get a -surface, it should mount it. - -**3. Absence must be loud.** A composition that legitimately serves nothing -should say so once at boot, naming the remedy — never leave a bare 404 to be -diagnosed. The same rule applies to tooling: a verifier that silently degrades -(reusing a stale build, skipping a check it could not run) is worse than no -verifier, because it reports success. Prefer failing to falling back. - -**4. Machine-readable surfaces must not lie.** `/discovery` and friends are read -by SDKs, codegen and AI clients. Advertise only what is actually mounted, and -mount everything advertised (ADR-0076 D12) — a wrong answer here propagates into -everything built on top of it. - -**Verifying any of this:** "who serves this path" is a question about the -composed, *provisioned* runtime — not about which plugin declares it, not about -registration order, and not about a minimal harness that merely boots. #4073 was -answered wrongly three times, once per each of those shortcuts. Boot the real -composition with its real services, or do not claim an answer. +Four rules, each paid for by a real bug. They matter more than usual here because this +repo is largely written by agents, and every one of them is a trap that reads as +reasonable code. + +**1. One route, one owner.** Never add a second implementation of a path that another +package already serves, however convenient. A shadowed duplicate is code that `grep` +finds and the runtime never runs — the exact input that makes an agent (or a human) +reason confidently from dead code. It also silently forks every future invariant: a +retired duplicate data surface had to re-learn the anonymous-deny gate, honest batch +capability reporting and discovery accuracy, each after the fact, each because someone +fixed the real owner and never knew about the copy. + +**2. Explicit composition over default magic.** A capability that appears because of a +default nobody wrote down is invisible at every call site — and call sites are the +primary evidence an agent reasons from (the classic misdiagnosis checks *who passes the +option* and misses *who relies on the default*). If a host should get a surface, it +should mount it. + +**3. Absence must be loud.** A composition that legitimately serves nothing should say +so once at boot, naming the remedy — never leave a bare 404 to be diagnosed. The same +rule applies to tooling: a verifier that silently degrades (reusing a stale build, +skipping a check it could not run) is worse than no verifier, because it reports +success. Prefer failing to falling back. + +**4. Machine-readable surfaces must not lie.** `/discovery` and friends are read by +SDKs, codegen and AI clients. Advertise only what is actually mounted, and mount +everything advertised (ADR-0076 D12) — a wrong answer here propagates into everything +built on top of it. + +**Verifying any of this:** "who serves this path" is a question about the composed, +*provisioned* runtime — not about which plugin declares it, not about registration +order, and not about a minimal harness that merely boots. The question has been +answered wrongly three times in one investigation, once per each of those shortcuts. +Boot the real composition with its real services, or do not claim an answer. --- ## Degradation log levels — `warn` vs `error` -Nearly every `catch` in this repo is a best-effort degradation, and nearly every -one of them logs `warn`. That default is wrong for a specific, recurring class, -and the cost of getting it wrong is not noise — it is silent data loss. Decide -the level with **one question**, not with an adjective: +Nearly every `catch` in this repo is a best-effort degradation, and nearly every one of +them logs `warn`. That default is wrong for a specific, recurring class, and the cost of +getting it wrong is not noise — it is silent data loss. Decide the level with **one +question**, not with an adjective: > **After the degradation, does the system still look "normal" from the outside, > while something it claims is persisted has not actually landed?** > **Yes → `error`. No → `warn`/`info` is right.** -- **Functional degradation → `warn` / `info`.** A screen is missing, a trigger is - not armed, a capability is not enabled, an optional service never showed up. - The system is *visibly* smaller than it should be, and the next person to use - the missing thing finds out. `ScheduleTriggerPlugin: job service not available - — scheduled flows will not run until one is registered` is exactly right at - `warn`. -- **Durability / data-consistency degradation → `error`.** A write that claims - to persist does not, DDL that was supposed to run did not, persisted state and - runtime state disagree. Nothing looks broken; the loss surfaces a release - later, to someone who cannot connect it to this line. - -**Why this is a rule and not a preference.** #4420: the durable suspended-run -store attached to a table that was never created, every write failed into a -`warn` nobody read, and every restart dropped all in-flight approvals — the -symptom surfaced a release after the cause. #4460 raised that one site to -`error`; #4632 made it the rule, because the *class* is what recurs. It is the -same failure Prime Directive #10 names — advertising a capability (here: -durability) the runtime does not deliver — and the same instinct as "Absence must -be loud" above: **prefer failing to falling back**, and when you must fall back, -say what was lost. +- **Functional degradation → `warn` / `info`.** A screen is missing, a trigger is not + armed, a capability is not enabled, an optional service never showed up. The system + is *visibly* smaller than it should be, and the next person to use the missing thing + finds out. `ScheduleTriggerPlugin: job service not available — scheduled flows will + not run until one is registered` is exactly right at `warn`. +- **Durability / data-consistency degradation → `error`.** A write that claims to + persist does not, DDL that was supposed to run did not, persisted state and runtime + state disagree. Nothing looks broken; the loss surfaces a release later, to someone + who cannot connect it to this line. + +**Why this is a rule and not a preference.** The founding incident: a durable +suspended-run store attached to a table that was never created, every write failed into +a `warn` nobody read, and every restart dropped all in-flight approvals — the symptom +surfaced a release after the cause. It is the same failure Prime Directive #10 names — +advertising a capability (here: durability) the runtime does not deliver — and the same +instinct as "Absence must be loud" above: **prefer failing to falling back**, and when +you must fall back, say what was lost. **An `error` here owes two things**, both, in the first line it prints -(`packages/services/service-automation/src/plugin.ts` `start()` is the reference -text): - -1. the **consequence**, concretely — *what* is not durable, and that the system - will keep looking healthy anyway; -2. the **fix** — the composition/config change that restores durability, or the - explicit opt-out that makes the degradation deliberate (`suspendedRunStore: - 'memory'`, `OS_SKIP_SCHEMA_SYNC`). - -Say it **once**, at the first degradation, not once per failed write. - -**Do not over-apply it.** Escalating a functional degradation to `error` is the -mirror-image failure: it trains everyone to skim `error`, which is what made the -#4420 `warn` unreadable in the first place. In particular, an `if (!service)` -composition branch is usually functional and usually belongs at `warn`; a `catch` -around a write, a DDL call, or a store initialization is where this rule bites. - -**A failure handed to the CALLER is not a degradation at all** — this is the -third legal answer, and forgetting it is how the mirror-image failure gets -written. Ask the judgment question honestly: a `/meta` PUT whose `catch` answers -`errorFromThrown(e, 400)`, or a batch whose contract IS a per-item outcome report -and whose `catch` writes the failure into it, does **not** look normal from the -outside — the requester was told, per item, that the write did not land. That is -already louder than a log line. Do **not** bolt a `logger.error` onto such a -site: the common case on a validation path is an author submitting an off-spec -body, so you would emit one durability `error` per rejected keystroke, which is -exactly what makes `error` unreadable. Declare **how it delivers** instead — -`FAILURE_PROPAGATION_CALLEES` (repo-wide names like `errorFromThrown`) or the -function-scoped `FAILURE_PROPAGATION_SITES` (local report sinks, whose names mean -nothing repo-wide) in the checker, which then proves structurally that *every* -path out of the `catch` delivers. #5241 added it after #4754 was forced to park -three correct sites in the baseline for want of a way to say this. - -**It has teeth** (a rule this repo only writes down is the very "declared ≠ -enforced" shape it keeps paying to fix): `pnpm check:durability-log-level` walks -the AST for `catch` blocks guarding a declared vocabulary of durability-critical -operations and fails when one logs below `error` without rethrowing. It is -deliberately narrow — it cannot *discover* a new durability seam, only stop the -known ones from regressing. Found a new one? Add it to -`DURABILITY_CRITICAL_CALLEES` in `scripts/check-durability-degradation-log-level.mjs` -in the same PR that fixes it. Accepted exceptions live in -`scripts/durability-degradation.baseline.json`, hand-edited with a reason and -shrink-only — and that file is currently **empty**, which is its intended steady -state: an entry there means a real degradation nobody has fixed yet, never a site -the gate merely cannot classify. If a red seam turns out to hand its failure to -the caller, declare the propagation vocabulary above; do not baseline correct -code. - -**The same command has a SECOND set of teeth** — the read-seam invention rule -(#5186; family #4728 → #4825 → #5108 → #6116). Everything above grades *how loud -the catch is*, which is the right axis for a write or a DDL seam and structurally -blind to a read: `catch { return []; }` has no log to grade at all, and a read's -callees are `find` / `findOne` / `count`, names far too generic to declare -repo-wide. So the second rule asks a different question — **the read did not -happen; did you make an answer up anyway, and tell nobody?** — and goes red only -when every part holds: the `try` performs a storage read (the `IDataDriver` read -methods, or a same-file wrapper over one), the `catch` logs *nothing* at any -level (same-file helpers followed), some path out of it returns an **invented -answer** — an empty/zero value for that method (`[]`, `null`, `false`, `0`, `1`, -…), or one of the enclosing function's own parameters handed straight back -(#6451, from #6116: for an enrichment function the un-enriched input is the very -same bytes a successful read with nothing to hydrate returns) — and that path was -never reached by discriminating the error's **type**. What it protects is -DISTINGUISHABILITY, not the spelling of the returned value: the fix is to ask the -error's type, or to report the failure once — never to invent a different empty. - -Its scan surface is deliberately narrower than the log-level rule's: -`packages/metadata/src`, `packages/metadata-protocol/src` and -`packages/objectql/src` only. That is the maintainer's 2026-08-06 ruling — -「裁 3 —— 收窄先行」: prove the false-positive surface on the metadata/persistence -layer first, and evaluate widening as its own issue. It is also the only honest -way to afford `find`/`findOne`/`count` as a vocabulary at all — the names are -generic, so the SCOPE is what makes them mean "a storage seam" rather than "any -data read anywhere". Like its sibling it is a ratchet, not a proof: it cannot see -a read outside the `try`, nor an empty answer wrapped in a result envelope. - -**Found a new read seam?** The symmetric answer to `DURABILITY_CRITICAL_CALLEES` -above is `READ_FAILURE_DISCRIMINATORS`, in the same script: the declared -predicates that prove a read failure is benign (today `isMissingTableError`, -`packages/metadata/src/errors.ts`), which is why a hand-rolled `if (e.code === -'42P01')` is flagged on purpose — ask the shared predicate instead of growing a -second vocabulary of "which driver errors are benign" (#5841). A reviewed, -genuinely legitimate seam goes in this rule's OWN ledger, -`scripts/durability-read-invention.baseline.json` — **not** the empty -`durability-degradation.baseline.json` named above. The two rules share this -file, one CI step and one AST pass, and share no vocabulary, no baseline and no -verdict: a seam red under one is untouched by the other. That ledger is -shrink-only, hand-edited and fails on a stale entry exactly like its sibling, but -its steady state is *not* "empty": an entry is either a real unfixed instance -(`unfixed-degradation`, carrying the issue that tracks it) or a -`reviewed-legitimate` seam whose empty value is a declared "could not determine" -that a syntactic rule cannot see. Read the entries and the reasons they give — -never the count. +(`packages/services/service-automation/src/plugin.ts` `start()` is the reference text): +① the **consequence**, concretely — *what* is not durable, and that the system will +keep looking healthy anyway; ② the **fix** — the composition/config change that +restores durability, or the explicit opt-out that makes the degradation deliberate +(`suspendedRunStore: 'memory'`, `OS_SKIP_SCHEMA_SYNC`). Say it **once**, at the first +degradation, not once per failed write. + +**Do not over-apply it.** Escalating a functional degradation to `error` trains +everyone to skim `error` — which is what made the founding incident's `warn` unreadable +in the first place. An `if (!service)` composition branch is usually functional and +belongs at `warn`; a `catch` around a write, a DDL call, or a store initialization is +where this rule bites. **And a failure handed to the CALLER is not a degradation at +all** — the third legal answer: a `catch` that answers `errorFromThrown(e, 400)`, or a +batch whose contract IS a per-item outcome report, does not look normal from the +outside — the requester was told. Do **not** bolt a `logger.error` onto such a site +(on a validation path that emits one durability `error` per rejected keystroke); +declare **how it delivers** instead — `FAILURE_PROPAGATION_CALLEES` (repo-wide names) +or the function-scoped `FAILURE_PROPAGATION_SITES` in the checker, which then proves +structurally that *every* path out of the `catch` delivers. + +**It has teeth**: `pnpm check:durability-log-level` +(`scripts/check-durability-degradation-log-level.mjs`; its header is the authority) +walks the AST for `catch` blocks guarding a declared vocabulary of durability-critical +operations and fails when one logs below `error` without rethrowing. It is deliberately +narrow — it cannot *discover* a new seam, only stop known ones from regressing; found a +new one, add it to `DURABILITY_CRITICAL_CALLEES` in the same PR that fixes it. Its +baseline (`scripts/durability-degradation.baseline.json`) is shrink-only and currently +**empty — its intended steady state**: an entry means a real unfixed degradation, never +a site the gate cannot classify; a caller-propagating seam is declared via the +propagation vocabulary, not baselined. + +**The same command has a SECOND set of teeth — the read-seam invention rule.** The +log-level axis is structurally blind to reads: `catch { return []; }` has no log to +grade. The second rule asks — **the read did not happen; did you make an answer up +anyway, and tell nobody?** — and goes red only when every part holds: the `try` +performs a storage read (`IDataDriver` read methods or a same-file wrapper), the +`catch` logs nothing at any level, some path out of it returns an **invented answer** +(an empty/zero value, or an enclosing function's own parameter handed straight back — +for an enrichment function the un-enriched input is byte-identical to a successful +read with nothing to hydrate), and that path was never reached by discriminating the +error's **type**. What it protects is DISTINGUISHABILITY, not the spelling of the +returned value: fix by asking the error's type or reporting the failure once — never +by inventing a different empty. Its scan surface is deliberately narrow — +`packages/metadata/src`, `packages/metadata-protocol/src`, `packages/objectql/src` +only (maintainer ruling 2026-08-06, 「裁 3 —— 收窄先行」: prove the false-positive +surface on the persistence layer first; widening is its own issue) — which is also +what lets names as generic as `find`/`findOne`/`count` mean "a storage seam" at all. +Benign read failures are proven by the declared `READ_FAILURE_DISCRIMINATORS` +predicates (today `isMissingTableError`) — a hand-rolled `if (e.code === '42P01')` is +flagged on purpose; ask the shared predicate rather than growing a second vocabulary. +Reviewed-legitimate seams go in this rule's OWN shrink-only ledger, +`scripts/durability-read-invention.baseline.json` (**not** the empty one above); its +steady state is *not* empty — read the entries and their reasons, never the count. +The two rules share one script, one CI step and one AST pass, and share no vocabulary, +no baseline and no verdict. --- ## Startup registry reads — never record a verdict the boot can still contradict -A boot fills its registries incrementally. Asking a registry "is X there?" while -it is still filling is fine — the answer is simply not final yet. Turning that -not-yet into a **verdict and recording the verdict** is the defect, because the -provider registers a moment later and nothing goes back to undo the record. +A boot fills its registries incrementally. Asking a registry "is X there?" while it is +still filling is fine — the answer is simply not final yet. Turning that not-yet into a +**verdict and recording the verdict** is the defect, because the provider registers a +moment later and nothing goes back to undo the record. Decide with **one question**, the counterpart of the degradation-log-level one: -> **At the moment this code concludes "X is not registered", can a provider -> still register X during this same boot? And is that conclusion RECORDED -> anywhere that outlives the moment?** +> **At the moment this code concludes "X is not registered", can a provider still +> register X during this same boot? And is that conclusion RECORDED anywhere that +> outlives the moment?** > **Yes and yes → defect.** Three parts, all three or it is not a finding: -1. a read of a registry that is still filling — the service registry during - `init()`, or a plugin-extensible capability registry before it is sealed; +1. a read of a registry that is still filling — the service registry during `init()`, + or a plugin-extensible capability registry before it is sealed; 2. a terminal conclusion drawn from "absent"; 3. that conclusion **recorded** — cached in an instance field or module binding, asserted in a `warn`, or persisted. -Part 3 is what makes this a rule and not noise. **A read-only probe is -completely legal**: `AutomationEngine.getUnknownNodeTypeAudit()` reads the -executor registry on every call, records nothing, and is correct. - -**Why this is a rule and not a preference.** One showcase cold start on -2026-08-03 produced three instances, in three unrelated subsystems, written by -three people at three times: plugin-auth froze an `undefined` cache handle into -its config for the life of the process, so rate-limit counters never reached the -shared store and the printed warning sent operators to provision Redis for a -problem they did not have (#4772); service-automation asserted that eight -approval flows "will fail at execution time" 0.8s before the executor that runs -them was registered, and a deployment that genuinely lacked the plugin emitted -the identical eight, so the signal could not tell the two apart (#4771); objectql -wrote an ADR-0104 attestation into `sys_migration` during the same boot that was -still seeding rows contradicting it, so the next restart rejected its -predecessor's data (#4769). Whether the kernel contract itself should be -tightened further is #4776. +Part 3 is what makes this a rule and not noise. **A read-only probe is completely +legal**: `AutomationEngine.getUnknownNodeTypeAudit()` reads the executor registry on +every call, records nothing, and is correct. + +**Why this is a rule and not a preference.** One showcase cold start produced three +instances in three unrelated subsystems, written by three people at three times: an +auth plugin froze an `undefined` cache handle into its config for the life of the +process (the printed warning sent operators to provision Redis for a problem they did +not have); the automation service asserted eight flows "will fail at execution time" +0.8s before their executor registered — indistinguishable from a deployment that +genuinely lacked the plugin; and the query engine persisted a schema attestation the +same boot was still contradicting, so the next restart rejected its predecessor's +data. **The three cures, in preference order:** 1. **Resolve where it is used, not where you start.** A lazy accessor or a `kernel:ready` hook sees a provider that registered later — `createLazyCacheRateLimitStorage()` in plugin-auth is the reference. -2. **Declare the ordering (ADR-0116).** `dependencies` / `optionalDependencies` - / `requiresServices` make the kernel hoist the provider ahead or assert it - registered, which makes "absent" a *fact*. Tolerance belongs in the plugin's - own declaration, where the kernel enforces it — not in a checker's ledger. +2. **Declare the ordering (ADR-0116).** `dependencies` / `optionalDependencies` / + `requiresServices` make the kernel hoist the provider ahead or assert it registered, + which makes "absent" a *fact*. Tolerance belongs in the plugin's own declaration, + where the kernel enforces it — not in a checker's ledger. 3. **Seal the vocabulary, then judge.** For a registry that is open by contract - (ADR-0018 flow node types), the host declares the moment it can no longer - grow — `AutomationEngine.sealNodeTypeVocabulary()`, called at - `kernel:bootstrapped` — and only then is an absence worth reporting. + (ADR-0018 flow node types), the host declares the moment it can no longer grow — + `AutomationEngine.sealNodeTypeVocabulary()`, called at `kernel:bootstrapped` — and + only then is an absence worth reporting. **It has teeth**: `pnpm check:startup-registry-verdict` walks the AST for that three-part shape and fails on it; accepted exceptions live in the shrink-only, -hand-edited `scripts/startup-registry-verdict.baseline.json`. Like -`check:durability-log-level` it is deliberately narrow — it cannot *discover* a -new seam, only stop known ones from regressing, and it under-matches on purpose -rather than risk a false positive: `getService('cache')` is visible, a -`resolveCacheOrFallback()` three layers down another package is not, and #4769's -"registry" is a database table it can never see. Found a new open registry? Add -it to `OPEN_CAPABILITY_REGISTRIES` in the same PR that fixes it. +hand-edited `scripts/startup-registry-verdict.baseline.json`. Like its durability +sibling it is deliberately narrow and under-matches on purpose rather than risk a false +positive — it cannot discover a new seam, only stop known ones from regressing. Found a +new open registry? Add it to `OPEN_CAPABILITY_REGISTRIES` in the same PR that fixes it. --- ## Post-Task Checklist 1. `pnpm test` — verify nothing broke. Touched a type-check-covered package? `pnpm typecheck` too. -2. **Land it — don't leave passing work in the working tree.** Once tests pass, - create a feature branch, commit, push, open a PR, and — once remote CI is - fully green and the PR is accepted — arm auto-merge so the queue lands it - (Multi-agent discipline §7: never straight to `main`; never arm a PR that - isn't green yet). A finished task = a merged PR, not a dirty working tree. - ⛔ **Except a diff touching `docs/adr/**`**: push it, open the PR, and stop - there — landing it is the maintainer's, by hand (Prime Directive #14). For - that one class, a finished task = a PR left visibly awaiting a human merge. +2. **Land it — don't leave passing work in the working tree.** Once tests pass, create + a feature branch, commit, push, open a PR, and — once remote CI is fully green and + the PR is accepted — arm auto-merge so the queue lands it (Multi-agent discipline + §7: never straight to `main`; never arm a PR that isn't green yet). A finished task + = a merged PR, not a dirty working tree. ⛔ **Except a diff touching `docs/adr/**`**: + push it, open the PR, and stop there — landing it is the maintainer's, by hand + (Prime Directive #14). For that one class, a finished task = a PR left visibly + awaiting a human merge. 3. **Add a changeset for feature work.** When the change is a feature or functional improvement, run `pnpm changeset` (or add a `.changeset/*.md` entry) describing it before committing. Pure bug fixes do **not** require a changeset. **Breaking changesets must carry their migration.** If the change removes or renames anything an author can write (a spec key, an export, a config field), the changeset body must state the FROM → TO mapping and the one-line fix — this text ships to consumers as `CHANGELOG.md` inside the npm package and is what an upgrading agent greps after the tombstone error. Removing an authorable spec key also requires a tombstone so the rejection itself carries the prescription — `retiredKey()` (`packages/spec/src/shared/retired-key.ts`) on a non-strict schema, or an entry in the relevant `UNKNOWN_KEY_GUIDANCE` / `*_RETIRED_KEY_GUIDANCE` map (see `object.zod.ts`, `ai/tool.zod.ts`) when the schema is `.strict()`. The changeset is one of fourteen surfaces a retirement touches — follow the `spec-property-retirement` skill (`.claude/skills/`) rather than reconstructing the kit, and note the two routes imply **opposite** liveness-ledger dispositions. **A breaking changeset must also state its ADR-0087 disposition, in writing.** Add exactly one marker to the changeset body — `pnpm check:adr-0087-registration` enforces it, and the CI step is *Require an ADR-0087 disposition on a declared-breaking changeset*: @@ -1038,8 +806,8 @@ it to `OPEN_CAPABILITY_REGISTRIES` in the same PR that fixes it. ``` - Why it is asked of you at all: the two ADR-0087 gates (`check:spec-changes`, `check:upgrade-guide`) pin ledger ↔ **artifact synchrony**, and the artifacts are a pure projection of the registry — so a retirement whose entry was **never written** leaves the two perfectly consistent and every gate in the repo green. PR #6048 removed `ctx.user.roles` that way and only a human comparing by eye caught it (#6011, backfilled by PR #6138). Ledger entries are the sole data source for `objectstack migrate meta`, `spec-changes.json` and the generated upgrade guide, and for a surface with **no spec schema** (`ctx.user` is only a runtime TS interface) there is no tombstone and no schema rejection either — the ledger entry is the *only* channel that reaches an upgrader. Measured: roughly **1 declared-breaking change in 7** needs an entry, so `not-required` is the ordinary answer and costs one line. Three of the four dispositions are re-verified mechanically on every run, and the fourth is refused when the changeset's own body carries a FROM → TO prescription — a changeset that ships migration instructions cannot also claim nobody must migrate (#6148). -4. **Added or removed a `packages/spec` export? Run `pnpm --filter @objectstack/spec gen:api-surface` and commit the result.** The `TypeScript Type Check` job diffs spec's built export surface against `api-surface/` (one shard per entry point since #5837); a new export makes the snapshot stale and turns the job red. It reads the **built `dist` declarations**, so `OS_SKIP_DTS=1` — the flag you reach for to make local builds fast — skips exactly the artifact the gate inspects, and the check passes locally while failing in CI. Same shape for the other generated-artifact gates in that job (`check:docs`, `check:skill-refs`, `check:react-blocks`), which read `src/` and so do reproduce locally. + Why it is asked of you at all: the ADR-0087 gates pin ledger ↔ **artifact synchrony**, and the artifacts are a pure projection of the registry — a retirement whose entry was **never written** leaves everything perfectly consistent and every gate green (a removal has shipped that way, caught only by a human comparing by eye). Ledger entries are the sole channel that reaches an upgrader (`objectstack migrate meta`, `spec-changes.json`, the upgrade guide) — for a surface with no spec schema there is no tombstone or schema rejection either. Roughly 1 declared-breaking change in 7 needs an entry, so `not-required` is the ordinary answer and costs one line; the markers are re-verified mechanically, and `no-migration-prescription` is refused when the changeset's own body carries a FROM → TO prescription. +4. **Added or removed a `packages/spec` export? Run `pnpm --filter @objectstack/spec gen:api-surface` and commit the result.** The `TypeScript Type Check` job diffs spec's built export surface against `api-surface/` (one shard per entry point); a new export makes the snapshot stale and turns the job red. It reads the **built `dist` declarations**, so `OS_SKIP_DTS=1` — the flag you reach for to make local builds fast — skips exactly the artifact the gate inspects, and the check passes locally while failing in CI. Same shape for the other generated-artifact gates in that job (`check:docs`, `check:skill-refs`, `check:react-blocks`), which read `src/` and so do reproduce locally. 5. Update `CHANGELOG.md` / `ROADMAP.md` if user-facing or architectural. 6. **Delete temporary artifacts** — screenshots, traces, scratch logs, `.playwright-mcp/`, throwaway `tmp*.ts`, ad-hoc scripts. Repo must look identical to before, minus intended changes. diff --git a/CLAUDE.md b/CLAUDE.md index 01d31288b0..de9ca6443d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,7 +19,7 @@ All agents share one GitHub identity, so the assignee field can't tell you wheth is **yours** — a claim is assign **plus a claim comment with your session ID and branch** (`claude/issue--`), and before writing code you must re-read the comments: an earlier claim with a different session ID means it's taken, whatever the assignee says. -(#4551 was implemented twice in one morning because this read was skipped — see #4588.) +(Skipping that read is how one issue got implemented twice in one morning.) ## ⛔ Worktree-first — before your FIRST file edit (AGENTS.md Prime Directive #11) @@ -52,8 +52,8 @@ stack — your `pop` restores whatever the other agent pushed a moment earlier, own changes stay on the stack for them to take. `pop` reports **success**; the only symptom is someone else's files appearing in your `git status`, and a following `git add -A` merges their work into your PR. Not hypothetical: it happened between two -parallel agents mid reverse-verification (objectui#3430) and cost both of them their -in-flight changes, recoverable only as unreachable commits. +parallel agents mid reverse-verification and cost both of them their in-flight +changes, recoverable only as unreachable commits. Use one of these instead — no shared state, all inside your own worktree: diff --git a/scripts/pm/check-skill-id-lint.mjs b/scripts/pm/check-skill-id-lint.mjs index bbcc69a92f..930bf46a88 100644 --- a/scripts/pm/check-skill-id-lint.mjs +++ b/scripts/pm/check-skill-id-lint.mjs @@ -20,22 +20,24 @@ * ## What it scans * * Every .md file under .claude/skills/pm-dispatch/ (the PM operating protocol, - * including its references/) plus .claude/agents/os-dev.md (the dev-side twin). - * The pattern is /#[0-9]{3,}/ — real issue/PR numbers in this repo are 3+ digits, - * while placeholders (`#`, `#N`, `epic:#`) and small literals (`#12 #34` - * in the argument-syntax example) stay legal. + * including its references/), plus .claude/agents/os-dev.md (the dev-side twin) + * and AGENTS.md (the repo-wide agent instruction file, rewritten to the same + * principles-only standard). The pattern is /#[0-9]{3,}/ — real issue/PR numbers + * in this repo are 3+ digits, while placeholders (`#`, `#N`, `epic:#`), + * Prime-Directive cross-references (`#14`) and small literals (`#12 #34` in the + * argument-syntax example) stay legal. * - * ## The one legacy waiver, and how it expires by itself + * ## The legacy-waiver mechanism (LEGACY_EXACT), currently unused * - * .claude/agents/os-dev.md is rewritten to the same standard in its OWN PR (the - * two PRs are deliberately independent — no cross-PR ordering). Until that - * rewrite lands, this gate would be red on either merge order. So os-dev.md - * carries an EXACT legacy count: the file passes only when its match count is - * exactly the recorded pre-rewrite value (untouched legacy) or exactly 0 - * (rewritten). Any other count is red — you cannot add an ID, and a partial - * cleanup must finish the job. Once the rewrite lands the entry is dead weight - * with zero tolerance (0 matches required); deleting the entry then is a - * one-line cleanup, not a prerequisite. + * A file rewritten to this standard in a DIFFERENT PR than the one that adds it + * to the scan set needs merge-order independence: an EXACT legacy count lets the + * file pass only at its recorded pre-rewrite value (untouched legacy) or at 0 + * (rewritten) — any other count is red, so nobody can add an ID and a partial + * cleanup must finish the job. os-dev.md used this for its own rewrite; its + * entry became dead weight once that landed (worse: it would have re-admitted + * exactly 81 IDs) and was deleted. AGENTS.md needs no entry — its rewrite and + * its scan-set addition travel in the same PR. The mechanism stays for the next + * split rewrite; the self-test exercises it with a synthetic entry. * * Missing file or empty read is RED, never a pass — a gate that cannot find its * input must fail, not skip. @@ -49,13 +51,11 @@ import process from 'node:process'; const REPO_ROOT = join(fileURLToPath(import.meta.url), '..', '..', '..'); export const SCAN_ROOT = '.claude/skills/pm-dispatch'; -export const EXTRA_FILES = ['.claude/agents/os-dev.md']; +export const EXTRA_FILES = ['.claude/agents/os-dev.md', 'AGENTS.md']; export const ID_PATTERN = /#[0-9]{3,}/g; // Exact-count legacy waiver (see header): pass iff count === legacy || count === 0. -export const LEGACY_EXACT = new Map([ - ['.claude/agents/os-dev.md', 81], -]); +export const LEGACY_EXACT = new Map([]); function mdFilesUnder(dir) { const out = []; @@ -142,13 +142,17 @@ function run() { } function selfTest() { + // The waiver mechanism is tested with a synthetic entry so it stays covered + // while LEGACY_EXACT is empty (see header). + LEGACY_EXACT.set('synthetic-legacy.md', 81); const cases = [ ['clean file -> green', verdictFor('x.md', 0).ok, true], ['one ID -> red', verdictFor('x.md', 1).ok, false], - ['legacy file at exact count -> green', verdictFor('.claude/agents/os-dev.md', 81).ok, true], - ['legacy file at zero -> green', verdictFor('.claude/agents/os-dev.md', 0).ok, true], - ['legacy file grew -> red', verdictFor('.claude/agents/os-dev.md', 82).ok, false], - ['legacy file partially cleaned -> red', verdictFor('.claude/agents/os-dev.md', 40).ok, false], + ['un-waived scanned files carry no waiver', LEGACY_EXACT.has('.claude/agents/os-dev.md') || LEGACY_EXACT.has('AGENTS.md'), false], + ['legacy file at exact count -> green', verdictFor('synthetic-legacy.md', 81).ok, true], + ['legacy file at zero -> green', verdictFor('synthetic-legacy.md', 0).ok, true], + ['legacy file grew -> red', verdictFor('synthetic-legacy.md', 82).ok, false], + ['legacy file partially cleaned -> red', verdictFor('synthetic-legacy.md', 40).ok, false], ['red message names the standard', verdictFor('x.md', 2).msg.includes('self-contained'), true], ['placeholders stay legal', ('# #N epic:# #12 #34'.match(ID_PATTERN) ?? []).length, 0], ['real IDs match', ('see #4650 and #12345'.match(ID_PATTERN) ?? []).length, 2], @@ -159,6 +163,7 @@ function selfTest() { if (!ok) failed++; console.log(` ${ok ? '✓' : '✗'} ${name}`); } + LEGACY_EXACT.delete('synthetic-legacy.md'); if (failed) { console.error(`✗ check-skill-id-lint self-test: ${failed} of ${cases.length} case(s) failed.`); process.exit(1);