From ad501746172c7e423c47d2fd8228075a9f8d2bb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 23:34:03 +0000 Subject: [PATCH] =?UTF-8?q?docs(agents):=20ADR=20=E5=90=88=E5=B9=B6?= =?UTF-8?q?=E7=A6=81=E4=BB=A4=E5=86=99=E5=85=A5=20AGENTS.md=20=E2=80=94?= =?UTF-8?q?=E2=80=94=20PD=20#14=20+=20=E4=B8=A4=E5=A4=84=E5=8A=A8=E6=89=8B?= =?UTF-8?q?=E5=89=8D=E7=9A=84=E5=88=86=E5=8F=89=20(#6801)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #6741 要求 1 是 AGENTS.md 里的一条款,此前从未落笔。补上: - **PD #14**(权威落点,紧邻管 ADR 权威的 PD #13):维护者裁决原文引用、 未翻译;起草/推分支/开 PR 对所有座位照旧开放,被保留的只有**落地** (merge / 入队 / arm auto-merge);「已复核 + 已批准 + 全绿」不构成例外; 撤回机制(只有转 draft 才真的踢出队列,`disable_pr_auto_merge` 单独调用 不解除成员资格)及其实测上限(#6732 在 draft 状态下仍被合并);交叉引用 #6785(机器门禁)。 - **§7 合并队列条**:在「arm auto-merge」那句话之后立刻分叉 —— 手在那里。 - **Post-Task Checklist 第 2 条**:同一分叉,dev 收尾时手在那里。 ⛔ 未内联进 CLAUDE.md —— 理由写在 PR 正文里(该文件的自述判据是「遗漏会 毁掉其他 agent 的工作」,本条的受害面不是这个,且它的执行层是 #6785)。 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F8q5J1MQyocgtNspb15fSn --- AGENTS.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index fcf5af81dd..bf34f9cc26 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -139,6 +139,15 @@ Other scripts: `objectui:bump` (pull only), `objectui:build`, `objectui:clean`. 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.json` (`pnpm check:adr-anchors`) 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: + + > **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. --- @@ -280,6 +289,12 @@ Even inside your own worktree, operate defensively: 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. + ⛔ **One class of PR never enters this path, however green: a diff that + touches `docs/adr/**`.** Do not merge it, do not queue it, do not arm it — + read the PR's file list (`get_files`) before you arm anything, and see + **Prime Directive #14** for the ruling, 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 @@ -973,6 +988,9 @@ it to `OPEN_CAPABILITY_REGISTRIES` in the same PR that fixes it. 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*: