From 0ec20ac710e811c21ff3d24b0d05328c01b8c786 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 13:58:22 -0500 Subject: [PATCH 01/15] test: no-ODS selectability respects cross-year toggle (red) A no-ODS year with no roster file should be selectable at job creation when the partner has cross-year matching enabled (EDU can supply the roster), and still rejected when the toggle is off. Tests fail until resolveJobDestination reads the partner toggle. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/api/integration/tests/jobs.spec.ts | 70 ++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/app/api/integration/tests/jobs.spec.ts b/app/api/integration/tests/jobs.spec.ts index cabdf206..5511b959 100644 --- a/app/api/integration/tests/jobs.spec.ts +++ b/app/api/integration/tests/jobs.spec.ts @@ -400,6 +400,76 @@ describe('POST /jobs', () => { } }); + it('should create a no-ODS job without a roster file when cross-year matching is enabled', async () => { + await prisma.schoolYearConfig.create({ + data: { + partnerId: tenantA.partnerId, + schoolYearId: '2324', + isEnabled: true, + sendToOds: false, + }, + }); + await prisma.partner.update({ + where: { id: tenantA.partnerId }, + data: { crossYearMatchingEnabled: true }, + }); + + const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; + doesFileExistMock.mockResolvedValue(false); + + try { + const res = await request(app.getHttpServer()) + .post(endpoint) + .set('Cookie', [sessionA.cookie]) + .send({ + ...postJobDto, + schoolYearId: '2324', + }); + + expect(res.status).toBe(201); + + const job = await prisma.job.findUnique({ where: { id: res.body.id } }); + expect(job?.odsId).toBeNull(); + expect(job?.sendToOds).toBe(false); + } finally { + doesFileExistMock.mockResolvedValue(true); + await prisma.partner.update({ + where: { id: tenantA.partnerId }, + data: { crossYearMatchingEnabled: false }, + }); + } + }); + + it('should reject a no-ODS job with no roster file when cross-year matching is disabled', async () => { + await prisma.schoolYearConfig.create({ + data: { + partnerId: tenantA.partnerId, + schoolYearId: '2324', + isEnabled: true, + sendToOds: false, + }, + }); + // partner A defaults to crossYearMatchingEnabled=false + + const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; + doesFileExistMock.mockResolvedValue(false); + + try { + const res = await request(app.getHttpServer()) + .post(endpoint) + .set('Cookie', [sessionA.cookie]) + .send({ + ...postJobDto, + schoolYearId: '2324', + }); + + expect(res.status).toBe(400); + expect(res.body.message).toContain('No roster file found'); + } finally { + doesFileExistMock.mockResolvedValue(true); + } + }); + it('should reject requests when the school year is not enabled', async () => { const res = await request(app.getHttpServer()) .post(endpoint) From 005e815eca11b9839d46b3010fd7df419be92b7d Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 13:58:30 -0500 Subject: [PATCH 02/15] feat: allow EDU-sourced rosters for no-ODS job selectability (green) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveJobDestination now reads the partner's crossYearMatchingEnabled flag. A no-ODS year is valid if a roster file exists OR the toggle is on; the S3 existence check is short-circuited when the toggle is on. Partner setting only — no creds/connection check, per spec. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/api/src/jobs/jobs.service.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/app/api/src/jobs/jobs.service.ts b/app/api/src/jobs/jobs.service.ts index ef2e5354..d1228722 100644 --- a/app/api/src/jobs/jobs.service.ts +++ b/app/api/src/jobs/jobs.service.ts @@ -95,10 +95,21 @@ export class JobsService { } if (!config.sendToOds) { - const rosterKey = rosterFileKey({ partnerId: input.tenant.partnerId, tenantCode: input.tenant.code }, config.schoolYear); - const rosterExists = await this.fileService.doesFileExist(rosterKey, this.appConfig.rosterBucket()); - if (!rosterExists) { - return { status: 'error', code: 'roster_file_missing' }; + // A no-ODS year is valid if a roster file exists OR the partner has + // cross-year matching enabled (EDU can supply the roster). This is the + // partner setting only — no creds/connection check (see project brief). + // Short-circuit the S3 check when the toggle is on; we don't need the file. + const partner = await this.prisma.partner.findUnique({ + where: { id: input.tenant.partnerId }, + select: { crossYearMatchingEnabled: true }, + }); + + if (!partner?.crossYearMatchingEnabled) { + const rosterKey = rosterFileKey({ partnerId: input.tenant.partnerId, tenantCode: input.tenant.code }, config.schoolYear); + const rosterExists = await this.fileService.doesFileExist(rosterKey, this.appConfig.rosterBucket()); + if (!rosterExists) { + return { status: 'error', code: 'roster_file_missing' }; + } } return { From 07f4393fa4258aa76d5e60a8c2715be7ba098e33 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 14:06:45 -0500 Subject: [PATCH 03/15] test: getTenantConfig hasRoster reflects cross-year toggle (red) For a no-ODS year, hasRoster should be true when the partner toggle is on even with no S3 file (and the S3 check should be skipped). ODS years stay null. Fails until the toggle is folded into hasRoster. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/school-year-config.spec.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/app/api/integration/tests/school-year-config.spec.ts b/app/api/integration/tests/school-year-config.spec.ts index 6e6656d4..d67c59a9 100644 --- a/app/api/integration/tests/school-year-config.spec.ts +++ b/app/api/integration/tests/school-year-config.spec.ts @@ -354,5 +354,36 @@ describe('GET /school-year-config/tenant', () => { doesFileExistMock.mockResolvedValue(true); } }); + + it('should return hasRoster=true for a no-ODS year when cross-year matching is enabled, without an S3 check', async () => { + const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; + doesFileExistMock.mockClear(); + doesFileExistMock.mockResolvedValue(false); + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: true }, + }); + + try { + const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [cookieA]); + + // 2526 is sendToOds=false; toggle on → hasRoster true even with no file + const row2526 = res.body.find((r: any) => r.schoolYearId === '2526'); + expect(row2526.hasRoster).toBe(true); + + // ODS years stay null regardless of the toggle + const row2425 = res.body.find((r: any) => r.schoolYearId === '2425'); + expect(row2425.hasRoster).toBeNull(); + + // Toggle short-circuits the S3 check entirely + expect(doesFileExistMock).not.toHaveBeenCalled(); + } finally { + doesFileExistMock.mockResolvedValue(true); + await global.prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: false }, + }); + } + }); }); }); From d01a37b19f4da6e26779057599893aa378461d46 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 14:06:46 -0500 Subject: [PATCH 04/15] feat: fold cross-year toggle into getTenantConfig hasRoster (green) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasRoster for a no-ODS year is now true when crossYearMatchingEnabled is on, short-circuiting the S3 doesFileExist call. ODS years stay null. No DTO change — hasRoster's shape is unchanged, only its computation, so the crossYearMatchingEnabled flag never reaches the FE. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../school-year-config.controller.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/api/src/school-year-config/school-year-config.controller.ts b/app/api/src/school-year-config/school-year-config.controller.ts index 38f9136a..d63195bd 100644 --- a/app/api/src/school-year-config/school-year-config.controller.ts +++ b/app/api/src/school-year-config/school-year-config.controller.ts @@ -37,6 +37,15 @@ export class SchoolYearConfigController { @Authorize('school-year-config.read') @Get('tenant') async getTenantConfig(@TenantDecorator() tenant: Tenant) { + // When cross-year matching is enabled, EDU can supply the roster, so a + // no-ODS year has a roster regardless of any S3 file. Partner setting only + // — no creds/connection check (see project brief). + const partner = await this.prisma.partner.findUnique({ + where: { id: tenant.partnerId }, + select: { crossYearMatchingEnabled: true }, + }); + const crossYearMatchingEnabled = partner?.crossYearMatchingEnabled ?? false; + const schoolYears = await this.prisma.schoolYear.findMany({ where: { schoolYearConfig: { @@ -75,6 +84,8 @@ export class SchoolYearConfigController { const hasRoster = config.sendToOds ? null + : crossYearMatchingEnabled + ? true : await this.fileService.doesFileExist( rosterFileKey({ partnerId: tenant.partnerId, tenantCode: tenant.code }, schoolYear), this.appConfig.rosterBucket(), From 04a2368ea47f389c36f8f38721351be654fbff22 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 14:07:07 -0500 Subject: [PATCH 05/15] test: payload omits rosterFilePath when EDU is the roster source (red) On a no-ODS job with cross-year matching available, the executor payload should carry appUrls.roster and omit rosterFilePath (a dangling S3 pointer). Fails until rosterFilePath is gated on crossYearMatchAvailable. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/tests/earthbeam-api.spec.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/app/api/integration/tests/earthbeam-api.spec.ts b/app/api/integration/tests/earthbeam-api.spec.ts index e9c7454f..f6e371bf 100644 --- a/app/api/integration/tests/earthbeam-api.spec.ts +++ b/app/api/integration/tests/earthbeam-api.spec.ts @@ -168,6 +168,27 @@ describe('Earthbeam API', () => { expect(res.body.crossYearMatchAvailable).toBe(false); expect(res.body.appUrls.roster).toBeUndefined(); }); + + it('omits rosterFilePath on a no-ODS job when cross-year matching is available (EDU is the source)', async () => { + const authService = app.get(EarthbeamApiAuthService); + const noOdsJob = await seedJob({ + sendToOds: false, + schoolYearId: '2324', + bundle: bundleA, + tenant: tenantA, + }); + const noOdsRun = noOdsJob.runs[0]; + const noOdsToken = await authService.createAccessToken({ runId: noOdsRun.id }); + + const res = await request(app.getHttpServer()) + .get(`/earthbeam/jobs/${noOdsRun.id}`) + .set('Authorization', `Bearer ${noOdsToken}`); + + expect(res.status).toBe(200); + expect(res.body.crossYearMatchAvailable).toBe(true); + expect(res.body.appUrls.roster).toBeDefined(); + expect(res.body.rosterFilePath).toBeUndefined(); + }); }); // TODO: add tests for things other than descriptor mappings From 0c1071f37d128afb3a79868e82673ed1e7728d0b Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 14:07:07 -0500 Subject: [PATCH 06/15] feat: omit rosterFilePath when cross-year matching is available (green) When crossYearMatchAvailable is true the executor pulls the roster from EDU via appUrls.roster, so the S3 rosterFilePath would be a dangling pointer. Omit it. The executor only reads rosterFilePath in its non-cross-year branch, so the absent key is safe. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/api/src/earthbeam/api/earthbeam-api.service.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/api/src/earthbeam/api/earthbeam-api.service.ts b/app/api/src/earthbeam/api/earthbeam-api.service.ts index 9c2e9ba1..e929688e 100644 --- a/app/api/src/earthbeam/api/earthbeam-api.service.ts +++ b/app/api/src/earthbeam/api/earthbeam-api.service.ts @@ -171,9 +171,14 @@ export class EarthbeamApiService { }, crossYearMatchAvailable, sendToOds: job.sendToOds, - rosterFilePath: job.sendToOds - ? undefined - : `s3://${this.configService.rosterBucket()}/${rosterFileKey(job, job.schoolYear)}`, + // When cross-year matching is available, the executor pulls the roster + // from EDU via appUrls.roster, so the S3 file path would be a dangling + // (often nonexistent) pointer — omit it. The executor only reads + // rosterFilePath in its non-cross-year branch. + rosterFilePath: + job.sendToOds || crossYearMatchAvailable + ? undefined + : `s3://${this.configService.rosterBucket()}/${rosterFileKey(job, job.schoolYear)}`, // odsConnection check narrows the type — the early guard ensures it's present when sendToOds assessmentDatastore: odsConnection && job.sendToOds From 5230571cf652e527847db5da7c66a1a21cb5bb3e Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 14:07:36 -0500 Subject: [PATCH 07/15] feat: make roster copy source-agnostic on FE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasRoster now folds in the cross-year toggle, so all FE gating (already keyed off hasRoster === true) is correct with no logic change. Drop "file" from user-facing strings — a roster may come from EDU or an S3 file, and the FE shouldn't reason about the source. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/fe/src/app/Pages/Jobs/JobCreatePage.tsx | 2 +- app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx b/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx index b859b885..843c6daa 100644 --- a/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx +++ b/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx @@ -210,7 +210,7 @@ export const JobCreatePage = () => { year.sendToOds && !year.hasOds ? ' (no ODS configured)' : !year.sendToOds && year.hasRoster !== true - ? ' (no roster file loaded)' + ? ' (no roster loaded)' : '' }`, value: year.schoolYearId, diff --git a/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx b/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx index 1e9f1f64..5c0201b5 100644 --- a/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx +++ b/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx @@ -228,8 +228,7 @@ const RosterYearContent = ({ hasRoster }: { hasRoster: boolean }) => { {!hasRoster && ( - A roster file is required to match student IDs. Contact support to have a roster file - loaded. + A roster is required to match student IDs. Contact support to have a roster loaded. )} @@ -243,10 +242,10 @@ const RosterYearContent = ({ hasRoster }: { hasRoster: boolean }) => { {hasRoster ? : } - {hasRoster ? 'roster file loaded' : 'roster file not loaded'} + {hasRoster ? 'roster loaded' : 'roster not loaded'} - {!hasRoster && } + {!hasRoster && } ); From dccb7d34c66495f8b73fc068c2c2c6aefd0b56e6 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 14:07:36 -0500 Subject: [PATCH 08/15] docs: document roster sources and no-ODS selectability Add a roster-source precedence note (ODS -> EDU -> S3 file) and the partner-setting-only selectability/green gate to AGENTS.md. The executor already prefers EDU over the S3 file (merged in #60); no executor change in this work. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 1c7c33c6..59bea658 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -178,6 +178,16 @@ flowchart TD Cross-year-matched rows are never sent to the ODS — they're only made available through the Runway app's API, which EDU and other external consumers query. +### Roster sources & no-ODS year selectability + +A roster is the student lookup the executor matches input rows against. Source precedence: + +1. **ODS** — for `sendToOds` years, the executor fetches the roster from the ODS API. +2. **EDU** — for no-ODS (`sendToOds=false`) years, if `crossYearMatchAvailable`, the executor pulls the roster from EDU (Snowflake) via `appUrls.roster` as NDJSON. EDU is preferred over the S3 file when available (executor handles this; no app change in PR 3). +3. **S3 roster file** — the fallback for no-ODS years when cross-year matching is unavailable (`__rosters/...jsonl`). The app omits `rosterFilePath` from the payload when `crossYearMatchAvailable` is true (it would be a dangling pointer). + +A no-ODS year is **selectable** at job creation, and shows **green** ("roster loaded") on the ODS-config page, when a roster file exists **OR** the partner has cross-year matching enabled. This gate is the partner setting only (`crossYearMatchingEnabled`) — no creds/connection check, so a year can be selectable while the executor's `crossYearMatchAvailable` (which also requires `canConnect`) is false; that case fails cleanly at run time per run atomicity. + ### S3 Path Structure ``` From 0cce5f7eedb034cdc43fe0e92c625235113d892c Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 15:21:45 -0500 Subject: [PATCH 09/15] fix: round 1 review fixes for cross-year roster sourcing PR 3 makes no-ODS roster availability source-agnostic (S3 file or EDU), but two reachable FE surfaces still implied an S3 roster file was required. Make the copy source-neutral so it stays accurate when the roster comes from EDU: - SetupRequiredPage: "Roster File Required" -> "Roster Required" and drop "file" from the body + contact-support message. - UnmatchedStudents: no-ODS branch "the roster file" -> "the roster". Also add a positive smoke test for the external API v1 job-create path. Both job-create paths (internal POST /jobs and external v1) flow through the shared resolveJobDestination, but only the internal path had a toggle-on/no-file coverage test. Add the analogous test for the v1 endpoint: no-ODS year, no roster file, partner cross-year matching enabled -> 201 with odsId=null/sendToOds=false. Passes against the behavior already on this branch. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/tests/external-api.v1.spec.ts | 40 +++++++++++++++++++ .../src/app/Pages/Home/SetupRequiredPage.tsx | 6 +-- .../JobViewComponents/UnmatchedStudents.tsx | 2 +- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/app/api/integration/tests/external-api.v1.spec.ts b/app/api/integration/tests/external-api.v1.spec.ts index 9449c1ee..4e7ca21e 100644 --- a/app/api/integration/tests/external-api.v1.spec.ts +++ b/app/api/integration/tests/external-api.v1.spec.ts @@ -275,6 +275,46 @@ describe('ExternalApiV1', () => { expect(job?.odsId).toBeNull(); expect(job?.sendToOds).toBe(false); }); + + it('should create a no-ODS job without a roster file when cross-year matching is enabled', async () => { + await prisma.schoolYearConfig.create({ + data: { + partnerId: partnerA.id, + schoolYearId: '2324', + isEnabled: true, + sendToOds: false, + }, + }); + await prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: true }, + }); + + const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; + doesFileExistMock.mockResolvedValue(false); + + try { + const res = await request(app.getHttpServer()) + .post(endpoint) + .set('Authorization', `Bearer ${token}`) + .send({ + ...jobInput, + schoolYear: '2024', + }); + + expect(res.status).toBe(201); + + const job = await prisma.job.findUnique({ where: { uid: res.body.uid } }); + expect(job?.odsId).toBeNull(); + expect(job?.sendToOds).toBe(false); + } finally { + doesFileExistMock.mockResolvedValue(true); + await prisma.partner.update({ + where: { id: partnerA.id }, + data: { crossYearMatchingEnabled: false }, + }); + } + }); }); describe('API Client Info', () => { diff --git a/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx b/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx index 422c5bbb..255928a5 100644 --- a/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx +++ b/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx @@ -30,12 +30,12 @@ export const SetupRequiredPage = () => { const doesAnyYearSendToOds = yearConfigs.some((y) => y.sendToOds); if (!doesAnyYearSendToOds) { return ( - + - Before you can start uploading assessments, a roster file must be loaded for your + Before you can start uploading assessments, a roster must be loaded for your district. Please contact support for assistance. - + ); } diff --git a/app/fe/src/app/Pages/Jobs/JobViewComponents/UnmatchedStudents.tsx b/app/fe/src/app/Pages/Jobs/JobViewComponents/UnmatchedStudents.tsx index 79ef83ac..6911cf99 100644 --- a/app/fe/src/app/Pages/Jobs/JobViewComponents/UnmatchedStudents.tsx +++ b/app/fe/src/app/Pages/Jobs/JobViewComponents/UnmatchedStudents.tsx @@ -57,7 +57,7 @@ export const UnmatchedStudents = ({ job }: { job: GetJobDto }) => { )} If the file already contains the correct ID, then the student likely does not exist in - {job.sendToOds ? ' the ODS' : ' the roster file'}. Contact your district administrator. + {job.sendToOds ? ' the ODS' : ' the roster'}. Contact your district administrator. From 3c3b93dbe19a5dde5596932613e8a4c5108bba5a Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 15:24:53 -0500 Subject: [PATCH 10/15] docs: correct roster-precedence note to "no executor change" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The EDU-precedence bullet said "no app change in PR 3", but PR 3 does change the app (selectability + rosterFilePath omission). The point being made is that the executor is unchanged — it already prefers EDU over the S3 file (merged in #60). Reword to match the kickoff. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 59bea658..624e8f86 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -183,7 +183,7 @@ Cross-year-matched rows are never sent to the ODS — they're only made availabl A roster is the student lookup the executor matches input rows against. Source precedence: 1. **ODS** — for `sendToOds` years, the executor fetches the roster from the ODS API. -2. **EDU** — for no-ODS (`sendToOds=false`) years, if `crossYearMatchAvailable`, the executor pulls the roster from EDU (Snowflake) via `appUrls.roster` as NDJSON. EDU is preferred over the S3 file when available (executor handles this; no app change in PR 3). +2. **EDU** — for no-ODS (`sendToOds=false`) years, if `crossYearMatchAvailable`, the executor pulls the roster from EDU (Snowflake) via `appUrls.roster` as NDJSON. EDU is preferred over the S3 file when available (executor handles this; no executor change in PR 3). 3. **S3 roster file** — the fallback for no-ODS years when cross-year matching is unavailable (`__rosters/...jsonl`). The app omits `rosterFilePath` from the payload when `crossYearMatchAvailable` is true (it would be a dangling pointer). A no-ODS year is **selectable** at job creation, and shows **green** ("roster loaded") on the ODS-config page, when a roster file exists **OR** the partner has cross-year matching enabled. This gate is the partner setting only (`crossYearMatchingEnabled`) — no creds/connection check, so a year can be selectable while the executor's `crossYearMatchAvailable` (which also requires `canConnect`) is false; that case fails cleanly at run time per run atomicity. From e2bad1e85efa9faf1435ceed411663f555056e8a Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 16:28:36 -0500 Subject: [PATCH 11/15] refactor: rename hasRoster to hasNonOdsRoster and refine copy/docs Post-review polish: - Rename the GetTenantSchoolYearConfigDto field hasRoster -> hasNonOdsRoster. ODS years already have an ODS-fetched roster, so "hasRoster" was ambiguous; the field specifically reports whether a no-ODS year has a roster available (S3 file or EDU). Stays null for ODS years. Updated the controller, FE consumers (JobCreatePage, OdsConfigsPage, routes/index), and the integration tests. - Status copy "roster loaded" / "roster not loaded" -> "roster available" / "roster not available", and the JobCreate suffix "(no roster loaded)" -> "(no roster available)". "Available" reads better for a status that may be satisfied by EDU rather than a loaded file. - Drop transient cross-references from code comments and AGENTS.md (the "(see project brief)" notes and the "PR 3" mention) so they don't rot. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- .../tests/school-year-config.spec.ts | 22 +++++++++---------- app/api/src/jobs/jobs.service.ts | 2 +- .../school-year-config.controller.ts | 9 +++++--- app/fe/src/app/Pages/Jobs/JobCreatePage.tsx | 6 ++--- app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx | 4 ++-- app/fe/src/app/routes/index.tsx | 2 +- app/models/src/dtos/school-year-config.dto.ts | 5 ++++- 8 files changed, 29 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 624e8f86..b02b8b73 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -183,7 +183,7 @@ Cross-year-matched rows are never sent to the ODS — they're only made availabl A roster is the student lookup the executor matches input rows against. Source precedence: 1. **ODS** — for `sendToOds` years, the executor fetches the roster from the ODS API. -2. **EDU** — for no-ODS (`sendToOds=false`) years, if `crossYearMatchAvailable`, the executor pulls the roster from EDU (Snowflake) via `appUrls.roster` as NDJSON. EDU is preferred over the S3 file when available (executor handles this; no executor change in PR 3). +2. **EDU** — for no-ODS (`sendToOds=false`) years, if `crossYearMatchAvailable`, the executor pulls the roster from EDU (Snowflake) via `appUrls.roster` as NDJSON. EDU is preferred over the S3 file when available (the executor handles this preference). 3. **S3 roster file** — the fallback for no-ODS years when cross-year matching is unavailable (`__rosters/...jsonl`). The app omits `rosterFilePath` from the payload when `crossYearMatchAvailable` is true (it would be a dangling pointer). A no-ODS year is **selectable** at job creation, and shows **green** ("roster loaded") on the ODS-config page, when a roster file exists **OR** the partner has cross-year matching enabled. This gate is the partner setting only (`crossYearMatchingEnabled`) — no creds/connection check, so a year can be selectable while the executor's `crossYearMatchAvailable` (which also requires `canConnect`) is false; that case fails cleanly at run time per run atomicity. diff --git a/app/api/integration/tests/school-year-config.spec.ts b/app/api/integration/tests/school-year-config.spec.ts index d67c59a9..2d29a87c 100644 --- a/app/api/integration/tests/school-year-config.spec.ts +++ b/app/api/integration/tests/school-year-config.spec.ts @@ -286,22 +286,22 @@ describe('GET /school-year-config/tenant', () => { expect(yearIds).toContain('2526'); expect(yearIds).not.toContain('2324'); - // 2425: sendToOds=true → hasRoster is null (no S3 check), hasOds from tenant's ODS config + // 2425: sendToOds=true → hasNonOdsRoster is null (no S3 check), hasOds from tenant's ODS config const row2425 = res.body.find((r: any) => r.schoolYearId === '2425'); expect(row2425).toMatchObject({ sendToOds: true, hasOds: true, - hasRoster: null, + hasNonOdsRoster: null, startYear: 2024, endYear: 2025, }); - // 2526: sendToOds=false → hasRoster checked via S3, hasOds still reflects ODS config existence + // 2526: sendToOds=false → hasNonOdsRoster checked via S3, hasOds still reflects ODS config existence const row2526 = res.body.find((r: any) => r.schoolYearId === '2526'); expect(row2526).toMatchObject({ sendToOds: false, hasOds: true, - hasRoster: true, + hasNonOdsRoster: true, startYear: 2025, endYear: 2026, }); @@ -340,22 +340,22 @@ describe('GET /school-year-config/tenant', () => { expect(row2526.hasOds).toBe(true); }); - it('should return hasRoster=false when roster file does not exist', async () => { + it('should return hasNonOdsRoster=false when roster file does not exist', async () => { const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; doesFileExistMock.mockResolvedValue(false); try { const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [cookieA]); - // 2526 is seeded as sendToOds=false, so hasRoster reflects the S3 check + // 2526 is seeded as sendToOds=false, so hasNonOdsRoster reflects the S3 check const row2526 = res.body.find((r: any) => r.schoolYearId === '2526'); - expect(row2526.hasRoster).toBe(false); + expect(row2526.hasNonOdsRoster).toBe(false); } finally { doesFileExistMock.mockResolvedValue(true); } }); - it('should return hasRoster=true for a no-ODS year when cross-year matching is enabled, without an S3 check', async () => { + it('should return hasNonOdsRoster=true for a no-ODS year when cross-year matching is enabled, without an S3 check', async () => { const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; doesFileExistMock.mockClear(); doesFileExistMock.mockResolvedValue(false); @@ -367,13 +367,13 @@ describe('GET /school-year-config/tenant', () => { try { const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [cookieA]); - // 2526 is sendToOds=false; toggle on → hasRoster true even with no file + // 2526 is sendToOds=false; toggle on → hasNonOdsRoster true even with no file const row2526 = res.body.find((r: any) => r.schoolYearId === '2526'); - expect(row2526.hasRoster).toBe(true); + expect(row2526.hasNonOdsRoster).toBe(true); // ODS years stay null regardless of the toggle const row2425 = res.body.find((r: any) => r.schoolYearId === '2425'); - expect(row2425.hasRoster).toBeNull(); + expect(row2425.hasNonOdsRoster).toBeNull(); // Toggle short-circuits the S3 check entirely expect(doesFileExistMock).not.toHaveBeenCalled(); diff --git a/app/api/src/jobs/jobs.service.ts b/app/api/src/jobs/jobs.service.ts index d1228722..cd967f70 100644 --- a/app/api/src/jobs/jobs.service.ts +++ b/app/api/src/jobs/jobs.service.ts @@ -97,7 +97,7 @@ export class JobsService { if (!config.sendToOds) { // A no-ODS year is valid if a roster file exists OR the partner has // cross-year matching enabled (EDU can supply the roster). This is the - // partner setting only — no creds/connection check (see project brief). + // partner setting only — no creds/connection check. // Short-circuit the S3 check when the toggle is on; we don't need the file. const partner = await this.prisma.partner.findUnique({ where: { id: input.tenant.partnerId }, diff --git a/app/api/src/school-year-config/school-year-config.controller.ts b/app/api/src/school-year-config/school-year-config.controller.ts index d63195bd..9047f33e 100644 --- a/app/api/src/school-year-config/school-year-config.controller.ts +++ b/app/api/src/school-year-config/school-year-config.controller.ts @@ -39,7 +39,7 @@ export class SchoolYearConfigController { async getTenantConfig(@TenantDecorator() tenant: Tenant) { // When cross-year matching is enabled, EDU can supply the roster, so a // no-ODS year has a roster regardless of any S3 file. Partner setting only - // — no creds/connection check (see project brief). + // — no creds/connection check. const partner = await this.prisma.partner.findUnique({ where: { id: tenant.partnerId }, select: { crossYearMatchingEnabled: true }, @@ -82,7 +82,10 @@ export class SchoolYearConfigController { ); } - const hasRoster = config.sendToOds + // ODS years use an ODS-fetched roster, so this is null for them. For + // no-ODS years, a roster is available from an S3 file or, when + // cross-year matching is enabled, from EDU. + const hasNonOdsRoster = config.sendToOds ? null : crossYearMatchingEnabled ? true @@ -97,7 +100,7 @@ export class SchoolYearConfigController { endYear: schoolYear.endYear, sendToOds: config.sendToOds, hasOds: schoolYear.odsConfig.length > 0, - hasRoster, + hasNonOdsRoster, }; }) ); diff --git a/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx b/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx index 843c6daa..c89d7cae 100644 --- a/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx +++ b/app/fe/src/app/Pages/Jobs/JobCreatePage.tsx @@ -209,8 +209,8 @@ export const JobCreatePage = () => { label: `${year.startYear} - ${year.endYear} school year${ year.sendToOds && !year.hasOds ? ' (no ODS configured)' - : !year.sendToOds && year.hasRoster !== true - ? ' (no roster loaded)' + : !year.sendToOds && year.hasNonOdsRoster !== true + ? ' (no roster available)' : '' }`, value: year.schoolYearId, @@ -219,7 +219,7 @@ export const JobCreatePage = () => { const year = selectableYears.find((row) => row.schoolYearId === option.value); if (!year) return true; // narrow .find() return type if (year.sendToOds && !year.hasOds) return true; - if (!year.sendToOds && year.hasRoster !== true) return true; + if (!year.sendToOds && year.hasNonOdsRoster !== true) return true; return false; }} > diff --git a/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx b/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx index 5c0201b5..49c7cf75 100644 --- a/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx +++ b/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx @@ -103,7 +103,7 @@ export const OdsConfigsPage = () => { onDelete={() => odsConfig && confirmDelete(yearConfig, odsConfig)} /> ) : ( - + )} ); @@ -242,7 +242,7 @@ const RosterYearContent = ({ hasRoster }: { hasRoster: boolean }) => { {hasRoster ? : } - {hasRoster ? 'roster loaded' : 'roster not loaded'} + {hasRoster ? 'roster available' : 'roster not available'} {!hasRoster && } diff --git a/app/fe/src/app/routes/index.tsx b/app/fe/src/app/routes/index.tsx index e41d5b1e..3ae622fd 100644 --- a/app/fe/src/app/routes/index.tsx +++ b/app/fe/src/app/routes/index.tsx @@ -15,7 +15,7 @@ export const Route = createFileRoute('/')({ // the admin who needs to enable years in the first place and define // whether jobs for those years are sent to an ODS. const isAnyYearReadyForJobs = yearConfigs.some((y) => - y.sendToOds ? y.hasOds : y.hasRoster === true + y.sendToOds ? y.hasOds : y.hasNonOdsRoster === true ); const isPartnerAdmin = me?.roles?.includes('PartnerAdmin') ?? false; if (isAnyYearReadyForJobs || isPartnerAdmin) { diff --git a/app/models/src/dtos/school-year-config.dto.ts b/app/models/src/dtos/school-year-config.dto.ts index 621d5f6e..d2b4a9d7 100644 --- a/app/models/src/dtos/school-year-config.dto.ts +++ b/app/models/src/dtos/school-year-config.dto.ts @@ -42,8 +42,11 @@ export class GetTenantSchoolYearConfigDto { @Expose() hasOds: boolean; + // Null for ODS years (they use an ODS-fetched roster). For no-ODS years, + // true when a roster is available — from an S3 file or, when cross-year + // matching is enabled, from EDU. @Expose() - hasRoster: boolean | null; + hasNonOdsRoster: boolean | null; } export const toGetTenantSchoolYearConfigDto = makeSerializer( From defa9df01928b712b513b85b6a6fa7605d37a824 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Wed, 3 Jun 2026 17:04:55 -0500 Subject: [PATCH 12/15] fix: throw on missing partner in roster lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveJobDestination and getTenantConfig look up the partner to read crossYearMatchingEnabled. A tenant always has a partner (FK), so a missing row is an invariant violation — not a "cross-year disabled" case. Throw instead of silently treating it as false, so we don't proceed on a broken assumption. The controller (getTenantConfig) throws InternalServerErrorException; the service (resolveJobDestination) throws a plain Error, since services don't throw HTTP exceptions (AGENTS.md convention) — it surfaces as a 500 at the controller boundary. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/api/src/jobs/jobs.service.ts | 7 ++++++- .../school-year-config/school-year-config.controller.ts | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/api/src/jobs/jobs.service.ts b/app/api/src/jobs/jobs.service.ts index cd967f70..2d8d4232 100644 --- a/app/api/src/jobs/jobs.service.ts +++ b/app/api/src/jobs/jobs.service.ts @@ -103,8 +103,13 @@ export class JobsService { where: { id: input.tenant.partnerId }, select: { crossYearMatchingEnabled: true }, }); + if (!partner) { + // Every tenant has a partner (FK) — a missing one is an invariant + // violation, not a "cross-year disabled" case. Don't proceed. + throw new Error(`Partner not found: ${input.tenant.partnerId}`); + } - if (!partner?.crossYearMatchingEnabled) { + if (!partner.crossYearMatchingEnabled) { const rosterKey = rosterFileKey({ partnerId: input.tenant.partnerId, tenantCode: input.tenant.code }, config.schoolYear); const rosterExists = await this.fileService.doesFileExist(rosterKey, this.appConfig.rosterBucket()); if (!rosterExists) { diff --git a/app/api/src/school-year-config/school-year-config.controller.ts b/app/api/src/school-year-config/school-year-config.controller.ts index 9047f33e..d855c221 100644 --- a/app/api/src/school-year-config/school-year-config.controller.ts +++ b/app/api/src/school-year-config/school-year-config.controller.ts @@ -6,6 +6,7 @@ import { Get, Headers, Inject, + InternalServerErrorException, ParseArrayPipe, Put, Res, @@ -44,7 +45,12 @@ export class SchoolYearConfigController { where: { id: tenant.partnerId }, select: { crossYearMatchingEnabled: true }, }); - const crossYearMatchingEnabled = partner?.crossYearMatchingEnabled ?? false; + if (!partner) { + // Every tenant has a partner (FK) — a missing one is an invariant + // violation, not a "cross-year disabled" case. Don't proceed. + throw new InternalServerErrorException(`Partner not found: ${tenant.partnerId}`); + } + const crossYearMatchingEnabled = partner.crossYearMatchingEnabled; const schoolYears = await this.prisma.schoolYear.findMany({ where: { From 301bb5ed7e94989eb7d06a4862fdeb2ced3b4c11 Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 4 Jun 2026 09:57:28 -0500 Subject: [PATCH 13/15] test: tidy cross-year tests; clarify roster-unavailable error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test cleanups from review: - Drop redundant partner resets from finally blocks — the harness reseeds the database before each test, so DB mutations are already isolated. Only jest mock resets need to stay (mocks live on the app instance, which persists across tests). - Remove a duplicate toggle-off rejection test from jobs.spec.ts; the pre-existing "roster file does not exist" test already covers it (partner A defaults to crossYearMatchingEnabled=false) and is now annotated as such. - Retitle the school-year-config tests to name the cross-year condition, and drop the S3-not-called assertion — the behavior that matters is that a missing roster file doesn't matter when the toggle is on, not how it's computed. Also rename the destination code roster_file_missing -> roster_unavailable and extend the error message to "No roster file found and cross-year matching not enabled" — the rejection now means neither roster source is available, not just a missing file. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integration/tests/external-api.v1.spec.ts | 9 ++-- app/api/integration/tests/jobs.spec.ts | 41 +++---------------- .../tests/school-year-config.spec.ts | 16 +++----- .../src/external-api/v1/jobs.v1.controller.ts | 2 +- app/api/src/jobs/jobs.controller.ts | 2 +- app/api/src/jobs/jobs.service.ts | 18 ++++---- 6 files changed, 28 insertions(+), 60 deletions(-) diff --git a/app/api/integration/tests/external-api.v1.spec.ts b/app/api/integration/tests/external-api.v1.spec.ts index 4e7ca21e..f2ce9cdf 100644 --- a/app/api/integration/tests/external-api.v1.spec.ts +++ b/app/api/integration/tests/external-api.v1.spec.ts @@ -308,11 +308,8 @@ describe('ExternalApiV1', () => { expect(job?.odsId).toBeNull(); expect(job?.sendToOds).toBe(false); } finally { + // No partner reset needed — seed data is refreshed before each test doesFileExistMock.mockResolvedValue(true); - await prisma.partner.update({ - where: { id: partnerA.id }, - data: { crossYearMatchingEnabled: false }, - }); } }); }); @@ -577,7 +574,9 @@ describe('ExternalApiV1', () => { }); expect(res.status).toBe(400); - expect(res.body.message).toContain('No roster file found'); + expect(res.body.message).toContain( + 'No roster file found and cross-year matching not enabled' + ); } finally { doesFileExistMock.mockResolvedValue(true); } diff --git a/app/api/integration/tests/jobs.spec.ts b/app/api/integration/tests/jobs.spec.ts index 5511b959..fe04abd2 100644 --- a/app/api/integration/tests/jobs.spec.ts +++ b/app/api/integration/tests/jobs.spec.ts @@ -343,6 +343,8 @@ describe('POST /jobs', () => { expect(job?.sendToOds).toBe(false); }); + // Partner A defaults to crossYearMatchingEnabled=false, so this also + // guards the "cross-year matching disabled" rejection path. it('should reject no-ODS jobs when the roster file does not exist', async () => { await prisma.schoolYearConfig.create({ data: { @@ -366,7 +368,9 @@ describe('POST /jobs', () => { }); expect(res.status).toBe(400); - expect(res.body.message).toContain('No roster file found'); + expect(res.body.message).toContain( + 'No roster file found and cross-year matching not enabled' + ); } finally { doesFileExistMock.mockResolvedValue(true); } @@ -432,40 +436,7 @@ describe('POST /jobs', () => { expect(job?.odsId).toBeNull(); expect(job?.sendToOds).toBe(false); } finally { - doesFileExistMock.mockResolvedValue(true); - await prisma.partner.update({ - where: { id: tenantA.partnerId }, - data: { crossYearMatchingEnabled: false }, - }); - } - }); - - it('should reject a no-ODS job with no roster file when cross-year matching is disabled', async () => { - await prisma.schoolYearConfig.create({ - data: { - partnerId: tenantA.partnerId, - schoolYearId: '2324', - isEnabled: true, - sendToOds: false, - }, - }); - // partner A defaults to crossYearMatchingEnabled=false - - const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; - doesFileExistMock.mockResolvedValue(false); - - try { - const res = await request(app.getHttpServer()) - .post(endpoint) - .set('Cookie', [sessionA.cookie]) - .send({ - ...postJobDto, - schoolYearId: '2324', - }); - - expect(res.status).toBe(400); - expect(res.body.message).toContain('No roster file found'); - } finally { + // No partner reset needed — seed data is refreshed before each test doesFileExistMock.mockResolvedValue(true); } }); diff --git a/app/api/integration/tests/school-year-config.spec.ts b/app/api/integration/tests/school-year-config.spec.ts index 2d29a87c..32fa460a 100644 --- a/app/api/integration/tests/school-year-config.spec.ts +++ b/app/api/integration/tests/school-year-config.spec.ts @@ -340,14 +340,15 @@ describe('GET /school-year-config/tenant', () => { expect(row2526.hasOds).toBe(true); }); - it('should return hasNonOdsRoster=false when roster file does not exist', async () => { + it('should return hasNonOdsRoster=false when roster file does not exist and cross-year matching is not enabled', async () => { const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; doesFileExistMock.mockResolvedValue(false); try { const res = await request(app.getHttpServer()).get(endpoint).set('Cookie', [cookieA]); - // 2526 is seeded as sendToOds=false, so hasNonOdsRoster reflects the S3 check + // 2526 is seeded as sendToOds=false and partner A defaults to + // crossYearMatchingEnabled=false, so hasNonOdsRoster reflects the S3 check const row2526 = res.body.find((r: any) => r.schoolYearId === '2526'); expect(row2526.hasNonOdsRoster).toBe(false); } finally { @@ -355,9 +356,8 @@ describe('GET /school-year-config/tenant', () => { } }); - it('should return hasNonOdsRoster=true for a no-ODS year when cross-year matching is enabled, without an S3 check', async () => { + it('should return hasNonOdsRoster=true for a no-ODS year when cross-year matching is enabled, even with no roster file', async () => { const doesFileExistMock = app.get(FileService).doesFileExist as jest.Mock; - doesFileExistMock.mockClear(); doesFileExistMock.mockResolvedValue(false); await global.prisma.partner.update({ where: { id: partnerA.id }, @@ -374,15 +374,9 @@ describe('GET /school-year-config/tenant', () => { // ODS years stay null regardless of the toggle const row2425 = res.body.find((r: any) => r.schoolYearId === '2425'); expect(row2425.hasNonOdsRoster).toBeNull(); - - // Toggle short-circuits the S3 check entirely - expect(doesFileExistMock).not.toHaveBeenCalled(); } finally { + // No partner reset needed — seed data is refreshed before each test doesFileExistMock.mockResolvedValue(true); - await global.prisma.partner.update({ - where: { id: partnerA.id }, - data: { crossYearMatchingEnabled: false }, - }); } }); }); diff --git a/app/api/src/external-api/v1/jobs.v1.controller.ts b/app/api/src/external-api/v1/jobs.v1.controller.ts index cefe9a2b..214adc2c 100644 --- a/app/api/src/external-api/v1/jobs.v1.controller.ts +++ b/app/api/src/external-api/v1/jobs.v1.controller.ts @@ -112,7 +112,7 @@ export class ExternalApiV1JobsController { school_year_config_missing: `School year is not enabled: ${year}`, school_year_disabled: `School year is not enabled: ${year}`, ods_not_found: `No ODS found for school year: ${year}`, - roster_file_missing: `No roster file found for school year: ${year}`, + roster_unavailable: `No roster file found and cross-year matching not enabled for school year: ${year}`, }; throw new BadRequestException(messages[destination.code]); } diff --git a/app/api/src/jobs/jobs.controller.ts b/app/api/src/jobs/jobs.controller.ts index 0cdc6b27..979ded1b 100644 --- a/app/api/src/jobs/jobs.controller.ts +++ b/app/api/src/jobs/jobs.controller.ts @@ -173,7 +173,7 @@ export class JobsController { school_year_config_missing: `School year is not enabled: ${year}`, school_year_disabled: `School year is not enabled: ${year}`, ods_not_found: `No ODS found for school year: ${year}`, - roster_file_missing: `No roster file found for school year: ${year}`, + roster_unavailable: `No roster file found and cross-year matching not enabled for school year: ${year}`, }; throw new BadRequestException(messages[destination.code]); } diff --git a/app/api/src/jobs/jobs.service.ts b/app/api/src/jobs/jobs.service.ts index 2d8d4232..0eb040ad 100644 --- a/app/api/src/jobs/jobs.service.ts +++ b/app/api/src/jobs/jobs.service.ts @@ -59,7 +59,7 @@ export class JobsService { | 'school_year_config_missing' | 'school_year_disabled' | 'ods_not_found' - | 'roster_file_missing'; + | 'roster_unavailable'; } > { const config = await this.prisma.schoolYearConfig.findUnique({ @@ -96,8 +96,7 @@ export class JobsService { if (!config.sendToOds) { // A no-ODS year is valid if a roster file exists OR the partner has - // cross-year matching enabled (EDU can supply the roster). This is the - // partner setting only — no creds/connection check. + // cross-year matching enabled (EDU can supply the roster). // Short-circuit the S3 check when the toggle is on; we don't need the file. const partner = await this.prisma.partner.findUnique({ where: { id: input.tenant.partnerId }, @@ -110,10 +109,16 @@ export class JobsService { } if (!partner.crossYearMatchingEnabled) { - const rosterKey = rosterFileKey({ partnerId: input.tenant.partnerId, tenantCode: input.tenant.code }, config.schoolYear); - const rosterExists = await this.fileService.doesFileExist(rosterKey, this.appConfig.rosterBucket()); + const rosterKey = rosterFileKey( + { partnerId: input.tenant.partnerId, tenantCode: input.tenant.code }, + config.schoolYear + ); + const rosterExists = await this.fileService.doesFileExist( + rosterKey, + this.appConfig.rosterBucket() + ); if (!rosterExists) { - return { status: 'error', code: 'roster_file_missing' }; + return { status: 'error', code: 'roster_unavailable' }; } } @@ -428,5 +433,4 @@ export class JobsService { return { result: 'JOB_STARTED', job, run }; } - } From b6bcdd86bafd80aa82127aa35e787e59b1fbeb2f Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 4 Jun 2026 10:30:52 -0500 Subject: [PATCH 14/15] fix: source-agnostic roster setup copy; document EDU's two roster roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace "roster needs to be loaded" phrasing with "a roster source needs to be configured" on SetupRequiredPage and the ODS-config page — "loaded" implies an S3 file, but the source may be EDU cross-year matching. - AGENTS.md: the EDU roster entry read as no-ODS-only; it also serves ODS years as the second-pass match for IDs that miss the ODS roster. Also update the stale "roster loaded" badge quote. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 4 ++-- app/fe/src/app/Pages/Home/SetupRequiredPage.tsx | 8 ++++---- app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx | 5 +++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b02b8b73..1d175dbd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -183,10 +183,10 @@ Cross-year-matched rows are never sent to the ODS — they're only made availabl A roster is the student lookup the executor matches input rows against. Source precedence: 1. **ODS** — for `sendToOds` years, the executor fetches the roster from the ODS API. -2. **EDU** — for no-ODS (`sendToOds=false`) years, if `crossYearMatchAvailable`, the executor pulls the roster from EDU (Snowflake) via `appUrls.roster` as NDJSON. EDU is preferred over the S3 file when available (the executor handles this preference). +2. **EDU** — the cross-year roster from EDU (Snowflake), pulled via `appUrls.roster` as NDJSON when `crossYearMatchAvailable`. Two roles: for ODS years, it's the second-pass match for IDs that didn't match the ODS roster (see Cross-Year Matching Flow — those rows are never sent to the ODS); for no-ODS (`sendToOds=false`) years, it's the roster source, preferred over the S3 file (the executor handles this preference). 3. **S3 roster file** — the fallback for no-ODS years when cross-year matching is unavailable (`__rosters/...jsonl`). The app omits `rosterFilePath` from the payload when `crossYearMatchAvailable` is true (it would be a dangling pointer). -A no-ODS year is **selectable** at job creation, and shows **green** ("roster loaded") on the ODS-config page, when a roster file exists **OR** the partner has cross-year matching enabled. This gate is the partner setting only (`crossYearMatchingEnabled`) — no creds/connection check, so a year can be selectable while the executor's `crossYearMatchAvailable` (which also requires `canConnect`) is false; that case fails cleanly at run time per run atomicity. +A no-ODS year is **selectable** at job creation, and shows **green** ("roster available") on the ODS-config page, when a roster file exists **OR** the partner has cross-year matching enabled. This gate is the partner setting only (`crossYearMatchingEnabled`) — no creds/connection check, so a year can be selectable while the executor's `crossYearMatchAvailable` (which also requires `canConnect`) is false; that case fails cleanly at run time per run atomicity. ### S3 Path Structure diff --git a/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx b/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx index 255928a5..9b4ccefc 100644 --- a/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx +++ b/app/fe/src/app/Pages/Home/SetupRequiredPage.tsx @@ -10,8 +10,8 @@ import { tenantSchoolYearConfigQuery } from '../../api'; // user at whatever will unblock them: // - If any year is configured to send to an ODS, guide them to configure // one — that's the only step they can take themselves. -// - Otherwise, they need an admin to either load a roster file or enable -// school years, so point them at support. +// - Otherwise, they need an admin to either configure a roster source or +// enable school years, so point them at support. export const SetupRequiredPage = () => { const { data: yearConfigs } = useSuspenseQuery(tenantSchoolYearConfigQuery); @@ -32,10 +32,10 @@ export const SetupRequiredPage = () => { return ( - Before you can start uploading assessments, a roster must be loaded for your + Before you can start uploading assessments, a roster source must be configured for your district. Please contact support for assistance. - + ); } diff --git a/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx b/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx index 49c7cf75..4a4693a8 100644 --- a/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx +++ b/app/fe/src/app/Pages/Ods/OdsConfigsPage.tsx @@ -228,7 +228,8 @@ const RosterYearContent = ({ hasRoster }: { hasRoster: boolean }) => { {!hasRoster && ( - A roster is required to match student IDs. Contact support to have a roster loaded. + A roster is required to match student IDs. Contact support to have a roster source + configured. )} @@ -245,7 +246,7 @@ const RosterYearContent = ({ hasRoster }: { hasRoster: boolean }) => { {hasRoster ? 'roster available' : 'roster not available'} - {!hasRoster && } + {!hasRoster && } ); From 8494598840fadfbfdda38003169ce0dd63a37e2a Mon Sep 17 00:00:00 2001 From: Andy Kitson Date: Thu, 4 Jun 2026 10:36:49 -0500 Subject: [PATCH 15/15] fix: crossYearMatchAvailable mirrors the partner toggle alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the per-run canConnect check from the executor payload. canConnect is an existence check (creds present) sitting in a health-check position: it can't catch broken-but-present creds — those fail the run at roster fetch anyway — while its false negatives (creds deleted, transient AWS errors during the secret fetch) silently degraded match quality with no signal: ODS-year runs skipped the cross-year pass and sent students to manual resolution; no-ODS runs failed with a misleading S3 404 instead of an honest EDU error. In practice a tenant has either a roster file or an EDU connection, never both, so there was no file fallback for the check to preserve. Once the toggle is on (the admin enable endpoint still requires canConnect), the EDU connection is an assumed dependency like postgres or S3: failures are loud and bounded — the executor only fetches the cross-year roster for runs with unmatched students. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- .../integration/tests/earthbeam-api.spec.ts | 34 +++---------------- .../earthbeam/api/earthbeam-api.service.ts | 13 +++---- 3 files changed, 12 insertions(+), 37 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1d175dbd..6e94e9fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,7 +186,7 @@ A roster is the student lookup the executor matches input rows against. Source p 2. **EDU** — the cross-year roster from EDU (Snowflake), pulled via `appUrls.roster` as NDJSON when `crossYearMatchAvailable`. Two roles: for ODS years, it's the second-pass match for IDs that didn't match the ODS roster (see Cross-Year Matching Flow — those rows are never sent to the ODS); for no-ODS (`sendToOds=false`) years, it's the roster source, preferred over the S3 file (the executor handles this preference). 3. **S3 roster file** — the fallback for no-ODS years when cross-year matching is unavailable (`__rosters/...jsonl`). The app omits `rosterFilePath` from the payload when `crossYearMatchAvailable` is true (it would be a dangling pointer). -A no-ODS year is **selectable** at job creation, and shows **green** ("roster available") on the ODS-config page, when a roster file exists **OR** the partner has cross-year matching enabled. This gate is the partner setting only (`crossYearMatchingEnabled`) — no creds/connection check, so a year can be selectable while the executor's `crossYearMatchAvailable` (which also requires `canConnect`) is false; that case fails cleanly at run time per run atomicity. +A no-ODS year is **selectable** at job creation, and shows **green** ("roster available") on the ODS-config page, when a roster file exists **OR** the partner has cross-year matching enabled. The executor payload's `crossYearMatchAvailable` is the same partner setting (`crossYearMatchingEnabled`) — there is no creds/connection check at run-prep time. The admin enable endpoint requires working EDU creds to turn the toggle on; once on, the EDU connection is an assumed dependency like postgres or S3: if EDU is unavailable mid-run, the run fails loudly at roster-fetch time rather than silently degrading to weaker matching. (In practice a tenant has either a roster file or an EDU connection, not both, so there is no fallback to preserve.) ### S3 Path Structure diff --git a/app/api/integration/tests/earthbeam-api.spec.ts b/app/api/integration/tests/earthbeam-api.spec.ts index f6e371bf..26ac1c05 100644 --- a/app/api/integration/tests/earthbeam-api.spec.ts +++ b/app/api/integration/tests/earthbeam-api.spec.ts @@ -107,31 +107,17 @@ describe('Earthbeam API', () => { }); describe('cross-year ID matching', () => { - // Default state per test: both gates ON (toggle enabled + creds present) - // so the happy path requires no overrides and each negative test reads - // as "remove one condition, expect the flag to flip false." - let getInfoSpy: jest.SpyInstance; - + // crossYearMatchAvailable mirrors the partner toggle alone — there is no + // creds/connection check at run-prep time. EDU problems surface as loud + // run failures at roster-fetch time instead of silent degradation. beforeEach(async () => { await global.prisma.partner.update({ where: { id: partnerA.id }, data: { crossYearMatchingEnabled: true }, }); - const configService = app.get(AppConfigService); - getInfoSpy = jest.spyOn(configService, 'getEduConnectionInfo').mockResolvedValue({ - username: 'snowflake-user', - account: 'example', - database: 'edu_stg', - schema: 'public', - privateKey: Buffer.from('priv'), - }); - }); - - afterEach(() => { - getInfoSpy.mockRestore(); }); - it('sets crossYearMatchAvailable=true and emits appUrls.roster when toggle on and creds exist', async () => { + it('sets crossYearMatchAvailable=true and emits appUrls.roster when the partner toggle is on', async () => { const res = await request(app.getHttpServer()) .get(endpointA) .set('Authorization', `Bearer ${tokenA}`); @@ -157,18 +143,6 @@ describe('Earthbeam API', () => { expect(res.body.appUrls.roster).toBeUndefined(); }); - it('sets crossYearMatchAvailable=false and omits appUrls.roster when creds are missing', async () => { - getInfoSpy.mockResolvedValue(null); - - const res = await request(app.getHttpServer()) - .get(endpointA) - .set('Authorization', `Bearer ${tokenA}`); - - expect(res.status).toBe(200); - expect(res.body.crossYearMatchAvailable).toBe(false); - expect(res.body.appUrls.roster).toBeUndefined(); - }); - it('omits rosterFilePath on a no-ODS job when cross-year matching is available (EDU is the source)', async () => { const authService = app.get(EarthbeamApiAuthService); const noOdsJob = await seedJob({ diff --git a/app/api/src/earthbeam/api/earthbeam-api.service.ts b/app/api/src/earthbeam/api/earthbeam-api.service.ts index e929688e..400d73df 100644 --- a/app/api/src/earthbeam/api/earthbeam-api.service.ts +++ b/app/api/src/earthbeam/api/earthbeam-api.service.ts @@ -30,7 +30,6 @@ import { EventEmitterService, EVENT_EMITTER_SERVICE, } from 'api/src/event-emitter/event-emitter.service'; -import { EduSnowflakePoolService } from './edu-snowflake-pool.service'; @Injectable() export class EarthbeamApiService { @@ -41,8 +40,7 @@ export class EarthbeamApiService { private readonly encryptionService: EncryptionService, private readonly fileService: FileService, private readonly configService: AppConfigService, - @Inject(EVENT_EMITTER_SERVICE) private readonly eventEmitter: EventEmitterService, - private readonly eduPool: EduSnowflakePoolService + @Inject(EVENT_EMITTER_SERVICE) private readonly eventEmitter: EventEmitterService ) {} async earthbeamInputForRun(runId: Run['id']) { @@ -143,9 +141,12 @@ export class EarthbeamApiService { const executorBaseUrl = this.configService.executorCallbackBaseUrl(); - const partnerId = job.tenant.partnerId; - const crossYearEnabled = job.tenant.partner.crossYearMatchingEnabled; - const crossYearMatchAvailable = crossYearEnabled && (await this.eduPool.canConnect(partnerId)); + // The partner toggle alone — no creds/connection check. Once the toggle is + // on (the admin enable endpoint requires working creds to turn it on), the + // EDU connection is an assumed dependency like postgres or S3: if EDU is + // unavailable mid-run, the run fails loudly rather than silently degrading + // to weaker matching. + const crossYearMatchAvailable = job.tenant.partner.crossYearMatchingEnabled; const payload: EarthbeamApiJobResponseDto = { appDataBasePath: `${job.fileProtocol}://${job.fileBucketOrHost}/${job.fileBasePath}`,