Stabilize Cloudflare app/subkey test setup diagnostics to unblock deploy workflow#1653
Stabilize Cloudflare app/subkey test setup diagnostics to unblock deploy workflow#1653
Conversation
Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8223c4f00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tests/app.test.ts
Outdated
| expect(createApp.status).toBe(200) | ||
| if (createApp.status !== 200) { | ||
| const body = await createApp.json().catch(() => null) as any | ||
| const isDuplicate = body?.error === 'cannot_create_app' && body?.supabaseError?.code === '23505' |
There was a problem hiding this comment.
The new duplicate guard in setup can never trigger for the /app endpoint, so retries after a partial-success create still fail with 400 and keep the flake. In public/app/post.ts, duplicate inserts are surfaced as error: 'cannot_create_app' with details in moreInfo.error (not supabaseError.code), so checking body?.supabaseError?.code === '23505' will always be false for this path. This means the test still treats duplicate-app races as hard failures instead of idempotent success.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Stabilizes the Cloudflare backend test setup for app/subkey flows by making the app creation step tolerant of duplicate-creation races and adding a bounded retry for subkey creation.
Changes:
- Added “duplicate app” handling during test app creation to make setup idempotent in CI race conditions.
- Added a short retry loop for subkey creation to smooth over transient setup failures.
- Introduced a constant display name for the subkey test app and updated the corresponding assertion.
Comments suppressed due to low confidence (1)
tests/app.test.ts:35
- The duplicate-app detection is checking
body.supabaseError?.code, but API errors produced byquickError/onErrorare shaped like{ error, message, moreInfo }(with anysupabaseErrornested undermoreInfo). As written,isDuplicatewill never be true, so the test will still fail on duplicate creation races. Consider either checkingbody.moreInfo?.supabaseError?.code(if present) /body.moreInfocontents, or more robustly: on non-200, attempt a GET forAPPNAMEand proceed only if it exists; otherwise fail with the response body for debugging.
const body = await createApp.json().catch(() => null) as {
error?: string
supabaseError?: { code?: string }
} | null
const isDuplicate = body?.error === 'cannot_create_app' && body?.supabaseError?.code === '23505'
if (!isDuplicate) {
expect(createApp.status, JSON.stringify(body)).toBe(200)
}
tests/app.test.ts
Outdated
| expect(createSubkey.status).toBe(200) | ||
| const subkeyData = await createSubkey.json() as { id: number } |
There was a problem hiding this comment.
If subkey creation still fails after the retries, expect(createSubkey.status).toBe(200) won’t surface the response body, which makes CI flakes hard to diagnose. Consider capturing/parsing the final response payload once and including it in the assertion message (while also avoiding parsing the same response twice).
| expect(createSubkey.status).toBe(200) | |
| const subkeyData = await createSubkey.json() as { id: number } | |
| const subkeyBody = await createSubkey.json().catch(() => null) as { id: number } | null | |
| expect(createSubkey.status, JSON.stringify(subkeyBody)).toBe(200) | |
| const subkeyData = subkeyBody as { id: number } |
There was a problem hiding this comment.
Updated in a165f9d: the subkey response is now parsed once into subkeyBody, the assertion includes JSON.stringify(subkeyBody) for CI diagnostics, and we avoid parsing the same response twice.
tests/app.test.ts
Outdated
| const body = await createApp.json().catch(() => null) as any | ||
| const isDuplicate = body?.error === 'cannot_create_app' && body?.supabaseError?.code === '23505' |
There was a problem hiding this comment.
This block reintroduces as any and repeats the same body.supabaseError?.code duplicate check. It would be better to keep the response parsing typed (and aligned to the actual { error, message, moreInfo } shape) so the idempotency logic is both correct and consistent with the earlier create-app handling.
| const body = await createApp.json().catch(() => null) as any | |
| const isDuplicate = body?.error === 'cannot_create_app' && body?.supabaseError?.code === '23505' | |
| const body = await createApp.json().catch(() => null) as { error?: string; message?: string; moreInfo?: unknown } | null | |
| const isDuplicate = body?.error === 'cannot_create_app' |
There was a problem hiding this comment.
Addressed in a165f9d: replaced the untyped as any block with a typed { error, message, moreInfo } payload and kept duplicate detection explicit while preserving idempotent setup behavior.
|
@copilot fix merge issue |
Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
Fixed in a165f9d. I resolved the merge-related review issues in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Summary
Deploys were failing on
test:cloudflare:backenddue to a flaky setup path intests/app.test.ts([GET] /app operations with subkey), where retries could race with partially-created resources and produce400/401setup failures.Root cause addressed
Changes made
{ error, message, moreInfo }.moreInfo/message rather than usingany.idbefore using it.Representative snippet
Test plan
bun run test:cloudflare:backendtests/app.test.tspasses consistently, specifically:[GET] /app operations with subkey > should create app and subkey with limited rights[GET] /app operations with subkey > should not access another app with subkeyNote: local targeted execution was attempted, but dependency install was blocked in this environment by network errors while fetching JSR tarballs.
Screenshots
N/A (backend test-only change; no CLI/frontend behavior change).
Checklist
bun run lint:backend && bun run lint.accordingly.
my tests
Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.