Skip to content

fix(purge): a skipped seat refuses the whole synthetic account - #2255

Merged
lilyshen0722 merged 1 commit into
mainfrom
sre-purge-refuse-skipped-seat
Oct 29, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
sre-purge-refuse-skipped-seat

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

TASK-260 residual, per CTO's ruling (2026-10-16).

What happened. The purge's first apply (02:47Z, see #2233) deleted a journey account whose seat engsmoke-claude it had skipped because the name lacks the eng-smoke-journey- prefix. The seat's ephemeral registry row survived, pointing at a deleted publisher. Install refuses an ephemeral row to anyone but its publisher (routes/registry/install.ts:234), so that name is now dead for everyone, permanently. Every skip reason orphans the row the same way: no prefix, another user's install, or a bot older than the account.

Change. An account with any skipped seat is refused, not deleted. account.refused names each seat and its reason, the same shape as the foreign-human pod refusal, and the scheduler already logs a [synthetic-purge] refused <username>: … warn line for it. A leftover test account is harmless; a dead name is not. A refused account is re-examined and refused again on every run, so it stays visible in the log until someone looks at it.

Tests (syntheticAccountPurgeService.test.js):

  • The three seat-skip arms (another user's install, no prefix, older bot) now assert refusal, with the person, Scout, the pod and the seat's registry row, bot and memory all untouched. The no-prefix arm previously asserted the person WAS deleted.
  • New: a refused account stays refused on the next run, while an ordinary account beside it is still purged.
  • Mutation: dropping the refusal (so the account is deleted anyway) fails all four arms. 14/14 pass across this suite and schedulerService.syntheticPurge.test.js; lint 0 errors; tsc clean.

The two orphan rows already in production (engsmoke-claude, smoke-createpod-agent) are removed separately by a guarded one-off; before and after are on #2233.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MCgCsf3UJRCPRbnNKxsECe

@coderabbitai ignore

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code gate at 80e45a1f (the eng lead's ruling on the TASK-260 residual).

The change is the ruling, in the right place. If any seat is skipped, account.refused names each seat with its reason, and the loop continues before anything is deleted. It sits beside the foreign-human pod refusal and has the same shape, and it applies in dry run too, so the dry-run report shows the refusal before an apply would. The docstring states why: install refuses an ephemeral row to anyone but its publisher, so deleting the publisher kills the name for everyone.

The tests changed in the right direction. The no-prefix arm used to assert the person was deleted; it now asserts the whole account is untouched (expectUntouched: the person, Scout, the pod, the registry row, the bot, and the memory). The new re-run arm proves a refused account doesn't block an ordinary account beside it.

Run here at this head (Node 22):

result
BASE syntheticAccountPurgeService.test.js 9/9
purge + scheduler suites 8 suites, 36/36
M1: refusal dropped 4 failed
M2: refused set but no continue, so deletion proceeds 4 failed
M3: refuse only for the no-prefix reason 2 failed (the other two reasons)
M4: refuse every account (over-refusal) 5 failed (the ordinary-account arms)

Each mutation was restored from a copy and compared. eslint: 0 errors (the one max-len warning, at :214, predates this diff). tsc: clean for this file.

Not blocking:

  • The branch is one commit behind main (#2252, onboarding, no overlap); the merge queue tests the combination.
  • Test & Coverage was still pending when I filed this. The press waits for green CI.

One-off, for the record (#2233):

  • engsmoke-claude was deleted under the three guards, with a backup taken first.
  • smoke-createpod-agent was correctly skipped because it has its own bot user. My ruling: leave it. Deleting an agent's bot user to free one stale test name isn't worth breaking identity continuity, and freeing only the row would let a reinstall adopt that bot (TASK-270).

GATE: PASS

@lilyshen0722 lilyshen0722 added the no-bot-review Skip automatic bot review (see docs/development/external-review.md) label Oct 23, 2026
TASK-260 residual, CTO ruling 2026-10-16. The first apply (02:47Z) deleted
an account whose seat it had skipped for lacking the journey prefix. That
left the seat's ephemeral registry row pointing at a deleted publisher.
Install refuses an ephemeral row to anyone but its publisher
(routes/registry/install.ts), so the name is dead for everyone,
permanently. Every skip reason (no prefix, another user's install, a bot
older than the account) orphans the row the same way.

The purge now refuses an account that has any skipped seat, naming each
seat and its reason, in the same shape as the foreign-human pod refusal. A
leftover test account is harmless; a dead name is not.

The three seat-skip arms now assert refusal, with the person, Scout, the
pod and the seat's registry row, bot and memory all untouched. A new arm
shows a refused account stays refused on the next run while an ordinary one
beside it is still purged. Dropping the refusal fails all four arms. 14/14
pass across the purge and scheduler suites.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCgCsf3UJRCPRbnNKxsECe
@lilyshen0722
lilyshen0722 force-pushed the sre-purge-refuse-skipped-seat branch from 80e45a1 to 0fa4216 Compare October 26, 2026 05:31

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otto, gate measured at 0fa4216

Opus gate (Tier 2, purge). I checked the two things Lily asked for, and then whether a refused account can still lose anything.

Every refusal check on main is kept. The loop at this head has main's four account refusals unchanged and in main's order: the disposable Scout installed by another user (backend/services/syntheticAccountPurgeService.ts:207-212), a disposable agent in a DM outside the footprint (:213-220), a disposable pod holding an outside installation (:223-228), and a pod with another human member (:229-236). The file at the merge-base is byte-identical to origin/main, so the comparison is against current main. All four seat-skip arms are also kept as they were, including the admin-bot arm main gained after this PR was written (:176-179).

The skipped-seat refusal runs first (:200-204), ahead of the Scout check. Ordering isn't what keeps this safe, though. Everything before the first refusal is a read (User.findOne, AgentRegistry.find, AgentInstallation.exists, Pod.find), and every refusal continues before the first write, the per-pod remove loop at :238. A refused account loses nothing whichever refusal fires, and in a dry run it tallies nothing.

What I ran: both suites pass (18 tests) and CI is green at this head. I ran two mutants and restored each.

  • Refuse only when more than one seat was skipped: 4 tests fail, which is the three arms plus the re-run case.
  • Delete the admin-bot skip arm (:176-179): all tests still pass.

Non-blocking, so the follow-up doesn't wait on this PR. The admin-bot arm is the one skip reason no test pins, here or on main. With this PR, that arm now refuses the whole account. If it were removed, a seat bot promoted to admin after the account was created would be deleted. The predate arm covers the realistic case, and the gap predates this PR. A five-line arm beside the other three would close it. The body's "Every skip reason" sentence also lists three reasons and should add the fourth.

Written by Otto, a Commonly agent — https://commonly.me/v2/pods/6a920d3eb08dee6c42a81bfc

GATE: PASS

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate for #2255 (purge skipped-seat sync, rebased) at head 0fa4216.

Verdict: PASS.

Checked

  • Refusal checks kept. Main has four account.refused assignments (scoutBot installed by another user, a DM outside the synthetic footprint, a pod installation outside the footprint, a foreign human member). All four are present at head, and the PR diff only adds lines to syntheticAccountPurgeService.ts.
  • Order. The new skipped-seat block (syntheticAccountPurgeService.ts:200-203) runs before all four original checks. It sets account.refused and continues before any of them can run.
  • The block's reason string names every skipped seat with its reason.

Mutation (in /tmp/wt-2255, restored, tree clean)

  • New block removed: 4 tests fail.
  • continue dropped from the new block: 4 tests fail.

Tests: syntheticAccountPurgeService.test.js passes 13/13.

Non-blocking

  • No test pins the order. A future edit that moves the skipped-seat check below the scoutBot check would still pass the suite. A test that sets up an account with both a skipped seat and a scoutBot installed by another user would catch it, and would match the "runs first" clause in the request.

Written by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS

@lilyshen0722
lilyshen0722 added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit f8a6ed4 Oct 29, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-bot-review Skip automatic bot review (see docs/development/external-review.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant