Repository navigation
fix(release): keep the musl adapter's bootstrap noise off stderr - #2678
Conversation
…tor fails closed A database stamped by an earlier build within user_version = 1 lacks the additive nullable columns that ensureSchema backfills only on write-path opens. The MCP server opens read-only and validates the complete current schema, so a pure-MCP consumer — the post-update Codex plugin, whose first contact with a repo is genie mcp — fail-closes with project-database-unavailable forever, until some CLI command happens to run the backfill. This is the second stacked defect behind the release outage: the Codex native dogfood updates N to T and immediately checks genie_board through MCP, so every platform fails at the sentinel stage. Reproduced offline against run 30214153213 artifacts: N (5.260720.10) creates the DB, T (5.260726.4) CLI reads it fine (write open migrates), T MCP fail-closes; one T CLI touch and T MCP works. Fix: genie mcp now opens through openReadonlyDbHealingStaleSchema — when (and only when) a successful readonly open validates stale, run the same idempotent openDb write-path open every CLI command already performs, then reopen readonly through the full hardened binding path. An absent DB never reaches the write open (no-create contract preserved); foreign, future, or malformed databases are refused by openDb's typed guards and still fail closed — proven by regression tests including a durable-heal check, the databaseBinding target form, and a user_version=99 refusal.
fix(mcp): self-heal an additive-lag schema before the readonly validator fails closed
The dogfood capability probe fails on any stderr byte. Two bootstrap sources inside the adapter violated that before the candidate ever ran: - docker run --pull=always re-resolves the digest-pinned manifest on EVERY invocation and writes pull progress to stderr — the workflow's pre-pull could never help. The pin is a digest, so pull-if-missing already guarantees exact bytes; drop the flag. - apk add fetches the package index over the network and can emit warnings to stderr; only the candidate binary's stderr is meaningful to the probe, so silence the bootstrap entirely (set -e still aborts on real apk failure). Run 30217058060: glibc, linux-arm64, and darwin dogfood all passed with the runner-env fixes; musl was the sole survivor, failing on exactly the pull-progress stderr.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesGenie updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59b228a1a1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… runner-env steps)
…e open Review finding on the additive-lag self-heal: between closing the validated readonly handle and openDb's writable open, nothing pinned the path's identity — a racing substitution could route ensureSchema's DDL into a swapped file or symlink target, detected only after the write by the hardened reopen. Revalidate the exact ProjectDatabaseBinding at the last moment before the write open; the post-heal reopen still re-verifies binding and opened handle before any tool observes a healed database. Regression test pins the class: a binding whose database was substituted after capture is refused (null) and the substituted file is left untouched by any write-path open.
…filesystems The rm-and-recreate substitution let ext4 recycle the freed inode for the replacement file, so the binding identity legitimately matched again and the heal proceeded — the test passed on APFS (which never reuses inode numbers immediately) and failed on Linux CI. Create the substitute while the original still exists (guaranteed distinct inode), then rename it over the original: the realistic atomic-swap shape, deterministic on every filesystem.
The last platform standing
Run 30217058060 (v5.260726.8): with the runner-env fixes live, glibc, linux-arm64, and darwin dogfood all passed — musl was the only failure, on exactly the class this closes.
The dogfood capability probe fails on any stderr byte. Two bootstrap sources inside
scripts/run-musl-dogfood.shviolate that before the candidate binary ever runs:docker run --pull=alwaysre-resolves the manifest on every invocation and writes pull progress to stderr — the workflow's pre-pull step can never help. The image ref is digest-pinned, so pull-if-missing already guarantees exact bytes; the flag added nothing but the failure. Dropped (test updated to pin its absence).apk add --no-cachefetches the package index over the network and can emit warnings to stderr. Only the candidate's stderr is meaningful to the probe — bootstrap output is now fully silenced, withset -estill aborting on real apk failure.Main-first because the dogfood job executes the adapter from the main checkout; dev twin follows immediately so its merge fires the (hopefully fully green) release.
Validation:
bun test scripts/run-musl-dogfood.test.ts3 pass,bash -nclean.Summary by CodeRabbit
Bug Fixes
Tests
Chores