From cf14e83a566020492876155020e578480a924ea4 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 1 Sep 2026 04:39:07 -0700 Subject: [PATCH] =?UTF-8?q?test(backend):=20re-certify=20the=20agentRuntim?= =?UTF-8?q?eAuth=20projection=20guard=20=E2=80=94=20main=20is=20red?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Test & Coverage` fails on origin/main: agentUserShapeContract's load-bearing assertion expects 1 `.select(` in agentRuntimeAuth.ts and finds 2. Nothing is actually broken. #1312 (AgentCredential substrate, ADR-026 Phase 0, merged 2026-08-27) added AgentCredential.findById(credential.parentId).select('status') which is a projection on AgentCredential, not on the User row. The guard trips on ANY new `.select(` in the file by design, so that the author re-certifies it consciously — this is the guard working, not a false alarm to be silenced. Why it reached main: the guard (#1210) was green on its own base 799e0d7dd, where the file had exactly 1 projection. #1312 had added the second four days earlier. Merging #1210 at 11:05Z today produced a tree neither PR's CI had tested. The two never conflicted and never could: #1312 edits the middleware, #1210 adds a test that READS the middleware, so they touch disjoint paths and every merge check — including `git merge-tree` — calls them clean. A source assertion is coupled to files it does not modify, and that coupling is invisible to conflict detection. Re-certification rather than a bumped constant, which is what the guard's own comment demands. The count stays as the fail-closed backstop; added on top: - both certified projections pinned to their receivers by regex; - `selectReceivers()`, which maps each `.select(` back to the model it is called on and asserts the set is exactly {AgentCredential, Pod}. A future projection on User now fails by NAME, not just by count. Verified it still discriminates: adding `.select('_id username')` to the `User.findOne` in the middleware fails two tests (count 2→3, and the statement-level assertion). Reverted after. 45/45 in unit/middleware green. Renamed the test — it said "exactly one projection, and it is the Pod.find", which stopped being true in this diff. Co-Authored-By: Claude Opus 5 --- .../middleware/agentUserShapeContract.test.js | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/unit/middleware/agentUserShapeContract.test.js b/backend/__tests__/unit/middleware/agentUserShapeContract.test.js index 7f866baeb7..575f7c3e9c 100644 --- a/backend/__tests__/unit/middleware/agentUserShapeContract.test.js +++ b/backend/__tests__/unit/middleware/agentUserShapeContract.test.js @@ -36,6 +36,25 @@ const codeOnly = () => source const selectCallCount = () => (codeOnly().match(/\.select\s*\(/g) || []).length; +/** + * The model each `.select(` is called on, sorted. Walks back from every + * projection to the nearest preceding `Model.find*(` — crude on purpose, in + * the same spirit as the counter: it does not have to be right for the guard + * to fail closed, because `selectCallCount` already trips on any new + * projection. This exists so the failure names the receiver instead of only + * a number, which is what turns "bump the constant" into "re-certify". + */ +const selectReceivers = () => { + const code = codeOnly(); + const queries = [...code.matchAll(/\b([A-Z]\w*)\s*\.\s*(?:findOne|findById|find)\s*\(/g)]; + return [...code.matchAll(/\.select\s*\(/g)] + .map((m) => { + const preceding = queries.filter((q) => q.index < m.index); + return preceding.length ? preceding[preceding.length - 1][1] : '(none)'; + }) + .sort(); +}; + /** Statement starting at `User.findOne(`, up to the terminating semicolon. */ const userFindOneStatements = () => { const out = []; @@ -56,7 +75,7 @@ describe('the middleware reads the full User row', () => { expect(userFindOneStatements()).toHaveLength(2); }); - it('the file contains exactly one projection, and it is the Pod.find', () => { + it('every projection in the file is certified, and none is on a User query', () => { // THE LOAD-BEARING ASSERTION, and it fails CLOSED. @sprint-review found // the statement-scoping below fails OPEN on the most likely edit: it ends // a statement at the first `;` after `User.findOne(`, which is not the end @@ -77,8 +96,17 @@ describe('the middleware reads the full User row', () => { // is not. Enumerate the protected item instead: ANY new `.select(` // anywhere in the file trips this, and the author re-certifies it // consciously. A guard that must parse correctly to fail is not a guard. - expect(selectCallCount()).toBe(1); + expect(selectCallCount()).toBe(2); expect(codeOnly()).toMatch(/Pod\.find\([\s\S]{0,200}?\.select\('_id'\)/); + expect(codeOnly()).toMatch(/AgentCredential\.findById\([\s\S]{0,120}?\.select\('status'\)/); + // Re-certification, not a bumped constant. The count above is the + // fail-closed backstop; this is the record of WHICH projections are + // certified and on what. #1312 (ADR-026 Phase 0) added the + // AgentCredential one on 2026-08-27 and it is fine — the property is + // "no USER query is projected", and AgentCredential is not the User row. + // Bumping 1 to 2 without this list is the escape hatch the guard exists + // to close, so the receivers are enumerated below and asserted. + expect(selectReceivers()).toEqual(['AgentCredential', 'Pod']); }); it('projects neither User.findOne', () => {