Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: luvs01/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
|
✅ Deterministic PR hygiene checks passed. |
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. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Required full-suite validation remains incomplete
Review readiness requires bun run test before approval. The PR reports that run was interrupted, so exact-head readiness remains unverified.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The warm-up child was waited on through Bun.spawnSync, which made the spawn's own timeout the only bound it could honour — and no bound at all when the child or the primitive wedged: while a synchronous spawn blocks, the event loop is dead, so the hook budget and the per-test timeout freeze inside the same wait and nothing reports anything. Run 35511743422's macos 2/2 leg held that shape for eighteen silent minutes inside client-connect.test.ts before the job ceiling cut it and reported cancelled, which the ci gate reads as failure. The bound now lives on the parent's live loop: an asynchronous spawn, SIGKILL at the existing derived deadline, a short reap grace, and the call settles with or without the child's exit or EOF — so a descendant holding the pipes or a child that outlives its kill cannot turn a warm-up into an unbounded wait. A timed-out child now fails the warm-up by name instead of hanging the job. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…electors compactionRoutingKeepsProviderIdentity evaluated override.sourceModel as the raw client selector, so a synthetic --fast/--effort form of a policy or combo selector (ocx/primary--fast) missed resolvePolicyProfileId/resolveComboId and fell through routeConcreteModel to the default provider. The same fallthrough swallowed policy aliases renamed or deleted mid-conversation, since config.routingProfiles is mutated in place. Both cases could wrongly report identity match and let provider-private compaction state or caller credentials cross a backend boundary. Strip synthetic-row suffixes via parseSyntheticRowId before the identity checks, and treat a source that only routes through the default provider as unproven: it can never match a concrete identity. Co-Authored-By: Epinephrine <luvs01@hanmail.net>
|
Fixed in 7e59315. |
|
Fixed in 7e59315 with a fail-closed posture rather than preserving stale aliases: a source selector that reaches |
…compaction-routing-vulnerability
|
Consolidated into lidge-jun#5553 as a single related-function aggregate. Source head: All three unique commits 714119e, aa9b889 and 7e59315 are preserved in e6f9339, f86a534 and 76b40f9 with exact stable patch IDs and original authors. This includes the cold-spawn process-lifecycle follow-up and the synthetic/stale selector identity correction. The dev synchronization merge has no additional resolution delta. The final aggregate preserves all prepared contribution files and passed 604 distinct focused tests, including compaction identity and warm-up lifecycle. Full-suite completion, exact-head hosted CI and security review are explicitly pending on the draft replacement. Closing this duplicate standalone review entry as part of the requested consolidation after verifying coverage. This is not a merge or release claim; remaining integration checks and reviews are tracked on the replacement. Original branches are retained. |
1 participant
Motivation
policy/<id>or configured aliases) from being mistaken as the same provider identity during compaction routing, which could leak provider-private encrypted reasoning or native compaction blobs across backends.Description
compactionRoutingKeepsProviderIdentityby checkingresolvePolicyProfileIdand returningfalsewhen the source is a policy selector, instead of reconstructing identity viarouteConcreteModel.provider/modelon the same provider still returnstrueand keeps native compaction where appropriate.tests/responses/responses-compaction-policy-identity.test.tscovering canonicalpolicy/<id>, configured alias, and a same-provider concrete control case.structure/transports/responses.md) and register the new test inscripts/test-layout/layout.jsonandtests/fixtures/test-layout-expected.json.Testing
bun test tests/responses/responses-compaction-policy-identity.test.tswhich passed (3 pass, 0 fail).bun run typecheckwhich completed successfully.bun run structure:checkwhich passed.bun test tests/test-layout.test.ts tests/test-layout-tooling.test.tswhich passed (18 pass, 0 fail).bun run privacy:scanwhich passed.bun run test:changedcould not run in this checkout due to no resolvable comparison ref; full-suitebun run testencountered unrelated environment/live-server failures and was interrupted, but these failures are external to the focused change and the new regression is green.Codex Task