Skip to content

Commit 2be67d2

Browse files
committed
fix(plugin-auth): keep the manager outcome off rows[].code, which the spec ledger does not register
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
1 parent fcaa95d commit 2be67d2

4 files changed

Lines changed: 59 additions & 28 deletions

File tree

.changeset/18028-import-users-manager-second-pass.md

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,16 @@ exactly as they are on the endpoint, with the endpoint's own `reason`
3636
discriminator carried through. There is no second copy of those predicates.
3737

3838
**A manager problem never costs the row its identity.** The user is created
39-
either way; the failure is reported on that row — `rows[].code` is
40-
`MANAGER_UNRESOLVED` or `MANAGER_REFUSED` and `rows[].manager` carries the
41-
machine-readable outcome, in the shape `rows[].delivery` already uses. It is
42-
⛔ not a whole-import failure and ⛔ not a silent skip, and an engine fault
43-
while linking is reported the same way rather than turning a 200 that created
44-
N users into a 500 that reports none of them.
39+
either way; the failure is reported on that row — `rows[].manager` carries the
40+
machine-readable outcome in the shape `rows[].delivery` already uses
41+
(`unresolved`, or the refusal's own `reason`), and `rows[].error` carries the
42+
sentence. It is ⛔ not a whole-import failure and ⛔ not a silent skip, and an
43+
engine fault while linking is reported the same way rather than turning a 200
44+
that created N users into a 500 that reports none of them. No `rows[].code` is
45+
stamped for a manager outcome: a row-level code would have to be registered in
46+
the `packages/spec` error-code ledger, which this change is fenced out of, so
47+
the machine-readable half lives on `rows[].manager` instead of on a code the
48+
vocabulary does not carry.
4549

4650
**New on the response.** `data.summary.manager` is
4751
`{ linked, unresolved, refused }`, beside `data.summary.delivery`, and the

packages/plugins/plugin-auth/src/admin-import-users-manager-pass.test.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,13 @@ describe('import-users manager pass — an unresolved key is PER-ROW (#18028)',
285285
expect(res.status).toBe(200);
286286
const data = payload(res);
287287
expect(data.rows[0].manager).toBe('unresolved');
288-
expect(data.rows[0].code).toBe('MANAGER_UNRESOLVED');
289288
expect(String(data.rows[0].error)).toContain('manager_id');
289+
// ⛔ No row-level `code`: the obvious `MANAGER_UNRESOLVED` symmetry with
290+
// `INVITE_EMAIL_FAILED` needs a `packages/spec` error-code ledger entry
291+
// this lane is fenced out of, and `check:dispatcher-error-vocabulary`
292+
// refuses an unregistered one. Pinned so the symmetry cannot be restored
293+
// without the registration that makes it legal.
294+
expect(data.rows[0].code).toBeUndefined();
290295

291296
// Direction 2 — and it is not a whole-import failure. Asserting only the
292297
// first would pass against an implementation that aborts everything.
@@ -311,7 +316,8 @@ describe('import-users manager pass — an unresolved key is PER-ROW (#18028)',
311316

312317
const data = payload(res);
313318
expect(data.rows[0].manager).toBe('unresolved');
314-
expect(data.rows[0].code).toBe('MANAGER_UNRESOLVED');
319+
expect(String(data.rows[0].error)).toContain('phone number');
320+
expect(data.rows[0].code).toBeUndefined();
315321
expect(data.summary.manager.unresolved).toBe(1);
316322
});
317323

@@ -341,6 +347,9 @@ describe('import-users manager pass — an unresolved key is PER-ROW (#18028)',
341347

342348
const data = payload(res);
343349
expect(data.rows[0].code).toBe('INVITE_EMAIL_FAILED');
350+
expect(String(data.rows[0].error)).toContain('invitation email failed');
351+
// The delivery report keeps the shared `error` slot; the manager verdict is
352+
// still readable on its own field, so neither failure is lost to silence.
344353
expect(data.rows[0].manager).toBe('unresolved');
345354
expect(data.summary.manager.unresolved).toBe(1);
346355
});
@@ -404,7 +413,9 @@ describe('import-users manager pass — the refusals are the DELEGATE\'s (#18028
404413

405414
const data = payload(res);
406415
expect(data.rows[0].manager).toBe('self_assignment');
407-
expect(data.rows[0].code).toBe('MANAGER_REFUSED');
416+
// The delegate's own message, carried through rather than re-worded.
417+
expect(String(data.rows[0].error)).toContain('cannot be their own manager');
418+
expect(data.rows[0].code).toBeUndefined();
408419
expect(data.summary.manager).toEqual({ linked: 0, unresolved: 0, refused: 1 });
409420
expect(h.userBy('a@x.co')?.manager_id ?? null).toBeNull();
410421
});
@@ -424,7 +435,7 @@ describe('import-users manager pass — the refusals are the DELEGATE\'s (#18028
424435
const data = payload(res);
425436
expect(data.rows[0].manager).toBe('linked');
426437
expect(data.rows[1].manager).toBe('cycle');
427-
expect(data.rows[1].code).toBe('MANAGER_REFUSED');
438+
expect(String(data.rows[1].error)).toContain('loop');
428439
expect(data.summary.manager).toEqual({ linked: 1, unresolved: 0, refused: 1 });
429440
expect(h.userBy('b@x.co')?.manager_id ?? null).toBeNull();
430441
});

packages/plugins/plugin-auth/src/admin-import-users.ts

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,11 @@
8888
* identity are that function's to answer, and the refusal it returns is
8989
* reported on the row rather than re-worded here.
9090
* - **A manager problem never costs the row its identity.** The user is
91-
* created either way and the failure rides `rows[].code` / `rows[].error`,
92-
* exactly like the sibling post-write `INVITE_EMAIL_FAILED`; `rows[].manager`
93-
* carries the machine-readable outcome in `rows[].delivery`'s shape. ⛔ Not a
94-
* whole-import failure, and ⛔ not a silent skip.
91+
* created either way; `rows[].manager` carries the machine-readable outcome
92+
* in `rows[].delivery`'s shape and `rows[].error` carries the sentence.
93+
* ⛔ Not a whole-import failure, and ⛔ not a silent skip. ⛔ And no
94+
* `rows[].code` — see {@link noteManagerFailure} for why that half is fenced
95+
* out of this lane rather than forgotten.
9596
* - **The pass does not run on `dryRun`.** It is a post-write pass like
9697
* delivery: with nothing created there are no ids to link, and the five
9798
* refusals cannot be evaluated against rows that do not exist. A dry run
@@ -331,23 +332,36 @@ export interface IdentityImportRowResult extends ImportRowResult {
331332
/**
332333
* Stamp a row's manager failure.
333334
*
334-
* `manager` is ALWAYS set — that is this outcome's own channel. `code`/`error`
335-
* is the SHARED row error channel the sibling `INVITE_EMAIL_FAILED` also writes
336-
* to, so it is claimed only when free: a row whose invitation already failed
337-
* keeps that report and still carries its manager verdict on `manager`,
338-
* ⛔ rather than one of the two failures overwriting the other into silence.
335+
* `manager` is ALWAYS set — that is this outcome's own channel, and it is the
336+
* machine-readable one: `'unresolved'` and each refusal `reason` are distinct
337+
* members of {@link ImportManagerOutcome}, so a caller discriminates on one
338+
* field without parsing a sentence.
339+
*
340+
* ⛔ NO `rows[].code` IS STAMPED, and that is a fence rather than an oversight.
341+
* The sibling post-write failure writes `code: 'INVITE_EMAIL_FAILED'`, and a
342+
* matching `MANAGER_UNRESOLVED` / `MANAGER_REFUSED` pair would read as the
343+
* obvious symmetry — but `check:dispatcher-error-vocabulary` refuses a code
344+
* this package's `packages/spec` ledger entry does not register, and
345+
* registering one is a `packages/spec` edit this lane is fenced out of
346+
* (the closed-vocabulary question for this endpoint's refusals is already
347+
* carried by #17995). So the failure rides `error` — the human half — and
348+
* `manager` — the machine half — and the row-level code is left to the seat
349+
* that owns the vocabulary. ⛔ Reaching for an already-registered code whose
350+
* meaning is something else would be the lenient alias Prime Directive #12
351+
* refuses.
352+
*
353+
* `error` is the SHARED row channel that sibling also writes to, so it is
354+
* claimed only when free: a row whose invitation already failed keeps that
355+
* report and still carries its manager verdict on `manager`, ⛔ rather than one
356+
* of the two failures overwriting the other into silence.
339357
*/
340358
function noteManagerFailure(
341359
row: IdentityImportRowResult,
342360
outcome: ImportManagerOutcome,
343-
code: string,
344361
message: string,
345362
): void {
346363
row.manager = outcome;
347-
if (row.code === undefined) {
348-
row.code = code;
349-
row.error = message;
350-
}
364+
if (row.error === undefined) row.error = message;
351365
}
352366

353367
/**
@@ -724,7 +738,6 @@ export async function runAdminImportUsers(
724738
noteManagerFailure(
725739
r,
726740
'unresolved',
727-
'MANAGER_UNRESOLVED',
728741
`The ${MANAGER_COLUMN} cell is neither an email address nor a phone number this deployment `
729742
+ 'can read, so it names no identity. This row landed; only its manager link did not.',
730743
);
@@ -763,7 +776,6 @@ export async function runAdminImportUsers(
763776
noteManagerFailure(
764777
r,
765778
'unresolved',
766-
'MANAGER_UNRESOLVED',
767779
`No user matches this row's ${MANAGER_COLUMN} key, in this import or already in the `
768780
+ 'directory, so the manager link was not written. The rest of this row landed.',
769781
);
@@ -786,15 +798,14 @@ export async function runAdminImportUsers(
786798
noteManagerFailure(
787799
r,
788800
'unresolved',
789-
'MANAGER_UNRESOLVED',
790801
`The manager link could not be written: ${(e as Error)?.message ?? String(e)}. `
791802
+ 'This row itself landed.',
792803
);
793804
managerLinks.unresolved++;
794805
continue;
795806
}
796807
if (refusal) {
797-
noteManagerFailure(r, refusal.reason, 'MANAGER_REFUSED', refusal.message);
808+
noteManagerFailure(r, refusal.reason, refusal.message);
798809
managerLinks.refused++;
799810
continue;
800811
}

scripts/engine-double-contract.pinned.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2376,6 +2376,11 @@
23762376
"verb": "update",
23772377
"pinned": 1
23782378
},
2379+
{
2380+
"file": "packages/plugins/plugin-auth/src/admin-import-users-manager-pass.test.ts",
2381+
"verb": "update",
2382+
"pinned": 2
2383+
},
23792384
{
23802385
"file": "packages/plugins/plugin-auth/src/admin-import-users.test.ts",
23812386
"verb": "update",

0 commit comments

Comments
 (0)