test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@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

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@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

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@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

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@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

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@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

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@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

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@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

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle - #14815

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins
Sep 3, 2026
Merged

test(plugin-auth): pin the SCIM DELETE of the last administrator, de-flake the timed-ban face, and describe the 1.7.2 lifecycle#14815
os-sales merged 4 commits into
mainfrom
claude/issue-14555-scim-lifecycle-pins

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14555

Resumption of a previous agent's branch. That agent pushed two commits and died before opening a PR or reporting, so nothing on the branch had been verified by anyone. This PR verifies the existing work item-by-item against the triage ruling, adds the one gate repair the work had broken, and lands it.

All measurements below were taken at f63bc0d22.

1. The previous agent's work, checked against the ruling

The ruling (issuecomment-5514107240) scoped five items. The branch carries all five, and each matches.

#Ruling asked forWhat the diff actually doesMatch
1Pin the SCIM DELETE of the last administrator; add a DELETE face to the existing suiteAdds one it(...) at scim-deactivation-reconcile-user.test.ts:487, inside the existing deactivating the last administrator is refused through SCIM describe. Asserts 403, the SCIM error schema, status: "403", a detail naming last administrator and ADR-0024 D5.2; then that the row survives, is unbanned, still signs in, and that the vendor tombstone rolled backyes
1bDo not restructure face (c) or the PATCH pathThe whole test diff removes exactly five lines: three header-comment lines and the two timing constants of item 3. Face (c) at :430, its positive control at :468, and every PATCH face are byte-identicalyes
1cRegister through the harness list, not a hand-written object listUntouched. The file still has import { authIdentityObjects } from './manifest.js' (line 71), const AUTH_OBJECTS = authIdentityObjects (line 102), consumed at line 174. The diff adds no object listyes
2Rewrite the stale last-admin-guard.ts header to describe 1.7.2Bullets 1 and 2 rewritten (quoted below)yes
3Widen expiry to 5 s, wait expiry + 500 msTIMED_BAN_MS = 5_000, TIMED_BAN_WAIT_MS = TIMED_BAN_MS + 500. Both faces carry 60_000 timeouts; the whole face measured 25.84 s wall for the entire 13-test file, so 5.5 s of sleep sits far inside ityes
3bThe positive control — "the most valuable single line in this card"Present at :640. Identical setup, same timed ban, same wait, no SCIM deactivation, sign-in accepted, row unbannedyes
4One stale word in the auth-manager.ts docblockadmin-ban-endpoints.ts becomes user-ban-write.tsyes
5One clause: DELETE over a timed administrator ban also makes it permanentAdded to the same docblock (quoted below)yes

Item 4 — proof that user-ban-write.ts is the live module, and that both modules exist. Both files are present on disk. The live import in auth-manager.ts is:

import{applyUserBan,applyUserUnban,SCIM_DEACTIVATION_BAN_REASON,}from'./user-ban-write.js';

admin-ban-endpoints.ts still exists and still exports runAdminBanUser / runAdminUnbanUser, so the old spelling was a stale attribution, not a dangling reference — the docblock credited the wrong module for the write this path performs.

2. Prose changed — before and after

Item 2, last-admin-guard.ts bullet 1. Before:

the better-auth admin plugin's ban endpoint writes it, and @better-auth/scim maps a SCIM active: false onto that same admin ban (which is why SCIM forces the admin plugin on — ADR-0071).

After: it now says 1.6.x mapped active: false onto the admin ban itself, that 1.7.0 removed that write in favour of an optional host callback, and that since #14360 the ban is landed by plugin-auth's own identity.reconcileUser — "the same column and the same hook, a different author". The installed dependency is @better-auth/scim1.7.2, so the version claim is current.

Item 2, bullet 2. Before:

deleting the sys_user row (#5941) — how every remove lands: SCIM DELETE /Users/{id}, better-auth's /admin/remove-user and /delete-user, an import, a script.

After, SCIM DELETE is removed from that list and called out as explicitly not a row delete on 1.7.2, arriving instead as bullet 1's ban write, "refused by guardBan on beforeUpdate and never reaches guardDelete". The rest of the row-delete half is unchanged.

Item 5, auth-manager.ts. Before:

account. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

After:

account — by the SAME branch as active: false, including over an administrator's timed ban, whose banExpires a DELETE therefore clears too: a deprovision cannot be outlived by an expiry the administrator set. For the same reason a DELETE is judged by the beforeUpdate guard below and never by any beforeDelete — deleting the last administrator through SCIM is refused exactly as deactivating them is. Re-provisioning through the tombstone re-links the same user, the state turns active, and the SCIM ban is lifted by the second bullet.

3. Ablations — both pins proved able to fail

Every leg: mutate, prove the mutation reached disk by blob hash and by literal-text counts on both the removed and injected text, run, restore against HEAD (never a bare git checkout -- path, which restores from a possibly-poisoned index), then prove restoration by blob equality and an empty git diff HEAD. Each ran under a trap ... EXIT INT TERM. No mutation was proved by a comment — esbuild strips those.

Ablation A — the positive control can fail. Inverted its sign-in expectation (expectSignInAccepted becomes expectSignInBanned).

HEAD blob = c27ebf78aec1fcdf64d3a2be894f076f61eb1be5
post blob = 8c90af927452afb782bfb322ecebb0174a8963f0 (differs: mutation on disk)
before: FROM-count=1 TO-count=0 after: FROM-count=0 TO-count=1

Mutated: Tests 1 failed | 12 skipped (13), failing precisely on the mechanism the control exists to observe:

AssertionError: expected BANNED_USER, got 200 {... "banned":true,
"banReason":"Policy violation","banExpires":"2026-09-03T02:56:41.983Z" ...}
expected 200 to be 403

That is the vendor's session.create auto-lift admitting a principal whose banExpires has passed — exactly the alternative explanation the ruling wanted excluded. Restored: blob back to c27ebf78, empty git diff HEAD, Tests 1 passed | 12 skipped (13).

Ablation B — the DELETE face exercises the guard. The ruling's own prose claims this face is refused through beforeUpdate and never reaches beforeDelete, so both legs were run. Predicted directions were fixed before running, and both held.

LegMutationPredictedMeasured
B1registerHook('beforeUpdate', guardBan, ...) replaced by a no-opredredTests 1 failed | 12 skipped (13)
B2registerHook('beforeDelete', guardDelete, ...) replaced by a no-opgreengreenTests 1 passed | 12 skipped (13)
B1: HEAD blob = 40daa74e67a583fab91f31ee7eabe93dcebd2631
post blob = c9de082ca878d49388fe6d7252b54bfcb2d11263
B2: post blob = 8ea396623d8f680b8d01750dac79c8fda87177cd
both restored to 40daa74e..., empty `git diff HEAD`

B1's failure is the DELETE succeeding and returning an empty body where the 403 SCIM error JSON was expected (SyntaxError: Unexpected end of JSON input at the res.json() on line 503). B2 is green because the SCIM DELETE never reaches guardDelete at all.

Together these confirm the measured claim the branch ships in the guard header — "removing the beforeDelete registration leaves the SCIM DELETE face green, removing the beforeUpdate one reddens it" — is accurate as written, and that the new face is genuinely pinned to the guard rather than to an incidental 403.

4. One gate the branch had broken, repaired here

check-system-context-census was red on the branch as pushed, and would have reddened CI:

::error::[anchor-is-not-a-read-site] the page anchors .../last-admin-guard.ts:286 ...
::error::[ledger-row-unused] NON_READ_ANCHORS excuses .../last-admin-guard.ts:299 ...

Cause: the item-2 header rewrite added 13 net lines above the census's read site, moving the needle line applies to EVERY context, isSystem included from 286 to 299 (verified against the merge base: 286 there, 299 here). The gate's ledger row finds that line by needle, so it followed the code; content/docs/permissions/system-context.mdx hard-codes the number, so it did not — reddening both directions at once. Repaired with the gate's own --fix, which is the authority on the form:

re-anchored content/docs/permissions/system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files,
all anchored; 145 anchors resolve, 27 declared non-read.

5. Verification

All at f63bc0d22.

  • Target file:Test Files 1 passed (1) / Tests 13 passed (13)
  • Whole affected package (@objectstack/plugin-auth, run because better-auth-schema-parity.test.ts reads auth-manager.ts as source text and a comment-only edit is not self-evidently inert to it): Test Files 91 passed (91) / Tests 1866 passed (1866)
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, including check:test-typecheck over tsconfig.test.json. This matters here: the build tsconfig.json excludes **/*.test.ts, so tsc --noEmit alone says nothing about the new test code; the test-layer program is what covers it, and this file carries no test-typecheck-debt.json entry, so it must be at zero errors.
  • Gate family, re-derived in-worktree with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack. The first derivation over the three original paths gave 28 commands; the census repair added content/docs/..., which pulled the count to 52. Re-deriving after that edit was load-bearing — 24 doc gates would otherwise have gone unrun. Union at f63bc0d22: 49 pass, 3 PREREQUISITE NOT MET.

The three unmeasured, each of which self-declares that it is neither a pass nor a finding:

GateExitWhy
check-test-completeness.mjs3Needs a saved turbo run test log, which only CI tees. Its own text names the dispatch-gates family as this branch and says to record NOT MEASURED
check:dual-build-cjs-loads3Reads built output; 40 packages have no dist/. Needs a full pnpm build
check:type-check-debt3Needs the whole workspace closure built, or it "would silently measure a DIFFERENT WORLD"

check:skill-examples initially exited 1 on an unmet prerequisite (client-react unbuilt). Rather than record it unmeasured I built the closure and re-ran it: pass, 256 prose examples type-check across 3 surfaces.

6. Changeset

None owed. The diff is comment-only in two source files, one test file, and one docs anchor — no exported symbol, no payload key, no behaviour change. Both edited docblocks are off the published surface: the auth-manager.ts block documents private async reconcileScimUserLifecycle, and the last-admin-guard.ts block is a file-level header before the first import. This is pr-automation.yml's route 2 verbatim ("It releases nothing (.github/, .claude/, skills/, docs/, content/, examples/, tests-only, and the like) -- apply the skip-changeset label"), so the label is applied rather than an empty changeset.

7. Co-occupancy

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

… de-flake the timed-ban face
Adds a face driving `DELETE /Users/{id}` against the last platform
administrator. On `@better-auth/scim` 1.7.2 that verb is not a row
delete: the vendor tombstones the SCIM source, the aggregate turns
inactive, and the deprovision arrives as the same ban write as
`PATCH active: false` — so the break-glass guard refuses it through
`guardBan` on `beforeUpdate`, and `guardDelete` never runs. The
mechanism covered this from the day the reconcile callback landed;
nothing drove it.
Widens the timed-ban face's expiry from 1.5 s to 5 s and waits
`expiry + 500 ms`. The pre-PATCH sign-in has to land while the ban is
still in force, and on a loaded shard wall-clock time is not the test's
progress. Nothing the face proves depends on the number.
Adds the positive control the widened window makes necessary: same
setup, no SCIM deactivation, same wait, sign-in accepted. Without it a
green face is equally well explained by "the expiry never elapsed".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…still describes 1.6.x
`last-admin-guard.ts`'s header listed two things that stopped being true
when the vendor's own ban write was removed in 1.7.0. Shape (1) credited
`@better-auth/scim` with mapping `active: false` onto the admin ban; that
mapping is now `plugin-auth`'s own `identity.reconcileUser`. Shape (2)
filed SCIM `DELETE /Users/{id}` under "deleting the `sys_user` row"; on
1.7.2 the vendor tombstones the SCIM source and the deprovision arrives
as shape (1)'s ban write, so it is refused by `guardBan` on
`beforeUpdate` and never reaches `guardDelete` — measured by ablation,
both directions.
`auth-manager.ts`'s reconcile docblock named `admin-ban-endpoints.ts` as
the home of the platform ban write while the module's own import reads
`./user-ban-write.js`. Both modules exist, so the pointer sent the reader
to the wrong one of two live files rather than to a missing one.
Its DELETE paragraph also left implicit what the PATCH bullet above it
states: the deactivation branch clears `banExpires`, so a DELETE makes an
administrator's timed ban permanent too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… the guard header grew
The last-admin-guard header rewrite in this branch added 13 net lines above the
`applies to EVERY context, isSystem included` line, moving it from 286 to 299.
`check-system-context-census` anchors that line from
`content/docs/permissions/system-context.mdx`, and its NON_READ_ANCHORS row
finds it by NEEDLE rather than by number — so the ledger row followed the code
and the page's hard-coded number did not, reddening the gate in both
directions at once (anchor-is-not-a-read-site + ledger-row-unused).
Repaired by the gate's own `--fix`, which is the authority on the form:
re-anchored .../system-context.mdx:201 `last-admin-guard.ts:286` -> `:299`
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 2 documentable anchor(s).

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

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol, a top-level class))
What this run could not see
  • 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 — 11 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 35da8cc60775679ebda2af1255c4cf1972ecdaaa — the merge of head f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 35da8cc60775679ebda2af1255c4cf1972ecdaaa && git checkout 35da8cc60775679ebda2af1255c4cf1972ecdaaa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79 && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff f63bc0d22ab6fbfdf63fd6569d0e94d13eb9df79
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时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 队列共有 64 个失败构建(不含本次)。

分诊清单:

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    @objectstack/cli:test: FAIL integration test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
    ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时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 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-sales@claude