chore: implement audit remediation fixes#1153
Conversation
- Fix IPv6 rate limit header parsing - Add bounded similarity scores to semantic rerank - Sync PWA updates across tabs via BroadcastChannel - Add non-blocking backoff lock for test-run-lock - Update vitest configuration and test suite assertions
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR applies audit remediation across client-IP parsing, semantic reranking, PWA update signaling, PDF extraction errors, skill manifest synchronization, live-lock handling, proxy session refresh, and related policy and review records. ChangesAudit remediation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ServiceWorker
participant TabA
participant TabB
ServiceWorker->>TabA: controller change
TabA->>TabA: activate update-ready state
TabA->>TabB: BroadcastChannel sw-updated
TabB->>TabB: activate update-ready state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: 6f87e0ec88
ℹ️ 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".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f87e0ec88
ℹ️ 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".
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #5377 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@cursoragent fix the branch merge conflict please |
Add a regression test that an acquire against a live owner with startedAt older than five minutes still throws and retains the lock, so long-running builds/tests cannot be stolen by a later worktree.
Pin the catalog contract to 32 canonical skills, 8 aliases, and 40 discovered definitions so accidental catalog drift fails the suite.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
@cursoragent resolve the failing CI policy check please |
|
PR policy is green. I couldn't edit the PR description directly, so I added
Latest check on   |
Add PR_POLICY_BODY.md so CI can write the required Clinical Governance Preflight and RAG impact declaration that unblock the PR policy check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32a3780693
ℹ️ 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".
Nudge CI so PR policy re-evaluates the already synced description after the sync/policy race on the prior tip.
|
PR policy is green again. Synced the required description via
Latest PR policy run on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60387f5145
ℹ️ 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".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 732: Move all four PR `#1153` entries in the branch-review ledger to the
end of the file, preserving their existing chronological append order and
content. Do not alter earlier historical entries or the ledger’s append-only
structure.
In `@PR_POLICY_BODY.md`:
- Line 17: Update the Rollback guidance in PR_POLICY_BODY.md to avoid assuming
merge commits or a specific merge strategy. Instruct maintainers to revert the
exact commit(s) introduced by the PR, or explicitly document the repository’s
enforced merge strategy if that is guaranteed.
In `@scripts/sync-skills.mjs`:
- Around line 24-30: Update the YAML generation in the sync-skills script to
serialize skill.name, shortDesc, and defaultPrompt as safely escaped
double-quoted scalars, using JSON.stringify or an equivalent YAML serializer.
Preserve the existing field names and allow_implicit_invocation value while
ensuring quotes, backslashes, and newlines cannot produce invalid YAML.
- Around line 13-18: Update the shortDesc normalization logic in
scripts/sync-skills.mjs to guarantee a minimum length of 25 characters,
including when the original description is empty or only a few characters long.
Replace or extend the current suffix strategy so the final value always remains
within the documented 25–64 character range while preserving the existing
truncation behavior for descriptions over 64 characters.
In `@tests/pdf-extractor.test.ts`:
- Around line 207-219: Update the test around the temporary directory setup
using a try/finally block, and ensure the finally clause calls rm on root with
recursive and force options. Keep the PDF/script creation and extractPdf
assertion in the try block so root is cleaned up regardless of test success or
failure.
In `@tests/test-runner-safety.test.ts`:
- Line 155: Remove the duplicate block-scoped retained declaration in the test
setup, keeping a single JSON.parse result for ownerPath that preserves the
existing token and pid typing and usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 30ce9b73-aa56-4171-9d8f-98219b838f24
📒 Files selected for processing (12)
PR_POLICY_BODY.mddocs/branch-review-ledger.mdpackage.jsonscripts/sync-skills.mjssrc/components/pwa-lifecycle.tsxsrc/lib/extractors/document.tssrc/lib/public-api-access.tssrc/lib/semantic-rerank.tstests/database-skills.test.tstests/pdf-extractor.test.tstests/proxy.test.tstests/test-runner-safety.test.ts
Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Same unblock as #1178: blocking npm audit keys off package-lock/.npmrc only. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Pad skill shortDesc to 25 chars and escape YAML double-quoted values; clean up PDF extractor temp dirs; clarify squash-merge rollback wording. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs: ledger row for merged PR #1153 babysit Co-authored-by: Cursor <cursoragent@cursor.com> * docs: ledger rows for merged PRs #1153 and #1178 Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
#1175 deleted the #1134 leftover, then #1153 reintroduced a sync template. Leaving it on main lets Sync PR policy body overwrite unrelated PR descriptions. Remove it again after descriptions are set. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…mplate cleanup (#1204) * fix(#30): match expected files by distinct document identity The merged #30 fix assigned each expectation to a distinct source position. Positions are not documents: answer.citations carries one entry per cited chunk, so a single combo-titled document arrives several times and could still fill both comparison slots. First-come assignment was also order-dependent -- a document matching two expectations could consume the only source a narrower expectation needed, so the same inputs produced different allHit results depending on expectedFiles order. Dedupe the window by document text and assign sources by maximum bipartite matching, so coverage is both distinct-by-document and order-independent. RAG impact: no retrieval behaviour change -- eval matching only. Also append label rows in place when building labelsByDocument; paging past the 1,000-row cap (#75) means that loop now sees the full label set, where per-row array rebuilds were quadratic. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * chore: remove leftover PR_POLICY_BODY.md after #1153 The sync-pr-policy-body CI job overwrites a pull request description with this file whenever it exists on the head commit. The copy on main holds the audit-remediation body from #1153, so every branch cut from main silently inherits that description -- which is how #1177 lost its own body. Remove it, as #953 did after #939; the job skips cleanly when no template is present. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(issues): record the #30 follow-up and open #80 Drop the stale queue row for #30/#75, which stayed in the recommended queue after both were archived, note the distinct-identity hardening on the #30 archive row, and open #80 to re-test the removed admission-to-discharge widening under approved eval evidence. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): append #1177 SHA correction and this review record Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
2 participants
Summary
RAG impact: no retrieval behaviour change — clamps non-finite/out-of-range similarity into [0,1] for scoring only; in-range scores, comparator key order, and release ranking are unchanged.
Verification
node scripts/run-vitest.mjs run tests/test-runner-safety.test.ts(17/17)node scripts/run-vitest.mjs run tests/database-skills.test.ts(4/4) andnpm run check:skillsnpm run verify:pr-localnot re-run after the latest merge; CI unit/static gates will re-validate on this head.npm run verify:uinot run in this cloud agent session.Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
PR_POLICY_BODY.mdexists so CI Sync PR policy body can write this description (agent token cannot edit the live PR body directly). Safe to delete after policy is green if you do not want the sync template retained.