Skip to content

fix(plugin-security): the sys_permission_set duplicate-name refusal carries UNIQUE_VIOLATION, and the packaged-set lock answers first - #19437

Merged
huangyiirene merged 3 commits into
mainfrom
claude/issue-19307-duplicate-name-refusal-code
Sep 21, 2026
Merged

huangyiirene merged 3 commits into
mainfrom
claude/issue-19307-duplicate-name-refusal-code

Conversation

@huangyiirene

@huangyiirene huangyiirene commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Fixes #19307

Clause-②: yes

The data door's duplicate-name refusal on sys_permission_set had two halves, both reproduced on today's head (c33707933) before any edit and re-measured after, on examples/app-showcase with a seeded admin over a cookie session.

The two halves

1. No machine-readable code. The insert leg threw a bare Error carrying .status = 409 and no .code. The flat { error, code } responder invents nothing for a producer that declared nothing, so the caller got prose — against ADR-0112's 2026-08-17 amendment (#9232), under which the flat door carries the closed member too.

2. ⭐ It ran BEFORE assertPermissionSetNotPackageDeclared. A package-declared set has a projected row, so its name is duplicate AND locked at once. The admin who opens the Clone dialog on a packaged set and types the base set's own name — the single most likely thing to type — got already exists, which names no remedy, and never reached NOT_OVERRIDABLE, which names the clone path.

Live readings — five legs, same script, before and after

Script: POST /api/v1/data/sys_permission_set as the seeded admin; full text in the report comment on #19307.

leg BEFORE (c33707933) AFTER (this branch, built)
1. duplicate name = package-declared showcase_manager 409 · {"error":"[Security] permission set 'showcase_manager' already exists","object":"sys_permission_set"}no code key 403 · "code":"NOT_OVERRIDABLE", message: "…Choose a different name for your set, or clone 'showcase_manager' (the "Clone" action…)"
2. create a NON-packaged set 201 201 (unchanged)
3. ⚖️ negative control — duplicate of that NON-packaged set 409 · no code 409 · "code":"UNIQUE_VIOLATION", message byte-identical
4. ⚖️ contrast control — unauthenticated PATCH, same resource 401 · "code":"UNAUTHENTICATED" 401 · "code":"UNAUTHENTICATED" (unchanged)
5. ⚖️ contrast control — PATCH the packaged set (non-duplicate route) 403 · "code":"NOT_OVERRIDABLE" unchanged

Legs 4 and 5 are what make legs 1 and 3 readings rather than constants: the flat door already varied its code by path, so the absence was this producer's and never the door's.

Why UNIQUE_VIOLATION, reused and not minted

sys_permission_set declares { fields: ['name'], unique: 'organization' }, and the reading recorded on that index itself (#8554) is org_yi 409 UNIQUE_VIOLATION. So the platform already answers this exact collision with this exact envelope whenever the index catches it instead of the projection's pre-check. A second spelling here would make one condition answer two envelopes depending only on which layer got there first — the drift @objectstack/rest and @objectstack/driver-memory deliberately registered the SAME code to avoid. RESOURCE_CONFLICT (the standard member 409 derives from) is what the door supplies for a producer that named no condition; using it would be that second spelling.

The packages/spec touch is one provenance row

check:error-code-provenance recognises objlit, assign and *_CODE constdef stamp sites and states in its own bounds line that it is blind to class fields. Written as a class-field literal, this package would have become an unlisted EMITTER of a registered code with every gate in the repo green — the invisibility the ledger header names ("no admission rule checks WHO emits"), found by hand three times already (#7504 / #13254 / #13353). So the code is stamped through an exported PERMISSION_SET_NAME_CONFLICT_CODE, which puts the emitter inside the gate's field of view, and the ledger gains the matching row under @objectstack/plugin-security.

Measured red then green, in that order:

  • class-field spelling, no ledger row: gate exit 0 — it never saw the stamp;
  • *_CODE constant, no ledger row: gate exit 1"@objectstack/plugin-security stamps 'UNIQUE_VIOLATION' (constdef) at packages/plugins/plugin-security/src/errors.ts:372 — not listed under its own owner key";
  • *_CODE constant + the row: gate exit 0, 339 stamp sites, 322 listed.

It is provenance, not identity, and that is measured rather than asserted. The deduped ledger union is 282 distinct codes before and 282 after, added [] / removed []UNIQUE_VIOLATION was already a member under two other packages. pnpm --filter @objectstack/spec check:generated reports all 15 generated artifacts up to date, which supports ONE claim only — no regeneration is MISSING. ⛔ It does NOT show the published TYPE is unchanged, and here it is not: ERROR_CODE_LEDGER is exported as const satisfies, so this row moves typeof ERROR_CODE_LEDGER['@objectstack/plugin-security'] from a 3-tuple to a 4-tuple. check:api-surface stayed GREEN THROUGH that change, because api-surface/api.json records the symbol name (ERROR_CODE_LEDGER (const)) and no signature, api-surface-signatures.json covers 27 define* helpers and not this const, and api-surface-declarations/ was withdrawn by #19024 and does not exist on this base. That is why this PR declares Clause-②: yes and grades @objectstack/spec minor.

⚠️ check-widening-tells still fires T4 on that line with Clause-②: no (exit 4), because the matcher cannot see the dedupe — it reads any new registry entry as an acceptance-set growth. The measurement above is the evidence it is false here. Reported for the owning seat rather than repaired in this PR; the declaration line is copied verbatim from the claim and is not mine to move.

One corner moved with the order, declared not incidental

An ordinary duplicate attempted while no artifact source can answer now takes the lock's fail-closed unknown refusal — 403 NOT_OVERRIDABLE (PackagedPermissionSetProvenanceUnknownError, "retry once the metadata layer is readable") — instead of the 409. Both are refusals and neither writes; case 5 of the new suite pins it so a reader finds a decision rather than an accident.

Tests

packages/plugins/plugin-security/src/permission-set-duplicate-name-refusal.test.ts — five cases: the ordinary duplicate's envelope (code + status + both status spellings, never a bare "it threw" — the unfixed producer threw too), the packaged-set case answering the lock (proved by the lock's own message and by saves.length === 0, since ADR-0005's tier gate answers the same code with a different message), the negative control that ordinary duplicates WHOSE PROVENANCE RESOLVES did not become NOT_OVERRIDABLE (the corner above is the case that qualifier excludes), the happy path, and the fail-closed corner.

Reverse verification — two ablations, each proved on disk and restored byte-identical to HEAD:

  • A — put the guard back behind the duplicate check. Mutation landed (anchor 1 → 0, marker 0 → 1, blob fed1af15857f3d92cb95990f); suite 2 failed / 3 passed — cases 2 and 5, the two ordering-dependent ones, and only those. Restored: git diff HEAD empty, blob back to fed1af15857f.
  • B — drop the code stamp. Mutation landed (target 1 → 0, marker 0 → 1); suite 2 failed / 3 passed — cases 1 and 3, expected undefined to be 'UNIQUE_VIOLATION'. Restored: blob back to 7b0914b1749a.

Both legs ran under a trap ... EXIT INT TERM with absolute paths, restored through git checkout HEAD -- path, and verified by git hash-object against the HEAD blob rather than by an exit code.

Verification

Run at final head 72d68b905 unless stated.

  • pnpm --filter @objectstack/plugin-security test116 files / 2240 tests passed; typecheck OK (test layer 0 files / 0 errors).
  • pnpm --filter @objectstack/spec test505 files / 14752 tests passed; typecheck OK; check:generated — all 15 artifacts up to date.
  • pnpm lint — the whole repo, eslint . --no-inline-config, exit 0. Not a narrowed run, so no narrowing needs defending.
  • scripts/pm/dispatch-gates.mjs --ran98 derived families, 98 run, 0 NOT-MEASURED, 0 UNRUN, each with a recorded exit code, all 0. Two of them were genuinely red mid-flight and are green only because the fix landed: check:engine-double-contract (the new double's update now opens with assertEngineUpdateDispatch, and its pinned seams are registered) and check:objectql-double-limit (the find double now holds the caller's bound by presence).
  • Control-character self-scan over every touched file: clean.

Builds and test runs went through scripts/pm/os-verify-lock.sh.


Generated by Claude Code


Generated by Claude Code

…TION, and the packaged-set lock speaks first

The data door's duplicate-name refusal on `sys_permission_set` threw a bare
`Error` with `.status = 409` and no `.code`, so the flat `{ error, code }`
responder — which invents nothing for a producer that declared nothing — put
prose on the wire, against ADR-0112's 2026-08-17 amendment.

It also ran BEFORE `assertPermissionSetNotPackageDeclared`. A package-declared
set has a projected row, so its name is duplicate and locked at once, and the
admin who opens the Clone dialog on a packaged set and types the base set's own
name never reached NOT_OVERRIDABLE — the refusal that names the remedy.

`UNIQUE_VIOLATION` is reused rather than minted: `sys_permission_set` declares
`{ fields: ['name'], unique: 'organization' }`, so the same collision already
answers 409 UNIQUE_VIOLATION when the index catches it instead of this
pre-check. The code is stamped through an exported `*_CODE` constant so
`check:error-code-provenance` can see this package as an emitter; the ledger
gains the matching provenance row.

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
…the lock's precedence

Five cases on `createPermissionSetWriteThrough`'s insert leg: the ordinary
duplicate's ADR-0112 envelope (`code` + `status`, never a bare throw), the
packaged-set case answering the lock rather than the conflict, the negative
control that ordinary duplicates did NOT become NOT_OVERRIDABLE, the happy
path, and the fail-closed `unknown` corner the reorder moved.

The engine double holds the caller's `limit` and opens `update` with
`assertEngineUpdateDispatch`, and the new pinned seams are registered in
`scripts/engine-double-contract.pinned.json`.

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests labels Sep 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-security, @objectstack/spec, touching 13 documentable anchor(s).

14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/api/error-catalog.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object), UNIQUE_VIOLATION (literal, a string literal in ERROR_CODE_LEDGER; a string literal in PERMISSION_SET_NAME_CONFLICT_CODE), sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/api/error-handling-server.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/data-modeling/drivers.mdx (via UNIQUE_VIOLATION (literal, a string literal in ERROR_CODE_LEDGER; a string literal in PERMISSION_SET_NAME_CONFLICT_CODE))
  • content/docs/deployment/environment-variables.mdx (via sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/kernel/contracts/data-engine.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/permissions/authorization.mdx (via sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/permissions/delegated-administration.mdx (via setName (symbol, a field of class PermissionSetNameConflictError), showcase_manager (literal, a string literal in a comment on a changed line), sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/permissions/field-level-security.mdx (via statusCode (symbol, a field of class PermissionSetNameConflictError))
  • content/docs/permissions/permission-sets.mdx (via sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/permissions/sharing-rules.mdx (via /sharing/rules/:idOrName/evaluate (route, bridged from symbol statusCode — its route source's handler names it))
  • content/docs/permissions/system-context.mdx (via createPermissionSetWriteThrough (symbol, a top-level function))
  • content/docs/protocol/kernel/error-handling.mdx (via UNIQUE_VIOLATION (literal, a string literal in ERROR_CODE_LEDGER; a string literal in PERMISSION_SET_NAME_CONFLICT_CODE))
  • content/docs/protocol/kernel/http-protocol.mdx (via UNIQUE_VIOLATION (literal, a string literal in ERROR_CODE_LEDGER; a string literal in PERMISSION_SET_NAME_CONFLICT_CODE))

7 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v12.mdx (via sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/releases/v13.mdx (via sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/releases/v15.mdx (via sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/releases/v17/17-0.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object), sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough))
  • content/docs/releases/v17/17-1.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object), sys_permission_set (literal, a string literal in a comment on a changed line; a string literal in createPermissionSetWriteThrough), /api/v1/data/sys_permission_set (route, a path literal in a comment in ERROR_CODE_LEDGER))
  • content/docs/releases/v17/17-3.mdx (via UNIQUE_VIOLATION (literal, a string literal in ERROR_CODE_LEDGER; a string literal in PERMISSION_SET_NAME_CONFLICT_CODE))
  • content/docs/releases/v17/17-4.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object), UNIQUE_VIOLATION (literal, a string literal in ERROR_CODE_LEDGER; a string literal in PERMISSION_SET_NAME_CONFLICT_CODE))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 139 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 57ceb9d6d2af35ebf83d0f8cc396cb612540af74packageMentionDocs.

Which tree this was computed on

This run read content/docs from 060b7340722e298e00d0e0fea330f3dd2167a2c1 — the merge of head f51a59889faf0a8e494b842385fcc8feeb1eb1e7 into base 57ceb9d6d2af35ebf83d0f8cc396cb612540af74, 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 060b7340722e298e00d0e0fea330f3dd2167a2c1 && git checkout 060b7340722e298e00d0e0fea330f3dd2167a2c1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 57ceb9d6d2af35ebf83d0f8cc396cb612540af74 f51a59889faf0a8e494b842385fcc8feeb1eb1e7 && git checkout -B drift-repro 57ceb9d6d2af35ebf83d0f8cc396cb612540af74 && git merge --no-ff f51a59889faf0a8e494b842385fcc8feeb1eb1e7

node scripts/docs-audit/affected-docs.mjs --json 57ceb9d6d2af35ebf83d0f8cc396cb612540af74

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 57ceb9d6d2af35ebf83d0f8cc396cb612540af74 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…es, and state the reorder's cost

The clause-② declaration is disjunctive and only the accept-set limb had been
measured. `ERROR_CODE_LEDGER` is exported `as const satisfies`, so its published
type carries literal tuples and the provenance row moves
`typeof ERROR_CODE_LEDGER['@objectstack/plugin-security']` from a 3-tuple to a
4-tuple — the public-surface limb. A purely additive widening of a published
surface takes at least `minor`; `@objectstack/plugin-security` stays `patch`.

Three wording repairs in the same file: the fail-closed corner names its code
(`403 NOT_OVERRIDABLE`), the "what did NOT move" claim is qualified to the case
where provenance resolves, and the reorder's undeclared side effect is stated —
`getMetaItemLayered` is now evaluated unconditionally, so a duplicate insert
pays one extra metadata round trip and the duplicate path is coupled to
metadata-layer reachability.

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator Author

契约复核 provenance —— 达档复核 PASS 在案,needs:contract-review 同笔剥除

domain:services seat · session_01AhQASwqJr2Z7XfGWUdvnbF · 写于 2026-09-20T23:18Z。

Reviewed-by: 隔离达档子代理(CONTRACT_REVIEW_TIER),⛔ 认领席未自审(本席判断档)
Reviewed-head: 72d68b905a27b9b00b3f4b4234ada199157554ef
Landing-head: f51a59889faf0a8e494b842385fcc8feeb1eb1e7

为什么复核判的 head 与落地 head 不同,而复核结论仍然有效

复核首轮判 FAIL(补丁轮),并在判词里自己划定了边界:「缺陷本体、复用选型、测试质量与范围四项全过,⛔ 无需改代码逻辑;所欠全在申报、载体与 changeset 定级三处,按 4 步一轮可清」。

四步已全部完成,且本席逐条现验:

做了什么 现验
1 卡侧申报 noyes Clause-②-correction: 5752398762(评论 5753070284),门读作 yes
2 PR 正文申报行 → yes PATCH 后回读为真
3 changeset @objectstack/spec patch → minor 现读 frontmatter 已 minor,plugin-security 留 patch
4 needs:contract-review 双载体 已挂,并由本评论同笔剥

⭐ 决定性的一条:git diff --name-only 72d68b905 f51a59889 -- 'packages/**' 'scripts/**' 为空 ⇒ 复核审过的代码逐字节未动,这一轮只改了 changeset 文本与 PR 正文。所以复核对代码的四项结论不需要重取

条款②的最终值与理由

Clause-②: yes —— 理由 ⛔ 不是接受集增长。去重并集实测 282 → 282,added/removed 皆空(本席与复核各用一种方法独立测得,本席按数组元素行计并排除注释行,复核交 JS 解析器求值)。yes 的依据是析取式的另一条肢:ERROR_CODE_LEDGER 是已发布导出(api-surface/api.json:325)且 as const satisfies(error-code-ledger.zod.ts:1286),新增一元素使该 key 的字面量 tuple 3 元组 → 4 元组,已发布类型变了 ⇒ 扩大公开面。在档同形先例 #17909 / PR #17924 以同一理由落地。

⇒ changeset 因此必须 ≥ minor,这是申报的硬耦合后果,⛔ 不是可选项。

入队前七闸,全部有仪器背书

读数
CI(按 check-name 取最新,钉 f51a59889) 32 success · 3 skipped · 0 失败 · 0 挂起
三条 cancelled 的 check 各有更晚的同名 run 为 success ⇒ 被取代,非未决
check-expected-skips exit 0 —— 5 个 skip 全在名册
check-clause2-carriers --pair 19437 exit 0(C2-CORRECTION 读作 yes)
check-governed-merges NOT governed,502 行 < 5000
.gitattributes os-regen 当场从 origin/main 重读(18 条路径),本 PR 六个文件无一命中
达档复核 PASS 本评论即记录

⚠️ 两条如实申报:① 复核环境 node_modules 未安装,其消融结论由读断言 + 读控制流推出,⛔ 非实跑;dev 侧的实跑读数与之同向。② 本席本地工作树停在 1739f71879,故上述 scripts/pm/* 门禁均改用 origin/main 抽出的当前版本重跑 —— 本班已因陈旧工作树出过一次行号错(5751763031)。


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review September 20, 2026 23:18
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 20, 2026
Merged via the queue into main with commit 8f6d831 Sep 21, 2026
44 of 47 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-19307-duplicate-name-refusal-code branch September 21, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants