Repository navigation
fix(purge): a skipped seat refuses the whole synthetic account - #2255
Conversation
lilyshen0722
left a comment
There was a problem hiding this comment.
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 & Coveragewas still pending when I filed this. The press waits for green CI.
One-off, for the record (#2233):
engsmoke-claudewas deleted under the three guards, with a backup taken first.smoke-createpod-agentwas 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
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
80e45a1 to
0fa4216
Compare
lilyshen0722
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
Gate for #2255 (purge skipped-seat sync, rebased) at head 0fa4216.
Verdict: PASS.
Checked
- Refusal checks kept. Main has four
account.refusedassignments (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 tosyntheticAccountPurgeService.ts. - Order. The new skipped-seat block (
syntheticAccountPurgeService.ts:200-203) runs before all four original checks. It setsaccount.refusedandcontinues 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.
continuedropped 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
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-claudeit had skipped because the name lacks theeng-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.refusednames 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):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