Skip to content

Commit 37e02ea

Browse files
committed
test(dogfood): identify the demotion history row by what it records, not its position (#4586)
The Dogfood Regression Gate failed on this file's own new assertion: expected '{"role":"admin"}' to contain 'member' Not a defect in what `trackHistory` stores — the audit writer's `update` leg writes a CHANGED-FIELDS DIFF, `old_value` the before-state and `new_value` the after-state (`diff()` in `audit-writers.ts` fills both halves), so a demotion really is recorded as `{"role":"admin"}` → `{"role":"member"}`. The audit question "who changed X from member to admin" was already answerable; #4586 adds WHO to a row that already knew WHAT. The defect was the TEST's row SELECTION. It took `rows[rows.length - 1]` as "the newest row", which is wrong twice over: 1. `find` without an explicit sort is unordered, so the last element is not the newest anything; 2. the audit row lands ASYNCHRONOUSLY after the endpoint returns, so at the moment the demotion test polled, the only `update` row present was the PROMOTION from the earlier test — and it asserted the demotion's expectation against it. Locally green, red in CI, because `packages/qa/dogfood` sits outside the `--filter` scope the change was verified under. `waitForHistoryMatching` now waits for the row that says the thing under test, so both hazards are gone. The demotion case additionally pins the pair (`old_value` admin → `new_value` member) and asserts the promotion survives as a DISTINCT row keeping its own actor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
1 parent be6967e commit 37e02ea

1 file changed

Lines changed: 51 additions & 7 deletions

File tree

packages/qa/dogfood/test/membership-actor-attribution.dogfood.test.ts

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ async function waitForMembership(ql: any, userId: string): Promise<any> {
5757
throw new Error(`no sys_member row appeared for ${userId}`);
5858
}
5959

60-
/** Audit rows this stack wrote for one `sys_member` row, newest last. */
60+
/** Audit rows this stack wrote for one `sys_member` row. */
6161
async function memberHistory(ql: any, memberId: string, action?: string): Promise<any[]> {
6262
const rows = await findRows(
6363
ql,
@@ -68,13 +68,38 @@ async function memberHistory(ql: any, memberId: string, action?: string): Promis
6868
return action ? rows.filter((r: any) => r.action === action) : rows;
6969
}
7070

71-
async function waitForHistory(ql: any, memberId: string, action: string): Promise<any> {
71+
/**
72+
* Wait for the history row that records a SPECIFIC transition, identified by
73+
* what it says rather than by its position.
74+
*
75+
* Selecting "the last row" is wrong twice over here: `find` is unordered
76+
* without an explicit sort, and — the trap this file walked into — the audit
77+
* row lands ASYNCHRONOUSLY after the endpoint returns, so a promote/demote
78+
* pair polled by count alone happily returns the *previous* transition and
79+
* asserts against it. Matching on the row's own `new_value` is immune to both.
80+
*
81+
* On `action: 'update'` the audit writer stores a CHANGED-FIELDS DIFF: the
82+
* before-state in `old_value`, the after-state in `new_value` (a demotion is
83+
* `old_value {"role":"admin"}` → `new_value {"role":"member"}`). Both halves
84+
* are recorded — this change adds WHO to a row that already knew WHAT.
85+
*/
86+
async function waitForHistoryMatching(
87+
ql: any,
88+
memberId: string,
89+
action: string,
90+
matches: (row: any) => boolean,
91+
what = action,
92+
): Promise<any> {
7293
for (let i = 0; i < 20; i++) {
73-
const rows = await memberHistory(ql, memberId, action);
74-
if (rows.length > 0) return rows[rows.length - 1];
94+
const hit = (await memberHistory(ql, memberId, action)).find(matches);
95+
if (hit) return hit;
7596
await new Promise((r) => setTimeout(r, 250));
7697
}
77-
throw new Error(`no '${action}' history row appeared for sys_member ${memberId}`);
98+
throw new Error(`no '${what}' history row appeared for sys_member ${memberId}`);
99+
}
100+
101+
async function waitForHistory(ql: any, memberId: string, action: string): Promise<any> {
102+
return waitForHistoryMatching(ql, memberId, action, () => true);
78103
}
79104

80105
describe('#4586: the better-auth actor reaches sys_member history and the grant', () => {
@@ -246,12 +271,31 @@ describe('#4586: the better-auth actor reaches sys_member history and the grant'
246271
const [row] = await findRows(ql, 'sys_member', { id: memberRowId }, 1);
247272
expect(row.role).toBe('member');
248273

249-
const updates = await memberHistory(ql, memberRowId, 'update');
250-
const demotion = updates[updates.length - 1];
274+
// Identify the demotion row by what it RECORDS, not by its position: the
275+
// promotion row is already there, and the new row lands asynchronously.
276+
const demotion = await waitForHistoryMatching(
277+
ql,
278+
memberRowId,
279+
'update',
280+
(r) => String(r.new_value).includes('member'),
281+
'demotion',
282+
);
283+
// The pair the diff actually stores: admin → member.
284+
expect(String(demotion.old_value)).toContain('admin');
251285
expect(String(demotion.new_value)).toContain('member');
252286
// Attribution works in both directions — taking authority away is exactly
253287
// as answerable as handing it out.
254288
expect(demotion.user_id).toBe(adminUserId);
289+
// …and it is a DISTINCT row from the promotion, which keeps its own actor.
290+
const promotion = await waitForHistoryMatching(
291+
ql,
292+
memberRowId,
293+
'update',
294+
(r) => String(r.new_value).includes('admin'),
295+
'promotion',
296+
);
297+
expect(String(promotion.id)).not.toBe(String(demotion.id));
298+
expect(promotion.user_id).toBe(adminUserId);
255299

256300
// NOTE: whether the org-admin GRANT is revoked on demotion is deliberately
257301
// not asserted here. It currently is not — `auto-org-admin-grant`'s only

0 commit comments

Comments
 (0)