Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 6a884db The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedToo many files! This PR contains 140 files, which is 40 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (140)
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mcansh
changed the title
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
packages | 6a884db | Oct 07 2026, 11:44 PM |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Coverage remains broken and the workspace configuration silently excludes all existing example apps.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Refreshes workspace tooling, test types, CI actions, and an asynchronous rejection assertion.
Changes:
- Upgrades pnpm, TypeScript, Vitest, tsdown, and CI actions.
- Updates HTTP Helmet test typings and setup.
- Correctly awaits temporary-file rejection testing.
| File | Description |
|---|---|
pnpm-workspace.yaml |
Updates workspace and tooling configuration. |
package.json |
Upgrades pnpm configuration. |
packages/http-helmet/vitest.setup.ts |
Uses Vitest-specific DOM matchers. |
packages/http-helmet/tsconfig.json |
Adds Node and matcher types. |
packages/create-temporary-files/src/lib.test.ts |
Awaits rejection assertion. |
.github/workflows/test.yml |
Pins updated test actions. |
.github/workflows/release.yml |
Pins updated release actions. |
.github/workflows/release-preview.yml |
Pins preview workflow actions. |
.github/workflows/release-experimental.yml |
Pins experimental release actions. |
.github/workflows/cleanup-cache.yml |
Pins checkout action. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: 4fb69b38d6
ℹ️ 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".
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: request changes — two fixes needed
What checks out: all 6 pinned action SHAs resolve to their claimed tags (checkout v7.0.1, setup-node v7.0.0, pnpm/action-setup v6.1.0, changesets/action v2.1.2, release-comment-action v0.5.1, codecov-action v7.1.1). allowBuilds is the correct pnpm v11+ replacement for onlyBuiltDependencies, and dropping rolldown from the allow-list is safe (its package has no install/postinstall scripts). The exact CI command pnpm install --recursive --frozen-lockfile --strict-peer-dependencies was run against this branch's lockfile: exit 0 — the packages/hibp importer (for #152) does not trip frozen mode, and strict peers pass (the coverage-v8/vitest mismatch is runtime-only, as documented in the PR body).
1. Blocking: changesets/action v1 → v2 breaks the release workflow
.github/workflows/release.yml bumps the action but keeps the v1-era inputs. Verified against v2.1.2 source:
- v2 calls
validateChangesetsCliVersion()at startup; root declares@changesets/cli: "^2.29.5"→ the action fails immediately with "This version of the Changesets action is designed to work with Changesets CLI v3. Changesets CLI v2 is not supported; use Changesets action v1 instead." — on every run, before doing anything. - Even with CLI v3, v2 hard-throws via
throwOnRenamedInputs()for all five inputs used here:version→version-script,commit→commit-message,title→pr-title,publish→publish-script,createGithubReleases→create-github-releases. - None of the stacked follow-ups (#151 exports, #152 HIBP, #153 TypeDoc) addresses this, so the next push to a
release*branch fails.
Suggestion: keep a pinned v1 SHA for this layer and move the v2 bump to a dedicated changesets-v3 migration (CLI bump + input renames), or do that migration in this PR.
2. Needs decision: workspace glob silently drops apps/
packages: changes apps/**/* → examples/*, but there is no examples/ directory. The three apps (example-react-router-7, example-remix-classic-compiler, example-remix-vite) are no longer workspace members: their deps are gone from the lockfile, pnpm won't link them, and build:apps (--filter "./apps/**") now silently no-ops — verified: "No projects matched the filters", exit 0, so CI won't catch it. The new glob matches the stale npm workspaces field this same diff deletes from package.json, so it looks copied from there. Either keep apps/**/* (or apps/*) or delete/migrate the apps.
Non-blocking notes
test.yml:78still uses unpinnedcodecov/test-results-action@v1while everything else got SHAs (line outside this diff).- Nit: adding
"node"totypesinhttp-helmet/tsconfig.jsonexposes Node globals to the published lib'ssrc/— probably only wanted for tests/setup. - Pre-existing (untouched, FYI): the
commentjob inrelease.yml:79is gated ongithub.repository == 'mcansh/http-helmet', which is never true in this repo; rootdev/dev:vitefilterexample-app/example-vite-app, which match no package names. - Good changes: the awaited
.rejectsinlib.test.tsnow actually asserts; thejest-dom/vitestsetup simplification is correct; the remaining SHA pinning all verifies.
Known/documented and not re-litigated: @vitest/coverage-v8@^3.2.4 vs vitest@^5.0.3 catalog mismatch (stack #150 → #151 → #152 → #153).
big-pickle (free) ｜ 𝕏
There was a problem hiding this comment.
Follow-up review: 3283c00 (oxlint/oxfmt migration) + 4cdc82f (remove semis)
Covers only the two new commits; my previous review (changesets v2 release break, examples/* workspace glob, test-results-action pin, tsconfig node types) stands unchanged.
Verified on a fresh install of this head:
pnpm install --frozen-lockfile --strict-peer-dependencies→ exit 0 (lockfile/catalog in sync, including the newoxfmt/oxlint/@oxlint/plugins/oxlint-tsgolintentries)vitest run→ exit 0, 127 tests / 0 failures (the semicolon removal breaks nothing)oxfmt --check→ exit 0, clean- oxlint config + all five
jsPluginsload and run without crashing
Blocking: pnpm lint fails on the repo this PR ships
pnpm lint (oxlint --fix) exits 1 with 46 errors, and the count is identical after a --fix run — none of them are auto-fixable:
- 16 ×
import/extensions— the config's ownimport/extensions: ["error", "always"]flags this repo's own extensionless relative imports (e.g.packages/vitest-response-matchers/src/index.ts:3) - 14 ×
anti-slop/no-runtime-typeof, 2 ×no-object-parameters, 1 ×require-safety-comment-for-type-assertion, 1 ×no-unknown-parameters, 1 ×no-known-value-widening— the new anti-slop rules flag existing package sources (e.g.packages/url/src/lib/url.ts:26,packages/vitest-response-matchers/src/matchers/*) - 5 ×
remix-typescript/no-typescript-accessibility - 6 ×
typescript/tsconfig-error— invalid tsconfigs inapps/http-helmet/*(the apps already orphaned by this stack's workspace glob)
CI never runs lint/format (pre-existing), so CI stays green — but on a clean checkout pnpm lint and therefore pnpm validate are red. Before this ships, either fix the flagged code, tune the rules/overrides (e.g. import/extensions consistency, rule scope), or scope the lint run so the shipped gate passes. Two related nits: a lint script containing --fix mutates sources under a name CI gates usually treat as read-only (consider lint + lint:fix), and the import/extensions-style rules disagree with the codebase as formatted.
Minor / nits
- Root
package.jsondeclaresoxfmtandoxlintin bothdependenciesanddevDependencies— please dedupe (devDependencies is the conventional home for tooling). .prettierignoreis still present even though prettier (config, plugins,formatscript) was fully removed.
Known/documented and not re-raised: coverage-v8/vitest mismatch, changesets action inputs, apps workspace question.
big-pickle (free) ｜ 𝕏
| ] | ||
| "devDependencies": { | ||
| "@oxlint/plugins": "catalog:", | ||
| "oxfmt": "catalog:", |
There was a problem hiding this comment.
oxfmt and oxlint are declared here in devDependencies and in dependencies (new lines 33-34). Please keep one — devDependencies is the conventional home for tooling.
There was a problem hiding this comment.
Confirmed still present on 56565a7: oxfmt and oxlint each appear in both dependencies and devDependencies. A cleanup is warranted: retain the devDependencies entries and remove the duplicate dependencies entries, then synchronize the lockfile. Leaving this thread unresolved; no code changes made during this comment triage.
Select Node ambient types explicitly and remove deprecated baseUrl settings while retaining relative path aliases. Wrap the streaming abort callback for the timer API. Signed-off-by: Logan McAnsh <logan@mcan.sh>
Use explicit TypeScript paths for package entry points and direct test imports to satisfy the import extension rule. Signed-off-by: Logan McAnsh <logan@mcan.sh>
Replace TypeScript private modifiers with native private fields. Document the tagged-template boundary check so invalid ordinary calls still receive the existing error. Signed-off-by: Logan McAnsh <logan@mcan.sh>
Keep runtime checks at public policy and header boundaries with documented lint exceptions. Type the internal header predicate, simplify undefined filtering, and explain the CSP key assertion. Signed-off-by: Logan McAnsh <logan@mcan.sh>
Use a JSON value contract in the matcher and Vitest augmentation. Report invalid responses through actual values, simplify undefined checks, and handle callbacks that return without throwing. Cover JSON primitives and arrays plus the non-throwing callback path. Validation: all 133 test cases, package typechecks, and oxlint pass. Signed-off-by: Logan McAnsh <logan@mcan.sh>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 748a5c56aa
ℹ️ 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".
Match the workspace coverage plugin to Vitest 5 while keeping the response matcher package on its existing Vitest 3 peer contract. Point ESM package types at the declarations emitted by tsdown. Signed-off-by: Logan McAnsh <logan@mcan.sh>
Remove the redundant root include pattern, which Vitest 5 used to exclude all source files from coverage. Package project configurations still collect all 200 test cases; coverage now contains instrumented source files. Signed-off-by: Logan McAnsh <logan@mcan.sh>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #150 +/- ##
===========================================
+ Coverage 94.23% 100.00% +5.76%
===========================================
Files 19 19
Lines 902 326 -576
Branches 224 82 -142
===========================================
- Hits 850 326 -524
+ Misses 52 0 -52 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d74f7d01a
ℹ️ 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".
Signed-off-by: Logan McAnsh <logan@mcan.sh>
There was a problem hiding this comment.
Important
Key issues need addressing before merge.
Reviewed changes
Reviewed the tooling refresh upgrading pnpm, TypeScript, Vitest, tsdown, and migrating to oxlint/oxfmt with pinned CI actions. The changes are well-structured overall, but several issues from prior automated reviews remain unresolved.
Substantive changes:
- CI/workflows: Pinned GitHub Actions to specific SHAs; however,
changesets/actionwas bumped to v2 while still using v1-era inputs (see inline comment). - Tooling migration: Switched from Prettier to oxlint/oxfmt with new configs;
pnpm lint(oxlint --fix) fails with ~46 auto-unfixable errors (import/extensions, anti-slop rules, tsconfig issues). - Workspace config:
pnpm-workspace.yamlchangesapps/**/*toexamples/*but there is noexamples/directory, dropping the existing apps from the workspace (see inline comment). - Package updates: Added
sourcefields, explicit.tsextensions, refined http-helmet contracts, improved vitest-response-matchers types; tests look correct.
⚠️ Important
Blocking/needs fixing:
changesets/action@v2.1.2with v1 inputs/CLI mismatch — v2 requires Changesets CLI v3 and renamed inputs (version-script,commit-message, etc.). With@changesets/cli@^2.29.5, this will fail at startup. Either pin back to v1 SHA or complete the CLI v3 + input migration.- Workspace glob drops apps —
apps/**/*→examples/*removesexample-react-router-7,example-remix-classic-compiler,example-remix-vitefrom the workspace;build:appsfilter will no-op. Keepapps/**/*or delete/migrate the apps. - Lint gate failures —
pnpm lintexits with errors (import/extensions policy, anti-slop rules, app tsconfigs). Tune rules/overrides or scope lint so the repo's lint target passes.
Non-blocking nits:
- Root
package.jsonhasoxfmt/oxlintduplicated independenciesanddevDependencies. http-helmet/tsconfig.jsonadds"node"types to the lib tsconfig (may only be needed for tests)..prettierignoreremains even though Prettier is removed.- Consider pinning
codecov/test-results-action@v1for consistency.
big-pickle (free) ｜ 𝕏
a29dbe3 to
f499a15
Compare
65ea303 to
0405c28
Compare
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09282b0b08
ℹ️ 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".
|
Review-comment triage against 56565a7 is complete: all 19 inline threads now have an explanation, including the five previously resolved without a reply. Seventeen threads are resolved; two remain open for actual changes:
For the additional notes in the review summaries: keeping .prettierignore is harmless now that no script invokes Prettier; deleting it is optional housekeeping. Pinning codecov/test-results-action@v1 would improve consistency, but is optional and does not prevent this tooling refresh from working. The pre-existing release comment job's mcansh/http-helmet repository condition and stale root dev/dev:vite filters still warrant separate fixes if those paths are intended to work; they are unchanged by this PR and do not reopen the resolved release/setup or workspace-build findings. Current verification: non-mutating oxlint and pnpm 12.9.1 frozen/offline lockfile validation pass; all nine current workspace test-matrix jobs pass. The current build job fails at the React Router example's missing AppLoadContext export, which skips the packed-package runtime/type checks. Workers Builds is also failing. Those checks remain outstanding; resolving the addressed review threads does not mean the PR is ready to merge. No source changes were made for this triage. |
packageManager from non root package.json
5772bee to
d57f279
Compare
d1bd8d7 to
6a884db
Compare
Summary
Refresh the workspace to pnpm 12, TypeScript 7, Vitest 5, and tsdown 0.23, with updated pinned CI actions. Fix HTTP Helmet's Node and Vitest matcher types and await the temporary-files rejection assertion.
The workspace patterns include packages and examples rather than apps. Use explicit source extensions, native private fields, documented input validation, and a JSON value contract for response matchers.
Evidence
Before: CI coverage failed with an incompatible Vitest 3 coverage provider; package validation referenced declaration files that were not emitted.
After: align coverage with Vitest 5 and declaration paths with tsdown output. The response matcher package retains its Vitest 3 peer contract and matching local coverage dependency.
Tooling-layer oxlint passes. Local package builds and strict frozen installation pass.
Full rebased stack: 19 test files, 181 passed cases and 19 expected failures. HIBP library/runtime typechecks and Node, Deno, Bun, and Workers contracts pass.
Coverage now reports instrumented source files after removing the redundant root test include pattern. The final pushed stack passes GitHub build/test workflows on Linux, macOS, and Windows, plus the HIBP runtime workflows.
Remaining external failure: Cloudflare Workers Builds is still failing. Its build-log API returns 403 with the existing local credential, so the deployment cause is unverified.
Merge Danger
Door: two-way
Revert this layer to restore the prior dependency and workflow versions.
Blast Radius: workspace
Major build/test upgrades, changed workspace discovery, and new action versions affect installation and CI across packages. Review the updated remote checks before merging.
Stack: #150 → #151 → #152 → #153. Merge from the bottom after checks pass.
Stack created with GitHub Stacks CLI