Dev - #2557
Dev#2557
Conversation
…lation, final aggregate - F20 closed: final post-test read-only comparison, 36/36 skills + 14/14 agent TOMLs byte-identical, zero sync markers - F41 recorded fixed; F42/F43 added as documented-open HIGH follow-ups (.steal debris availability; uninstall retry physical identity) - Merge simulation: merge-tree --write-tree origin/dev HEAD exit 0 (tree 3997028) vs origin/dev 5101fd3; diff --check clean - Final aggregate at d9b0afb: 1,382 pass / 1 skip / 0 fail across 63 files - B4 supersession flipped to SUPERSEDED on F20 evidence - Final-tree seven-lane replay, exact-final-SHA CI, and human approval remain open; F16-F18/F31 still block stable promotion
fix(release): PR #2545 ultra release-gate remediation follow-up
|
Important Review skippedToo many files! This PR contains 250 files, which is 100 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (11)
📒 Files selected for processing (250)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![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: 57e7240377
ℹ️ 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".
| await configureShortcuts(config, false); | ||
| await markSetupComplete(); | ||
| await configureShortcuts(config, false, deps); | ||
| await withSetupLease(deps, () => markSetupComplete()); |
There was a problem hiding this comment.
When a user runs genie setup --shortcuts and accepts installation (or the shortcuts are already installed), configureShortcuts only mutates the in-memory config.shortcuts.tmuxInstalled flag. This branch then calls markSetupComplete(), which reloads the existing config and writes only the setup-complete fields, and returns without saving the mutated config, so shortcuts.tmuxInstalled is never persisted for the section-specific setup path.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant hardening for the Genie agent-sync and Codex integration, including a new remediation ledger for tracking defects, a portable launcher for hook dispatch, and robust versioning/packaging verification. The review comments identified several actionable issues regarding UTF-8 character handling in stream processing, Windows path normalization, and portability of find commands in build scripts, all of which should be addressed to ensure cross-platform stability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| child.stdout.on('data', (/** @type {Buffer | string} */ chunk) => { | ||
| if (outputOverflow) return; | ||
| stdout += chunk.toString(); |
There was a problem hiding this comment.
Accumulating stream chunks by calling chunk.toString() without setting an explicit encoding can lead to multi-byte UTF-8 characters being split across chunk boundaries, resulting in corrupted output (replacement characters ``). Setting child.stdout.setEncoding('utf8') ensures Node's internal `StringDecoder` correctly handles multi-byte characters at chunk boundaries.
child.stdout.setEncoding('utf8');
child.stdout.on('data', (chunk) => {
if (outputOverflow) return;
stdout += chunk;
| lstat: fsApi.lstat || lstatSync, | ||
| realpath: fsApi.realpath || realpathSync.native, | ||
| access: fsApi.access || accessSync, |
There was a problem hiding this comment.
On Windows, fs.realpathSync.native can return a path prefixed with \\?\ (e.g., \\?\C:\Users\...), whereas path.resolve does not include this prefix. This mismatch will cause the canonical path check to fail. Normalizing the \\?\ prefix on Windows ensures the path comparison is robust.
const resolved = inspect.realpath(candidate);
const expected = pathApi.resolve(candidate);
const normResolved = platform === 'win32' ? resolved.replace(/^\\\\?\\/, '') : resolved;
const samePath = platform === 'win32' ? normResolved.toLowerCase() === expected.toLowerCase() : resolved === expected;
| throw new Error(`canonical Genie MCP binary is not a regular file: ${expected}`); | ||
| } | ||
| const canonical = realpathSync(expected); |
There was a problem hiding this comment.
On Windows, fs.realpathSync can return a path prefixed with \\?\ (e.g., \\?\C:\Users\...), whereas path.normalize does not include this prefix. This mismatch will cause the canonical path check to fail and throw an error. Normalizing the \\?\ prefix on Windows ensures the path comparison is robust.
const canonical = realpathSync(expected);
const normCanonical = platform === 'win32' ? canonical.replace(/^\\\\?\\/, '') : canonical;
const equivalentExpected = platform === 'darwin' && expected.startsWith('/') ? `/private\${expected}` : expected;
if (normalize(normCanonical) !== expected && normalize(normCanonical) !== equivalentExpected) {
| -iname 'spec_*.*' -o -iname '*_spec.*' -o \ | ||
| -iname 'test.*' -o -iname 'spec.*' \ | ||
| \) -delete | ||
| } | ||
|
|
||
| assert_no_release_tests() { | ||
| local root="$1" | ||
| local found | ||
| found="$(find "${root}" \( \ | ||
| \( -type d \( \ | ||
| -iname test -o -iname tests -o -iname __test__ -o -iname __tests__ -o \ | ||
| -iname spec -o -iname specs \ |
There was a problem hiding this comment.
BSD find (the default on macOS) does not support the -quit primary, which will cause this check to fail with a syntax error when building on macOS. Replacing -print -quit with -print | head -n 1 provides a portable way to find the first match on both Linux and macOS.
| -iname 'spec_*.*' -o -iname '*_spec.*' -o \ | |
| -iname 'test.*' -o -iname 'spec.*' \ | |
| \) -delete | |
| } | |
| assert_no_release_tests() { | |
| local root="$1" | |
| local found | |
| found="$(find "${root}" \( \ | |
| \( -type d \( \ | |
| -iname test -o -iname tests -o -iname __test__ -o -iname __tests__ -o \ | |
| -iname spec -o -iname specs \ | |
| found="$(find "${root}" \( \ | |
| \( -type d \( \ | |
| -iname test -o -iname tests -o -iname __test__ -o -iname __tests__ -o \ | |
| -iname spec -o -iname specs \ | |
| \) \) -o \ | |
| \( -type f \( \ | |
| -iname '*.test.*' -o -iname '*.spec.*' -o \ | |
| -iname 'test_*.*' -o -iname '*_test.*' -o \ | |
| -iname 'spec_*.*' -o -iname '*_spec.*' -o \ | |
| -iname 'test.*' -o -iname 'spec.*' \ | |
| \) \) \ | |
| \) -print | head -n 1)" |
|
|
||
| assert_release_tree_equal() { | ||
| local source_root="$1" | ||
| local extracted_root="$2" |
There was a problem hiding this comment.
BSD find (the default on macOS) does not support the -quit primary, which will cause this check to fail with a syntax error when building on macOS. Since the output is piped to grep -q ., grep will exit immediately upon the first match, closing the pipe and causing find to terminate. Thus, -quit is redundant and can be safely removed for portability.
| local extracted_root="$2" | |
| if find "${source_root}" "${extracted_root}" -type l -print | grep -q .; then |
| if find "${source_root}" "${extracted_root}" -type l -print -quit | grep -q .; then | ||
| echo "error: release staging/extracted trees must not contain symlinks" >&2 | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
BSD find (the default on macOS) does not support the -quit primary, which will cause this check to fail with a syntax error when building on macOS. Since the output is piped to grep -q ., grep will exit immediately upon the first match, closing the pipe and causing find to terminate. Thus, -quit is redundant and can be safely removed for portability.
| fi | |
| if find "${source_root}" "${extracted_root}" ! -type f ! -type d -print | grep -q .; then |
1 participant
No description provided.