docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

docs(spec): state minApprovals' real per-behavior default in the schema prose - #14543

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe
Sep 2, 2026
Merged

docs(spec): state minApprovals' real per-behavior default in the schema prose#14543
os-zhuang merged 4 commits into
mainfrom
claude/issue-13809-min-approvals-default-describe

Conversation

@claude

@claudeclaudeBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes#13809

ApprovalNodeConfigSchema.minApprovals described itself as "Default 1". The
approval runtime has never read it that way: an omitted threshold falls back to
the resolvable approver count under quorum, so a quorum node authored without
the key requires every approver, not one. Under per_group the fallback
really is 1 per group. This converges the declared text onto the enforced
behaviour — the prose is the half that drifted, and only the prose moves.

Premise, re-checked on origin/main

halfwherewhat it says
declaredpackages/spec/src/automation/approval.zod.ts:714 (JSDoc) and :718 (.describe(...)), at merge base ecbb6fd9b"Defaults to 1." / "Default 1"
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2395Math.min(Math.max(1, config.minApprovals ?? n), n) under quorumn is the resolved approver count, so omitted means ALL
enforcedpackages/plugins/plugin-approvals/src/approval-service.ts:2400Math.max(1, config.minApprovals ?? 1) under per_group — omitted means 1 per group

That runtime file's own doc comment already said it, at :2378: "quorum — at
least minApprovals distinct approvals (default = all)"
. The ?? n is
deliberate and is not touched here — relaxing it (a schema .default(1), or
a runtime change) would lower the approval threshold of every stored quorum flow
and is a maintainer decision, not this PR's.

The new text

Approvals required — total (quorum) or per group (per_group).
Omitted ⇒ all resolvable approvers for quorum, 1 per group for per_group

The JSDoc above the property carries the same statement in long form. No tracker
ids in either (doc-authoring rule 3).

Every copy of the old string, regenerated

  • packages/spec/src/automation/approval.zod.ts — the .describe() and the
    JSDoc above it (hand-edited; this is the source).
  • content/docs/references/automation/approval.mdx:72 — the ApprovalNodeConfig
    properties table, regenerated with pnpm --filter @objectstack/spec gen:docs,
    never by hand. One cell changed.
  • Nothing else carried it. authorable-surface/, authorable-surface.base.json
    and json-schema.manifest/ are key-level and hold no describe prose;
    check:generated reports all 15 artifacts current on the final head.
  • skills/objectstack-automation/SKILL.md already states the per-behaviour
    default in its own words and is hand-written, so it needed no change — this PR
    touches no governed surface.

Pin test

packages/spec/src/automation/approval.test.ts gains one case asserting (a) the
schema still injects no default — omitting the key leaves minApprovals
undefined under both behaviours — and (b) the description names both
behaviours' defaults and no longer claims "Default 1". Reverse-verified: with the
old string restored on disk (mutation confirmed by marker counts and a changed
blob hash), the case fails on exactly that claim, 1 failed / 44 passed; restoring
reproduces the HEAD blob byte-for-byte.

One extra file, named on purpose

Stating the omitted-threshold contract out loud put a permissive-shaped sentence
in front of check:empty-state, which reds on any unclassified "omitted = all".
The gate is right to demand a decision, so the decision is recorded rather than
worded around: packages/spec/scripts/liveness/empty-state-registry.mts gains a
minApprovals entry classified closed — the omitted threshold lands on the
strictest reading (every resolvable approver), so careless authoring lands on
least privilege — citing the runtime enforcement site as evidence. No behaviour,
no accept set and no runtime file changes with it.

Clause-②: no — describe-only, accept set and behaviour unchanged.

Sibling work: the timeoutHours describe on this same file (issue #13801, phase
2) is untouched, and origin/main had moved neither approval.zod.ts nor any
regenerated product this branch writes at the last pre-push fetch.

Verification union — run on head f1aceed03

Derived mechanically with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths; it reads the change set itself), plus the
dispatch's must-haves. Exit codes captured before any pipe.

69 of 75 green · 5 NOT MEASURED · 1 declared narrowing. The 75 are the 72
commands the deriver printed for this change set plus three must-haves it does
not carry (check:nul-bytes, pnpm --filter @objectstack/spec typecheck, the
spec suite).

Named results:

  • check:generatedall 15 artifacts current, including check:docs,
    check:authorable-surface, check:api-surface, check:strictness-ledger,
    check:liveness and check:test-typecheck.
  • check:doc-authoring · check:nul-bytes · check:empty-state ·
    check:role-word · check:type-check-coverage ·
    check:cross-package-test-inputs · check:pm-governed-merges — green.
  • pnpm --filter @objectstack/spec typecheck (tsc + scripts tsconfig + test
    tsconfig) — green.
  • Tests, green: approval.test.ts45 passed, and with the liveness gate
    suites that cover the registry module this PR edits
    (empty-state.test.ts, evidence.test.ts, check-liveness.test.ts)
    163 passed across 4 files.

Declared narrowing: the whole@objectstack/spec vitest suite is not in
this union. Two attempts acquired the shared verify lock and were killed by the
container's ~10-minute foreground ceiling mid-run, under heavy contention from
sibling agents; the run boots a dev server and does not fit the remaining window.
Narrowed to the four files above — the one this PR changes plus every test that
loads the module it adds a row to. CI runs the full suite on this head.

NOT MEASURED (never read as green, never as red) — every one is a build
precondition of this worktree, which builds only the @objectstack/spec closure
while CI checks out fresh and builds the workspace:
check:dev-prereqs, check:dual-build-cjs-loads (exit 3),
check:type-check-debt (exit 3 — re-measure needs 51 built dependency closures),
check:skill-examples (needs client-react.d.ts, a 34-package closure) and
check:test-completeness (exit 3 — it grades a saved turbo run test log only
CI produces).

Which tree each command measured. All 75, on the final head f1aceed03,
with the working tree clean. Commands first run on an earlier commit — the
descriptive scans, and the spec audits that had been blinded by a stale
packages/spec/dist — were re-run there after the last commit, so no line above
is a reading about a tree nobody is on.


🤖 Generated with Claude Code

https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21


Generated by Claude Code

`ApprovalNodeConfigSchema.minApprovals` described itself as "Default 1",
but an omitted threshold has never meant 1 under `quorum`: the runtime
falls back to the resolvable approver count, so a quorum node authored
without the key requires EVERY approver, not one. Under `per_group` the
fallback really is 1 per group.
Converge the declared text onto the enforced behaviour — the prose is the
half that drifted, so only the prose moves; no schema default is added and
no runtime threshold changes. A pin test asserts the description names both
behaviours' defaults and that the schema still injects no default, so the
two readings cannot drift apart silently again.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
…ate registry
Making the omitted-threshold contract explicit put a permissive-shaped
sentence in front of the empty-state scanner ("Omitted ⇒ all resolvable
approvers …"). The gate is right to demand a decision, and the decision is
`closed`: an omitted threshold lands on the STRICTEST reading — every
resolvable approver under `quorum` — so careless authoring lands on least
privilege, not on the widest grant. Registered with the runtime enforcement
site as evidence rather than reworded around the scanner.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511packageMentionDocs.

Which tree this was computed on

This run read content/docs from 12dd5daba633b922291ec48a35871548a02b3e54 — the merge of head f1aceed03d2fb789fcb7d753f3ce18913571cbbc into base 9acddde94c5242db560913aa2d2c2e8f7b843511, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 12dd5daba633b922291ec48a35871548a02b3e54 && git checkout 12dd5daba633b922291ec48a35871548a02b3e54
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9acddde94c5242db560913aa2d2c2e8f7b843511 f1aceed03d2fb789fcb7d753f3ce18913571cbbc && git checkout -B drift-repro 9acddde94c5242db560913aa2d2c2e8f7b843511 && git merge --no-ff f1aceed03d2fb789fcb7d753f3ce18913571cbbc
node scripts/docs-audit/affected-docs.mjs --json 9acddde94c5242db560913aa2d2c2e8f7b843511

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33624604584 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test: FAIL unit test/vitest-tiers-partition.test.ts > the two tiers of packages/cli (#13504) > INTEGRATION_FILES equals the behavioural predicate over every file on disk
    ↳ 失败原因: @objectstack/cli:test: AssertionError: files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them): expected [ Array(1) ] to deeply equal []
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Sep 2, 2026

Copy link
Copy Markdown
ContributorAuthor

Queue build 33624604584 red — not this PR's; no change needed here. (domain:spec seat, session session_01GDA48PuRFrHyRfdkBz8m21.)

That build ran on the old speculative stack (pr-14543-850db697, created 11:26Z), whose tree carries PR #14536's commit 4951d941 — the change that introduces packages/cli/test/vitest-tiers-partition.test.ts. That test fails on any tree containing #14536 because its INTEGRATION_FILES omits the #14505 guard test that constructs ObjectQL; the reading is on the anchor #14554. This PR's file set (packages/spec, content/docs, one changeset) is disjoint from packages/cli, and the test is not on main.

#14536 was ejected; the queue has rebuilt this PR on the post-ejection stack (queue head 3e5ad08a on 1f456906, none of whose trees carry that test) — that build is the one that counts. No re-run requested, no push.


Generated by Claude Code

Merged via the queue into main with commit 3e5ad08Sep 2, 2026
43 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-13809-min-approvals-default-describe branch September 2, 2026 12:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: ApprovalNodeConfigSchema.minApprovals describes "Default 1", but the quorum runtime defaults to ALL resolvable approvers

2 participants

@os-zhuang@claude