test(api): cross-stack CORS preflight + specialRequirements maxLength + stage-ID note#306
Open
WhatIfWeDigDeeper wants to merge 2 commits into
Open
test(api): cross-stack CORS preflight + specialRequirements maxLength + stage-ID note#306WhatIfWeDigDeeper wants to merge 2 commits into
WhatIfWeDigDeeper wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses cross-stack contract gaps found during rails-api spec coverage review by adding shared API coverage for CORS preflights, aligning the OpenAPI contract for specialRequirements max length with existing implementations, and documenting a deliberately non-contractual behavior around stage UUID stability on restore.
Changes:
- Add a shared Jest API test that validates CORS preflight behavior across stacks (
OPTIONS /applications). - Update
specs/core/api/openapi.yamlto setspecialRequirements.maxLengthto 5000 and add a shared API test ensuring 4000 chars are accepted. - Document that stage UUID stability across history restore is intentionally not part of the cross-stack contract.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/CLAUDE.md | Documents a deliberate contract gap (stage UUID stability on restore) and notes the new stack helper flag. |
| tests/api/helpers.ts | Adds expectedAllowedOrigin per stack to drive the new CORS preflight shared test. |
| tests/api/cors.test.ts | New shared Jest test validating preflight returns 2xx and an admitting Access-Control-Allow-Origin. |
| tests/api/application-crud.test.ts | Adds coverage to ensure long specialRequirements payloads are accepted cross-stack. |
| specs/core/api/openapi.yaml | Aligns specialRequirements maxLength with existing stack implementations (5000). |
2 tasks
WhatIfWeDigDeeper
added a commit
that referenced
this pull request
May 6, 2026
- tests/api/helpers.ts: fix swapped expectedAllowedOrigin for express-api (3010 → 3000, ui/ Next.js port) and koa-api (3000 → 3010, react-ui Vite port). Switch hono-api from :5173 to :3030 to match the actual svelte-ui dev port — using :5173 (an unrelated allowed origin) let the test pass vacuously while the real dev UI was blocked. Switch rails-api from :3100 (a magic port that no UI runs on) to :3000 to reflect real client usage. - hono-api/src/index.ts: add http://localhost:3030 to the CORS allowlist — svelte-ui's dev server runs on :3030 (not the Vite default :5173), so without this the dev UI couldn't actually call the API. Existing :5173 and :3000 entries kept for users who override UI_PORT. - specs/core/domain/entities.md, validation-rules.md: bump specialRequirements max from 1000 → 5000 to match the openapi.yaml change in this PR. Eliminates the conflicting source-of-truth in specs/core/. Co-authored-by: copilot-pull-request-reviewer[bot] <copilot-pull-request-reviewer[bot]@users.noreply.github.com>
WhatIfWeDigDeeper
added a commit
that referenced
this pull request
May 6, 2026
…307) ## Summary Resolves the high-severity Remote Code Execution advisory [GHSA-hffm-xvc3-vprc](GHSA-hffm-xvc3-vprc) by bumping the transitive `simple-git` from 3.35.2 → 3.36.0 (via `npm audit fix`). Pulled in via `nuxt → @nuxt/devtools → simple-git`. Other packages in the monorepo do not depend on `simple-git`, so no other lockfiles change. > Note on wording: "patch" in the PR title is used as a verb ("apply a fix") — it does **not** imply a semver patch-level bump. 3.35.2 → 3.36.0 is a minor bump, which is what `npm audit fix` recommends as the smallest available fix for this advisory. CI's `audit:ci:all` step has been failing on this advisory on every PR opened against `main`, blocking merges. This commit was cherry-picked from `feat/understand-codebase-graphs` (commit `f063bf3`) so the fix can land independently of that feature branch's review cycle. ## Test plan - [x] `cd nuxt-api && npx -y audit-ci --config .auditconfig.json` passes locally. - [ ] CI `audit:ci:all` passes on this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Update — additional pre-existing CI breakages folded in (seroval lockfile sync) After pushing the simple-git fix, CI surfaced additional pre-existing main-level breakages that had been masked by the audit failure: `npm ci` was failing in **both** `tanstack-start-ui/` and `react-apollo-ui/` with: ``` npm error Invalid: lock file's seroval@1.5.2 does not satisfy seroval@1.5.4 npm error Invalid: lock file's seroval-plugins@1.5.2 does not satisfy seroval-plugins@1.5.4 ``` `npm install` in each of those directories regenerates their lockfiles to 1.5.4 for both packages. Folded into this PR because (a) they're the same shape of pre-existing main breakage and (b) all three must land before any open PR can pass CI. PRs #305 and #306 are blocked on this PR landing. Commits in this PR: - `9331aa9` fix(deps): bump simple-git in nuxt-api to patch GHSA-hffm-xvc3-vprc - `1ade966` fix(deps): sync tanstack-start-ui lockfile (seroval 1.5.2 → 1.5.4) - `ec44aef` fix(deps): sync react-apollo-ui lockfile (seroval 1.5.2 → 1.5.4) --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…nts at 5000 Partial fix for #302 — three independent items the rails-api spec coverage review surfaced as cross-stack contract gaps. 1) **Shared CORS preflight test (`tests/api/cors.test.ts`)** — every API stack ships its own CORS middleware so the dev UI can call it from a different localhost port, but there was no shared test catching accidental removal or misconfiguration. Sends `OPTIONS /applications` with the configured dev-UI Origin and asserts the response has `Access-Control-Allow-Origin` either echoing the origin or `*`. Adds `expectedAllowedOrigin` per stack to `tests/api/helpers.ts`. 2) **`specialRequirements` maxLength: 1000 → 5000 in openapi.yaml** — every implementation (fastapi, nest-api, hono-api, lambda-api, rails-api) had already drifted to 5000; the contract was the wrong source of truth. Raising it is non-breaking (longer notes, no API consumer impact) and aligns with the sibling `notes` field which is also 5000. New application-crud test posts a 4000-char value and asserts 201, locking the convention going forward. 3) **Stage UUID stability across history restore — documented as deliberately not part of the cross-stack contract** in `tests/CLAUDE.md`. Only rails-api preserves stage IDs across restore (its `ApplicationRestoreService` does `destroy_all` + recreate-with-original-UUID); other stacks legitimately mint new IDs. Clients depending on stable stage IDs should query the history endpoint instead. No shared test added. Verified the new tests against the three currently-running stacks (rails-api, nest-api, yoga-api): cors.test.ts passes for each; application-crud.test.ts goes from 7 → 8 tests, all passing. The remaining items from #302 (terminal-status transition rejection across 8 stacks, salary-range `salary_max >= salary_min` enforcement) are tracked as follow-up PRs because they touch ~10 service implementations each. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- tests/api/helpers.ts: fix swapped expectedAllowedOrigin for express-api (3010 → 3000, ui/ Next.js port) and koa-api (3000 → 3010, react-ui Vite port). Switch hono-api from :5173 to :3030 to match the actual svelte-ui dev port — using :5173 (an unrelated allowed origin) let the test pass vacuously while the real dev UI was blocked. Switch rails-api from :3100 (a magic port that no UI runs on) to :3000 to reflect real client usage. - hono-api/src/index.ts: add http://localhost:3030 to the CORS allowlist — svelte-ui's dev server runs on :3030 (not the Vite default :5173), so without this the dev UI couldn't actually call the API. Existing :5173 and :3000 entries kept for users who override UI_PORT. - specs/core/domain/entities.md, validation-rules.md: bump specialRequirements max from 1000 → 5000 to match the openapi.yaml change in this PR. Eliminates the conflicting source-of-truth in specs/core/. Co-authored-by: copilot-pull-request-reviewer[bot] <copilot-pull-request-reviewer[bot]@users.noreply.github.com>
WhatIfWeDigDeeper
force-pushed
the
fix/openapi-maxlength-and-shared-cors-test
branch
from
a590f0a to
d454ab6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Partial fix for #302 — three independent items the rails-api spec coverage review surfaced as cross-stack contract gaps. The remaining #302 items (terminal-status transition rejection, salary-range ordering) are scheduled as follow-up PRs because they touch ~10 service implementations each.
1. Shared CORS preflight test
tests/api/cors.test.ts— every API stack ships its own CORS middleware so the dev UI can call it from a different localhost port, but there was no shared test catching accidental removal or misconfiguration. SendsOPTIONS /applicationswith the configured dev-UI Origin and asserts the response hasAccess-Control-Allow-Origineither echoing the origin or*. AddsexpectedAllowedOriginper stack totests/api/helpers.ts.2.
specialRequirementsmaxLength bumped to 5000 in openapi.yamlEvery implementation (fastapi, nest-api, hono-api, lambda-api, rails-api) had already drifted to 5000; the contract was the wrong source of truth. Raising it is non-breaking (longer notes, no API consumer impact) and aligns with the sibling
notesfield which is also 5000. Newapplication-crud.test.tstest posts a 4000-charspecialRequirementsand asserts 201, locking the convention.3. Stage UUID stability across restore — documented as not contractual
Added a section to
tests/CLAUDE.mdclarifying that stage UUID stability across history restore is not part of the cross-stack contract. Only rails-api preserves stage IDs (ApplicationRestoreServicedoesdestroy_all+ recreate-with-original-UUID); other stacks legitimately mint new IDs. Clients depending on stable stage IDs should query the history endpoint. No shared test added.Test plan
Access-Control-Allow-Originechoing the configured Origin.cors.test.tsagainst rails-api — passes (1/1).cors.test.tsagainst nest-api — passes (1/1).cors.test.tsagainst yoga-api — passes (1/1).application-crud.test.tsagainst rails-api — 8/8 (was 7, +1 for specialRequirements@4000).application-crud.test.tsagainst nest-api — 8/8.application-crud.test.tsagainst yoga-api — 8/8.🤖 Generated with Claude Code