Skip to content

Resolve rename-mapped reconciliation gaps#87

Merged
nisavid merged 7 commits into
mainfrom
nisavid/issue-75-rename-reconciliation
May 26, 2026
Merged

Resolve rename-mapped reconciliation gaps#87
nisavid merged 7 commits into
mainfrom
nisavid/issue-75-rename-reconciliation

Conversation

@nisavid
Copy link
Copy Markdown
Owner

@nisavid nisavid commented May 26, 2026

Summary

Resolve the rename-mapped reconciliation audit for issue #75 in this implementation PR. No standalone audit report is committed; this PR body is the durable ledger.

Closes #75.

Audit Ledger

Method:

  • Audited current upstream/main and prior sync candidates against docs/maintainers/fork-divergences.md, .agents/fork-sync-policy.toml, and docs/maintainers/fork-sync-policy.md.
  • Split the audit across naming/docs, identity/layout/package, and port-integration surfaces.
  • Applied label cleanup separately: issue Audit past fork syncs for rename-mapped missed upstream changes #75 now has enhancement and retained ready-for-agent.

Applied reconciliations:

  • Ported Thorium browser support from the Linux-port upstream's old linux-features/thorium-chrome-plugin shape into a disabled-by-default port-integrations/thorium-chrome-plugin.
  • Extended the launcher native-host manifest writer so enabled port integrations can add safe relative browser manifest locations such as .config/thorium/NativeMessagingHosts.
  • Reconciled RPM runtime dependencies to soname requirements with the local package identity preserved.
  • Added generated build provenance metadata for the app bundle and packaged update-builder, using local officialDmg and portIntegrations terminology.
  • Added a Linux tray-menu Build Information item backed by generated build metadata.
  • Clarified rename-map policy rows for AppImage/native package/updater/polkit/user-local renamed paths.

Candidate dispositions:

  • Already reconciled: official DMG aliases, legacy UPSTREAM_DMG_* compatibility, CODEX_UPSTREAM_APP_DIR compatibility, and inspect-upstream alias behavior.
  • Deliberate divergence: local codex-app / codex-app-updater identity and local install layout remain authoritative.
  • Deliberate divergence: Thorium support remains disabled by default because it is narrower than the tested core browser set.
  • Deliberate divergence: packaged update-builder omits checkout-local port-integration config while carrying source provenance.
  • Policy-map gap: renamed package/updater/polkit/user-local path rows are now explicit in fork-divergences.md.

Security/design review:

  • Security-sensitive surfaces reviewed: browser native-messaging manifest paths, Chrome/Thorium browser detection, packaged update-builder source provenance, and build metadata display.
  • Extra native-host manifest paths reject blanks, comments, absolute paths, and .. traversal.
  • Source provenance strips URL-form credentials, including HTTPS and SSH URLs, and rejects file/relative local remotes.
  • Visible build-info UI was kept to a standard tray item and information dialog; no new frontend surface or styling system was introduced.

Verification

  • node --test scripts/patch-linux-window-ui.test.js - pass, 158 tests.
  • node --test port-integrations/thorium-chrome-plugin/test.js - pass, 6 tests.
  • bash -n port-integrations/thorium-chrome-plugin/stage.sh - pass after Greptile review fix.
  • node --test --test-name-pattern "build info strips credentials" scripts/patch-linux-window-ui.test.js - failed before the sanitizer fix for repo.git, then passed after rejecting plain local-path remotes.
  • bash tests/scripts_smoke.sh - failed before the manifest path normalization fix for . entries, then passed after rejecting current-directory manifest paths.
  • bash -n launcher/start.sh.template tests/scripts_smoke.sh port-integrations/thorium-chrome-plugin/stage.sh - pass after CodeRabbit/Codex review fixes.
  • bash tests/scripts_smoke.sh - failed before the package source-info sanitizer fix for foo/bar.git, then passed after rejecting local-path remotes in packaged update-builder metadata.
  • bash -n scripts/lib/package-common.sh tests/scripts_smoke.sh - pass after the package source-info sanitizer fix.
  • node --test --test-name-pattern "build info preserves unknown dirty state" scripts/patch-linux-window-ui.test.js - failed before the dirty-state fix, then passed after preserving dirty: null when git status is unavailable.
  • node --test scripts/patch-linux-window-ui.test.js - pass, 159 tests after dirty-state coverage.
  • node --test port-integrations/thorium-chrome-plugin/test.js - failed before the manifest marker composition fix, then passed after preserving existing native-host manifest marker entries while appending Thorium.
  • node --test --test-name-pattern "adds Linux build information to the tray menu" scripts/patch-linux-window-ui.test.js - failed before the tray-detail provenance fix, then passed after rendering dirty: null as an unknown dirty state instead of a clean-looking short commit.
  • bash -n port-integrations/thorium-chrome-plugin/stage.sh - pass after manifest marker append/de-dupe fix.
  • node --test --test-name-pattern "build info keeps dirty suffixes" scripts/patch-linux-window-ui.test.js - failed before the short provenance fix, then passed after preserving -dirty in short source revisions.
  • node --test --test-name-pattern "build information tray detail prefers readable dirty-unknown revisions" scripts/patch-linux-window-ui.test.js - failed before the CodeRabbit review fix, then passed after preferring the readable short revision while still labeling dirty state unknown.
  • bash -n scripts/lib/package-common.sh - pass after aligning packaged update-builder source-info short revisions.
  • node --test scripts/patch-linux-window-ui.test.js - pass, 161 tests after readable dirty-unknown provenance coverage.
  • bash tests/scripts_smoke.sh - pass, All script smoke tests passed.
  • bash -n install.sh launcher/start.sh.template scripts/lib/package-common.sh scripts/lib/build-info.sh port-integrations/thorium-chrome-plugin/stage.sh tests/scripts_smoke.sh - pass.
  • git diff --check and git diff --cached --check - pass.
  • bash tests/scripts_smoke.sh > /tmp/codex-app-linux-smoke.log 2>&1 - pass, All script smoke tests passed.
  • stat -c '%n %Y %y' Codex.dmg - cached official DMG from 2026-05-26 03:04:17.212768856 -0400.
  • env npm_config_cache=/tmp/codex-app-linux-npm-cache NPM_CONFIG_CACHE=/tmp/codex-app-linux-npm-cache make build-app - pass; generated build info and patched app.asar.
  • rg -n -o "codexLinuxShowBuildInfo|Build Information|Codex App Linux build information|Official OpenAI DMG SHA256" /tmp/codex-app-asar-review-issue75-final.GuAiP7/.vite/build/main-*.js - confirms generated main bundle contains the build-info tray/dialog strings.
  • node -e '...' codex-app/.codex-linux/build-info.json - confirms official app version 26.519.41501, SHA256 3101c05dfd40314115418c4ad87edee02419d097f7164c677ddbdd4559ad8a42, source provenance git, and enabled port integrations.
  • cargo check -p codex-app-updater - pass.
  • scripts/build-pacman.sh > /tmp/codex-app-linux-pacman.log 2>&1 - pass; built dist/codex-app-26.519.41501-1-x86_64.pkg.tar.zst.
  • rg -n "Name :|Version :|Depends On :|/opt/codex-app/.codex-linux/build-info.json|update-builder/scripts/lib/build-info.js|update-builder/port-integrations/thorium-chrome-plugin|Built package" /tmp/codex-app-linux-pacman.log - confirms package metadata plus build-info and Thorium update-builder payload.
  • scripts/build-rpm.sh - blocked by host prerequisite: rpmbuild is required (install rpm-build). RPM spec coverage is included in smoke assertions.
  • Security grep over touched browser/provenance surfaces found only sanitizer code and fake credential fixtures used by tests.
  • rg -n "chrome-native-host-manifest-paths|thorium/NativeMessagingHosts|Chrome plugin not found" port-integrations/thorium-chrome-plugin launcher/start.sh.template tests/scripts_smoke.sh - confirms the manifest marker is written only by the Thorium stage hook and consumed by the launcher safe-path reader.

Ralph Review

  • Cycle 1 found two issues: the Thorium manifest-path marker was not consumed by the launcher, and the build-info tray patch skipped the current generated tray-menu shape. Both were fixed with tests.
  • Cycle 2 found one issue: URL-form SSH source remotes with credentials were not sanitized. Fixed in both build-info paths with tests.
  • Cycle 3 reviewed the exact staged commit candidate and found no new findings.
  • Cycle 4 handled Greptile review: the Thorium stage hook now checks for the bundled Chrome plugin before advertising the extra Thorium native-host manifest path, and the missing-plugin branch has a regression test. Refreshed thread-aware PR state showed zero unresolved threads.
  • Cycle 5 handled CodeRabbit and Codex Connector review: plain local-path git remotes are now rejected from build metadata; extra native-host manifest paths are normalized and reject current-directory entries; the smoke assertion now checks the actual absolute-path sink; Thorium docs distinguish checkout-local and installed-user port integration config; the build-info fixture uses a port-integrations.json override path.
  • Cycle 6 handled the follow-up Codex Connector review: packaged update-builder source-info now applies the same local-path remote rejection as generated app build metadata, with smoke coverage for foo/bar.git.
  • Cycle 7 handled the final Codex Connector review: override-only build provenance now preserves unknown git cleanliness as dirty: null instead of reporting a false clean state.
  • Cycle 8 handled the final Codex Connector review: the Thorium stage hook now appends its native-host manifest marker without replacing other integrations' entries, and the build-info tray detail preserves the full revision plus an explicit unknown-dirty-state label when provenance cleanliness is unavailable.
  • Cycle 9 handled CodeRabbit review: build metadata short revisions now preserve a -dirty suffix, the tray detail prefers that readable short revision for dirty-unknown builds, and packaged update-builder source-info uses the same short-revision logic.

Summary by CodeRabbit

  • New Features

    • Optional Thorium browser integration (disabled by default) with improved native-host discovery and plugin patching.
    • "Build Information" tray menu showing packaged app/build metadata.
  • Documentation

    • Updated maintainer rename/compatibility map and Thorium integration setup guide.
  • Chores

    • Installer writes build metadata during install; build provenance/timestamp tracking improved for reproducible builds and RPM dependency handling.
  • Tests

    • Expanded integration and packaging smoke tests covering new flows.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds build-info generation and staging, flake/installer wiring for reproducible source metadata, a Linux tray patch to display build metadata, a Thorium Chrome-plugin integration with patcher and stage hook, launcher app-local native-host manifest support, RPM ELF-suffixed .so Requires, docs updates, and expanded tests.

Changes

Build Reproducibility and Metadata

Layer / File(s) Summary
Build metadata generation and helpers
scripts/lib/build-info.js
New Node.js module generating build metadata (schema, generatedAt from SOURCE_DATE_EPOCH, app identity, DMG size/SHA256, macOS app version, sanitized git provenance, linux target, packageProfile, enabled port integrations) with CLI and exported helpers.
Build info Bash integration
scripts/lib/build-info.sh
New write_build_info() Bash wrapper that invokes the Node build-info script and writes JSON artifacts into the install layout.
Flake and installer wiring
flake.nix, install.sh
flake.nix exposes flakeSourceCommit/flakeSourceDateEpoch into install environment; installer sources scripts/lib/build-info.sh and calls write_build_info during main install.
Source info staging for update-builder
scripts/lib/package-common.sh
Probe for managed Node runtime, copy build-info.js/build-info.sh into staged update-builder scripts, and write sanitized .codex-linux/source-info.json during bundle staging.
Linux tray menu build information display
scripts/patches/main-process.js, scripts/patch-linux-window-ui.js, scripts/patches/core/all-linux/main-process/window-shell/patch.js
New patch injects a “Build Information” tray menu item that reads codex-linux-build-info.json and displays formatted metadata via Electron dialog; patch exported and registered for Linux main-bundle.

Thorium Chrome Plugin Integration

Layer / File(s) Summary
Thorium integration metadata and documentation
port-integrations/thorium-chrome-plugin/integration.json, port-integrations/thorium-chrome-plugin/README.md
New integration manifest and README describing optional Thorium Chrome plugin integration, enablement, runtime changes, and tests.
Thorium extension status patch
port-integrations/thorium-chrome-plugin/patch.js
Patch module that rewrites bundle strings to introduce Thorium identifiers/paths and exports a main-bundle patch descriptor.
Thorium Chrome plugin patcher
port-integrations/thorium-chrome-plugin/patch-chrome-plugin.js
Node CLI performing targeted replacements across plugin scripts to add Thorium native-host manifest paths, profile roots, browser inventory, running-process detection, extension-install fallbacks, and Thorium-specific open-window command selection.
Thorium stage hook and execution
port-integrations/thorium-chrome-plugin/stage.sh
Bash stage hook writing Thorium native-messaging-hosts path into manifest-paths and running the patcher; skips gracefully if plugin missing.
Thorium integration tests
port-integrations/thorium-chrome-plugin/test.js
Unit and integration tests covering integration enablement, patch application, stage-hook effects, missing-plugin behavior, and regression handling for minified shapes.

Chrome Native Host Manifest Path Configuration

Layer / File(s) Summary
Customizable native host manifest paths
launcher/start.sh.template
Embedded Python native-manifest writer now receives app_dir, reads app_dir/.codex-linux/chrome-native-host-manifest-paths with filtering, and writes generated manifests into each configured relative location when content differs.

RPM Architecture-Specific Dependencies

Layer / File(s) Summary
ELF dependency suffix macro and Requires
packaging/linux/codex-app.spec
Adds %global codex_elf_suffix with arch override and replaces generic package Requires with concrete lib*.so%{codex_elf_suffix} entries.

Documentation and Test Coverage

Layer / File(s) Summary
Fork divergences documentation updates
docs/maintainers/fork-divergences.md
Documents additional upstream→local rename mappings for AppImage desktop entry, native packaging names, updater services and scripts, privileged update-policy file, and user-local install experiment artifacts.
Patch exports and unit tests
scripts/patch-linux-window-ui.test.js, scripts/patch-linux-window-ui.js
Exports new tray patch helper and adds unit tests for build-info generation, git-remote sanitization, packageProfile behavior, and tray menu injection; updates expected default patch descriptor list.
Smoke test coverage expansion
tests/scripts_smoke.sh
Smoke tests updated to assert staged build-info scripts and source-info.json, flake source metadata exposure, RPM .so Requires checks, and expanded native-host-manifest behavior including Thorium and ignored paths.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • nisavid/codex-app-linux#12: Both PRs update the fork divergence documentation inventory in docs/maintainers/fork-divergences.md.
  • nisavid/codex-app-linux#42: Related changes to the Linux build-info tray patch pipeline and registration/export of the tray patch helper.

"A rabbit wrote the build and left a note,
Tucked thorium paths in every coat,
Manifests and trays now show their lore,
ELF suffixes whispered through the core,
Hops of tests and docs — hooray, I wrote!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Resolve rename-mapped reconciliation gaps' directly reflects the main purpose of the PR—addressing reconciliation gaps identified in the audit of rename-mapped changes across the codebase.
Linked Issues check ✅ Passed The PR successfully addresses issue #75 by conducting a comprehensive audit, documenting reconciliations including Thorium integration, build provenance metadata, RPM dependencies, native-host manifest handling, and tray menu additions, while expanding the rename-map policy in fork-divergences.md.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the reconciliation objectives: Thorium integration, build metadata infrastructure, RPM spec updates, manifest handling, tray UI, and documentation updates all address the identified rename-mapping gaps.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nisavid nisavid marked this pull request as ready for review May 26, 2026 07:22
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 26, 2026

Greptile Summary

This PR resolves the rename-mapped reconciliation audit (#75) by porting Thorium browser support as a disabled-by-default port integration, adding a "Build Information" tray menu item backed by generated provenance metadata, reconciling RPM runtime deps to soname requirements, and clarifying fork-divergence policy rows for renamed paths.

  • Thorium integration: stage.sh correctly gates the manifest-paths write behind a chrome-plugin existence check; launcher/start.sh.template reads extra manifest locations with full path-safety validation; patch-chrome-plugin.js patches the bundled plugin to detect Thorium.
  • Build provenance: build-info.js generates metadata (DMG hash, source commit, port integrations, distro profile) written at install time and displayed in a new tray dialog; sanitizeGitRemoteUrl strips URL-form credentials and rejects local-path remotes.
  • RPM spec: Requires replaced package names with soname + ELF-suffix expressions for correct cross-distro resolution.

Confidence Score: 4/5

Safe to merge with one inaccuracy in the build-info tray display: clean working-tree builds will always show "dirty state unknown" rather than a bare revision hash.

The runGit helper returns null for both git command failures and successful-but-empty output (a clean working tree). Because git status --porcelain produces no output on a clean tree, dirty in sourceInfoFromGit can only ever be null or true — never false. Every normal clean checkout build therefore shows "dirty state unknown" in the tray dialog instead of just the revision hash. The rest of the PR — Thorium staging, path-safety validation, credential sanitization, RPM soname deps, and flake provenance — is well-tested and correct.

scripts/lib/build-info.js — specifically the runGit function and how dirty is derived from its return value in sourceInfoFromGit.

Important Files Changed

Filename Overview
scripts/lib/build-info.js New module generating build provenance metadata; contains a P1 bug where runGit returns null for both clean-tree (empty stdout) and git failures, causing dirty: null on every clean build and "dirty state unknown" in the tray menu.
scripts/lib/package-common.sh Adds stage_update_builder_source_info with an inline Node heredoc that duplicates sanitizeGitRemoteUrl / shortSourceCommit from build-info.js; carries a maintenance risk if the two copies diverge.
scripts/patches/main-process.js Adds applyLinuxBuildInfoTrayPatch that injects build-info helpers and a "Build Information" tray item into the Electron main bundle; idempotency, variable-name resolution, brace-matching, and class-insertion ordering all look correct.
port-integrations/thorium-chrome-plugin/stage.sh Stage hook correctly checks for the bundled Chrome plugin before writing the manifest-paths entry, and uses grep de-duplication; previously-flagged ordering issue is resolved.
launcher/start.sh.template Extends write_chrome_native_host_manifests to read extra manifest locations from .codex-linux/chrome-native-host-manifest-paths; path validation correctly rejects blanks, comments, absolute paths, .. traversal, and current-directory entries.
packaging/linux/codex-app.spec Switches RPM Requires from package names to soname dependencies with a ()(64bit) ELF suffix for the listed 64-bit architectures; more portable and correctly resolved by RPM dependency solver.
flake.nix Exposes flakeSourceCommit and flakeSourceDateEpoch from flake self-metadata so reproducible builds carry real source provenance instead of the static epoch 1.
tests/scripts_smoke.sh Adds smoke coverage for build-info files in the update-builder bundle, source-info credential sanitization (SSH and local-path remotes), manifest-path de-duplication, and the RPM soname/ELF-suffix assertions.

Sequence Diagram

sequenceDiagram
    participant install as install.sh
    participant bish as build-info.sh
    participant bijs as build-info.js
    participant git as git
    participant fs as filesystem

    install->>bish: write_build_info(dmg_path, app_dir)
    bish->>bijs: node build-info.js args
    bijs->>git: rev-parse --is-inside-work-tree
    git-->>bijs: true/null
    bijs->>git: status --porcelain
    git-->>bijs: empty string (clean) OR output (dirty) OR null (error)
    Note over bijs: runGit() collapses empty stdout to null
    Note over bijs: dirty = null for BOTH clean trees AND git errors
    bijs->>fs: readFileSync(Codex.dmg) sha256
    bijs->>fs: writeFileSync(build-info.json)

    participant pkg as package-common.sh
    participant updater as update-builder/

    pkg->>updater: cp build-info.js
    pkg->>updater: stage_update_builder_source_info()
    Note over pkg: Inline Node heredoc duplicates sanitizeGitRemoteUrl

    participant tray as Electron tray (patched)
    participant dialog as dialog

    tray->>fs: readFileSync(build-info.json)
    tray->>dialog: showMessageBox(detail)
    Note over tray: dirty===null shows dirty state unknown even for clean builds
Loading

Fix All in Codex

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
scripts/lib/build-info.js:19-26
**Clean working trees reported as "dirty state unknown"**

`runGit` returns `null` for two distinct cases: git command failure (nonzero exit) and git command success with empty output. For `git status --porcelain`, an empty output means the tree is clean — but `runGit` collapses this to `null`, the same sentinel used for errors. Consequently `dirty` in `sourceInfoFromGit` is always `null` or `true`; it is never `false`. Every clean checkout build will show "dirty state unknown" in the tray menu instead of showing the bare revision hash.

The fix is to compute `dirty` without going through `runGit`: call `spawnSync` directly for the status check so exit-code-0-with-empty-stdout (clean) can be distinguished from a nonzero exit (unknown).

### Issue 2 of 2
scripts/lib/package-common.sh:508-616
**Credential-sanitization logic duplicated from `build-info.js`**

`sanitizeGitRemoteUrl`, `shortSourceCommit`, and `isoTimestamp` are pasted verbatim into the heredoc here. `build-info.js` is already copied into the update-builder bundle (`update_builder_root/scripts/lib/build-info.js`) earlier in `stage_update_builder_bundle`, so it exists on disk before `stage_update_builder_source_info` runs. Any future change to the sanitizer in `build-info.js` that is not mirrored in this heredoc could silently let credentials through in the packaged update-builder's `source-info.json`.

Reviews (7): Last reviewed commit: "fix(build-info): keep dirty short revisi..." | Re-trigger Greptile

Comment thread port-integrations/thorium-chrome-plugin/stage.sh Outdated
@coderabbitai coderabbitai Bot added the github_actions Pull requests that update GitHub Actions code label May 26, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27e95666db

ℹ️ 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".

Comment thread scripts/lib/build-info.js Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 `@launcher/start.sh.template`:
- Around line 758-773: The code currently accepts bare "." (and equivalents)
from the extra_locations file; normalize each relative entry and reject
current-directory forms before deduping. After computing relative and pure_path
in the extra_locations read loop, normalize the entry (e.g., via
pathlib.PurePosixPath(relative).as_posix() or os.path.normpath on the POSIX
string), skip if the normalized value is "" or "." (or begins with "./" that
normalizes to "."), continue to reject absolute or ".." as already done, and
then use the normalized string when checking/adding to manifest_locations so "."
will not be appended to manifest_locations.

In `@port-integrations/thorium-chrome-plugin/README.md`:
- Line 8: The README currently instructs users to modify
port-integrations/integrations.json but the correct documented config filename
is port-integrations.json; update the enablement instructions in
thorium-chrome-plugin/README.md to reference port-integrations.json (and any
related mentions of the implementation path or env vars) so they match the
canonical names used elsewhere (port-integrations/, port-integrations.json,
CODEX_PORT_INTEGRATIONS_*); ensure any example commands or copy text replace
"port-integrations/integrations.json" with "port-integrations.json".

In `@scripts/patch-linux-window-ui.test.js`:
- Around line 290-292: Replace the test fixture file name "integrations.json"
with the canonical "port-integrations.json" (and any corresponding writes) so
the test uses the real path/config naming; update the fs.writeFileSync call that
currently writes path.join(integrationsRoot, "integrations.json") (and the
matching enabled value for "zed-opener") to write path.join(integrationsRoot,
"port-integrations.json") instead, and likewise ensure any other occurrence in
the same test (e.g., the call around line 302) is updated; keep the
integrations.example.json write as-is if intended but ensure all integration
config references use the "port-integrations.json"/port-integrations convention
and the integrationsRoot variable.

In `@tests/scripts_smoke.sh`:
- Around line 3430-3436: The test injects an absolute path (/tmp/ignored) into
"$app_dir/.codex-linux/chrome-native-host-manifest-paths" but the assertion
later checks for "$home_dir/tmp/ignored" (wrong target); update the assertion in
tests/scripts_smoke.sh that verifies absolute-path rejection to check for the
actual absolute path "/tmp/ignored" (or assert both "/tmp/ignored" and
"$home_dir/tmp/ignored") under the same manifest file/installed paths (use the
existing variables app_dir and home_dir and the manifest path
".codex-linux/chrome-native-host-manifest-paths") so the test fails if an
absolute path is erroneously accepted and written as absolute.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b7627df8-35c7-4bca-b74b-487ccac9f8e5

📥 Commits

Reviewing files that changed from the base of the PR and between 6ea6c37 and 27e9566.

📒 Files selected for processing (19)
  • docs/maintainers/fork-divergences.md
  • flake.nix
  • install.sh
  • launcher/start.sh.template
  • packaging/linux/codex-app.spec
  • port-integrations/thorium-chrome-plugin/README.md
  • port-integrations/thorium-chrome-plugin/integration.json
  • port-integrations/thorium-chrome-plugin/patch-chrome-plugin.js
  • port-integrations/thorium-chrome-plugin/patch.js
  • port-integrations/thorium-chrome-plugin/stage.sh
  • port-integrations/thorium-chrome-plugin/test.js
  • scripts/lib/build-info.js
  • scripts/lib/build-info.sh
  • scripts/lib/package-common.sh
  • scripts/patch-linux-window-ui.js
  • scripts/patch-linux-window-ui.test.js
  • scripts/patches/core/all-linux/main-process/window-shell/patch.js
  • scripts/patches/main-process.js
  • tests/scripts_smoke.sh

Comment thread launcher/start.sh.template
Comment thread port-integrations/thorium-chrome-plugin/README.md Outdated
Comment thread scripts/patch-linux-window-ui.test.js
Comment thread tests/scripts_smoke.sh Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df36c8649a

ℹ️ 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".

Comment thread scripts/lib/package-common.sh
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
scripts/lib/build-info.js (1)

84-88: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Whitelist staged source-info fields before packaging them.

sanitizeSourceInfo() still preserves arbitrary keys, and sourceInfo() then spreads them into the final build-info payload. Since this metadata is now surfaced in the tray “Build Information” UI, any unexpected fields added to .codex-linux/source-info.json will be shipped verbatim and can leak builder-local data. Limit the staged object to the known provenance fields instead of copying everything through.

Proposed fix
 function sanitizeSourceInfo(info) {
-  const { sourceInfoPath, ...sanitized } = info;
-  void sourceInfoPath;
-  sanitized.remote = sanitizeGitRemoteUrl(sanitized.remote);
-  return sanitized;
+  return {
+    commit: typeof info.commit === "string" ? info.commit : null,
+    shortCommit: typeof info.shortCommit === "string" ? info.shortCommit : null,
+    branch: typeof info.branch === "string" ? info.branch : null,
+    remote: sanitizeGitRemoteUrl(info.remote),
+    describe: typeof info.describe === "string" ? info.describe : null,
+    dirty: typeof info.dirty === "boolean" ? info.dirty : null,
+    capturedAt: typeof info.capturedAt === "string" ? info.capturedAt : null,
+  };
 }

Also applies to: 114-117

🤖 Prompt for 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.

In `@scripts/lib/build-info.js` around lines 84 - 88, sanitizeSourceInfo currently
spreads all keys from info which can leak arbitrary staged metadata; change it
to construct and return a new object that only includes an explicit whitelist of
expected provenance fields (e.g., gitSha, branch, remote, buildHost, buildUser,
buildTimestamp) rather than ...sanitized, then set sanitized.remote =
sanitizeGitRemoteUrl(sanitized.remote) (or sanitize the remote when building the
whitelisted object); likewise update the other occurrence around the 114-117
region so both places only copy the known fields and ignore sourceInfoPath.
🤖 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.

Outside diff comments:
In `@scripts/lib/build-info.js`:
- Around line 84-88: sanitizeSourceInfo currently spreads all keys from info
which can leak arbitrary staged metadata; change it to construct and return a
new object that only includes an explicit whitelist of expected provenance
fields (e.g., gitSha, branch, remote, buildHost, buildUser, buildTimestamp)
rather than ...sanitized, then set sanitized.remote =
sanitizeGitRemoteUrl(sanitized.remote) (or sanitize the remote when building the
whitelisted object); likewise update the other occurrence around the 114-117
region so both places only copy the known fields and ignore sourceInfoPath.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f41eeb6c-7de0-49e3-aec8-e3b86aa00112

📥 Commits

Reviewing files that changed from the base of the PR and between 27e9566 and df36c86.

📒 Files selected for processing (5)
  • launcher/start.sh.template
  • port-integrations/thorium-chrome-plugin/README.md
  • scripts/lib/build-info.js
  • scripts/patch-linux-window-ui.test.js
  • tests/scripts_smoke.sh

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 26, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ebb5e5bf39

ℹ️ 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".

Comment thread scripts/lib/build-info.js Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
scripts/lib/package-common.sh (1)

517-599: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing error handling for embedded Node.js script failure.

If the heredoc Node.js script fails (e.g., fs.writeFileSync throws due to disk full or permission issues), the non-zero exit code is silently ignored since there's no set -e in this file and no explicit || error ... after the heredoc. This could produce an incomplete staged bundle without alerting the packager.

Proposed fix
 fs.writeFileSync(infoFile, `${JSON.stringify(info, null, 2)}\n`, "utf8");
 NODE
+    [ -f "$info_file" ] || error "Failed to write source-info.json"
 }
🤖 Prompt for 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.

In `@scripts/lib/package-common.sh` around lines 517 - 599, The embedded Node.js
heredoc that writes the info file can fail (e.g., fs.writeFileSync) but its
non-zero exit status is ignored; after the heredoc invocation ("$node_bin" -
"$REPO_DIR" "$info_file" <<'NODE' ... NODE) check the command's exit status and
fail the script if non-zero (for example, add an immediate if [ $? -ne 0 ]; then
echo "error: failed to write ${info_file}"; exit 1; fi) or enable strict shell
failure (set -euo pipefail) earlier; keep the Node script as-is (it defines
commit, isoTimestamp, sanitizeGitRemoteUrl, etc.) but ensure the wrapper treats
node failures as fatal so incomplete outputs aren't silently accepted.
🤖 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.

Outside diff comments:
In `@scripts/lib/package-common.sh`:
- Around line 517-599: The embedded Node.js heredoc that writes the info file
can fail (e.g., fs.writeFileSync) but its non-zero exit status is ignored; after
the heredoc invocation ("$node_bin" - "$REPO_DIR" "$info_file" <<'NODE' ...
NODE) check the command's exit status and fail the script if non-zero (for
example, add an immediate if [ $? -ne 0 ]; then echo "error: failed to write
${info_file}"; exit 1; fi) or enable strict shell failure (set -euo pipefail)
earlier; keep the Node script as-is (it defines commit, isoTimestamp,
sanitizeGitRemoteUrl, etc.) but ensure the wrapper treats node failures as fatal
so incomplete outputs aren't silently accepted.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 29a84cbf-5fda-44a7-92a1-39120229d27e

📥 Commits

Reviewing files that changed from the base of the PR and between df36c86 and ebb5e5b.

📒 Files selected for processing (2)
  • scripts/lib/package-common.sh
  • tests/scripts_smoke.sh

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 26, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 26, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19e703779a

ℹ️ 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".

Comment thread port-integrations/thorium-chrome-plugin/stage.sh Outdated
Comment thread scripts/patches/main-process.js Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@scripts/patches/main-process.js`:
- Around line 650-651: The code in codexLinuxBuildInfoDetail constructs the
source revision string using i.commit before i.shortCommit when i.dirty===null,
causing full SHAs to be shown; change the logic to prefer i.shortCommit over
i.commit in all cases (compute a base like const base = i.shortCommit ||
i.commit) and then append the dirty suffix: if i.dirty===true => `${base}
(dirty)`, if i.dirty===null => `${base} (dirty state unknown)`, else just
`base`, and fall back to `"unknown"` if no base exists; update the variables s/c
assignment in codexLinuxBuildInfoDetail accordingly so shortCommit is always
preferred.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 062da745-9089-45cd-890d-63830569a1fe

📥 Commits

Reviewing files that changed from the base of the PR and between 19e7037 and 5c46ffc.

📒 Files selected for processing (4)
  • port-integrations/thorium-chrome-plugin/stage.sh
  • port-integrations/thorium-chrome-plugin/test.js
  • scripts/patch-linux-window-ui.test.js
  • scripts/patches/main-process.js

Comment thread scripts/patches/main-process.js Outdated
@nisavid nisavid merged commit ff5e251 into main May 26, 2026
18 checks passed
@nisavid nisavid deleted the nisavid/issue-75-rename-reconciliation branch May 26, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audit past fork syncs for rename-mapped missed upstream changes

1 participant