Default useful port integrations#103
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughFour port integrations ( ChangesPort Integrations Promoted to Default-Enabled with Security Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: dd7cd80c76
ℹ️ 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".
|
| Filename | Overview |
|---|---|
| port-integrations/appshots/patch.js | Fixes operator-precedence bug in Linux availability expression and upgrades older already-patched bundle shapes; per-capture private temp directory replaces ad-hoc filename randomness. |
| scripts/bootstrap-wizard.sh | Refactored integration resolution to respect defaultEnabled manifests and persist disabled ids; no warning is emitted for stale/unknown disabled ids (see comment), but this is a minor UX gap, not a correctness bug. |
| port-integrations/codex-wrapper-updater/patch.js | Added CODEX_PACKAGE_HAS_UPDATER gating via codexLinuxWrapManagerAvailable; codexLinuxWrapStatusPayload now exposes enabled/available fields and skips status reads when manager is absent. |
| port-integrations/codex-wrapper-updater/apply-pending.sh | New package_has_updater() function correctly handles unset, empty, whitespace-only, and falsy string values, falling back to AppImage detection; stale markers are cleared early for no-updater packages. |
| launcher/start.sh.template | Moved export_packaged_runtime_env from just before cold-start hooks to just after source_feature_env_files, ensuring CODEX_PACKAGE_HAS_UPDATER is visible to all prelaunch hooks. |
| port-integrations/thorium-chrome-plugin/patch-chrome-plugin.js | Added new bundle-shape variant for browser-client.mjs and updated alreadyText to accept arrays for both old and new profile-root shapes. |
| port-integrations/copilot-reasoning-effort/test.js | Updated withTempIntegrationConfig default disabled parameter to derive from the full defaultEnabledIntegrationIds list (including copilot-reasoning-effort), resolving the previously flagged helper-mismatch concern. |
| packaging/appimage/codex-appimage-runtime.sh | Added CODEX_PACKAGE_HAS_UPDATER="0" export so AppImage runtime correctly signals no-updater status to the wrapper-updater integration. |
| packaging/linux/codex-packaged-runtime.sh | Added CODEX_PACKAGE_HAS_UPDATER="__CODEX_PACKAGE_ENABLE_UPDATER__" template placeholder rendered at build time. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[integration.json
defaultEnabled: true/false] -->|read by| B[bootstrap-wizard.sh
discover_integrations]
C[integrations.json
enabled / disabled arrays] -->|read by| D[read_config_ids]
B --> E[resolved_enabled_ids]
D --> E
E -->|defaultEnabled + config_enabled
minus config_disabled| F[final_resolved]
F -->|written to| G[port-integrations.json
persisted config]
G -->|read by| H[scripts/lib/port-integrations.js]
H -->|enabled set| I[ASAR patch pipeline]
I --> J[agent-workspace patch]
I --> K[appshots patch
operator-precedence fix]
I --> L[codex-wrapper-updater patch
CODEX_PACKAGE_HAS_UPDATER gate]
I --> M[copilot-reasoning-effort patch]
N[appimage-runtime.sh
CODEX_PACKAGE_HAS_UPDATER=0] -->|env var| O[launcher/start.sh
export_packaged_runtime_env
moved before prelaunch hooks]
P[codex-packaged-runtime.sh
CODEX_PACKAGE_HAS_UPDATER=__FLAG__] -->|build substitution| O
O -->|visible to| L
L -->|codexLinuxWrapManagerAvailable| Q{updater present?}
Q -- No --> R[hide UI, clear stale marker]
Q -- Yes --> S[normal wrapper update flow]
Reviews (4): Last reviewed commit: "fix(port-integrations/wrapper-updater): ..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/port-integrations-architecture.md`:
- Around line 225-230: The design rule requiring default-enabled integrations to
have "control and security surfaces documented and match the maintainer security
best practices" references a non-existent document
`maintainers/security-best-practices.md`. Create this missing document with
enumerated requirements for the four promoted integrations (agent-workspace,
appshots, codex-wrapper-updater, and copilot-reasoning-effort). Additionally,
ensure each of these four integration READMEs includes explicit section headings
documenting control surfaces (like agent-workspace-linux), local gates, and
security boundaries, with copilot-reasoning-effort serving as the template
example that already has a "## Security Boundary" section.
In `@port-integrations/appshots/patch.js`:
- Around line 15-17: The early return condition in the test on line 15 (checking
for the pattern with `linux` and `macOS` in parentheses) prevents the function
from fully normalizing remaining legacy availability gates in mixed-shape
bundles. Remove this early return check so the function continues processing and
normalizes all legacy gates (including unparenthesized or macOS-only gates)
before returning the patched source.
In `@README.md`:
- Around line 235-242: Resolve the inconsistency between the README
policy-boundary claims at lines 235-242 and the threat model documentation. The
README states that audio availability and host network exposure are policy gates
controlled by OpenAI-hosted services, but the threat model and
security-best-practices docs do not explicitly enumerate these boundaries.
Either expand the threat model documentation to formally enumerate audio
availability and host network exposure as documented policy gates, or revise the
README text to remove references to these specific boundaries and only mention
the policy gates that are actually documented in the maintainer security
documentation. Ensure both documents consistently enumerate the same set of
OpenAI-hosted policy boundaries.
🪄 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: 326cc25b-9e4c-477b-af85-5893c30b1e47
📒 Files selected for processing (30)
README.mddocs/README.mddocs/maintainers/fork-divergences.mddocs/maintainers/package-runtime-maintenance.mddocs/maintainers/security-backlog.mddocs/maintainers/security-best-practices.mddocs/maintainers/threat-model.mddocs/port-integrations-architecture.mddocs/usage/build-and-run.mddocs/usage/support-routing.mdport-integrations/README.mdport-integrations/agent-workspace/README.mdport-integrations/agent-workspace/integration.jsonport-integrations/agent-workspace/test.jsport-integrations/appshots/README.mdport-integrations/appshots/integration.jsonport-integrations/appshots/patch.jsport-integrations/appshots/test.jsport-integrations/codex-wrapper-updater/README.mdport-integrations/codex-wrapper-updater/integration.jsonport-integrations/codex-wrapper-updater/test.jsport-integrations/conversation-mode/test.jsport-integrations/copilot-reasoning-effort/README.mdport-integrations/copilot-reasoning-effort/integration.jsonport-integrations/copilot-reasoning-effort/test.jsport-integrations/open-target-discovery/test.jsport-integrations/remote-control-ui/test.jsport-integrations/thorium-chrome-plugin/patch-chrome-plugin.jsport-integrations/thorium-chrome-plugin/test.jsport-integrations/zed-opener/test.js
coderabbitai
Bot
added
the
github_actions
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a0314f75b
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
port-integrations/copilot-reasoning-effort/README.md (1)
12-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the canonical config filename in opt-out instructions.
Line 12 still points to
port-integrations/integrations.json, while this PR’s documentation set standardizes onport-integrations.json. Keeping this legacy path here creates conflicting operator instructions for the same control path.As per coding guidelines, “the implementation path and config APIs are
port-integrations/,port-integrations.json, andCODEX_PORT_INTEGRATIONS_*; use those exact names only for source paths, file paths, and environment variables.”Suggested doc fix
-Disable it by copying `port-integrations/integrations.example.json` to -`port-integrations/integrations.json` and adding the integration id: +Disable it in `port-integrations.json` and add the integration id:🤖 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 `@port-integrations/copilot-reasoning-effort/README.md` around lines 12 - 21, Update the file path references in the opt-out instructions section to use the standardized canonical configuration filenames. Replace the legacy path `port-integrations/integrations.json` with the canonical path `port-integrations.json` in the instruction text that describes copying from the example file. This ensures consistency with the PR's documentation standards and follows the coding guidelines that specify `port-integrations.json` as the canonical config filename for source paths and file paths.Source: Coding guidelines
🤖 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 `@port-integrations/codex-wrapper-updater/apply-pending.sh`:
- Around line 17-22: The package_has_updater() function evaluates the raw
CODEX_PACKAGE_HAS_UPDATER variable without normalization, so values like " False
" with whitespace and mixed case are treated as truthy. However, patch.js
normalizes the value by trimming and converting to lowercase before evaluation.
Normalize the CODEX_PACKAGE_HAS_UPDATER variable inside package_has_updater() by
trimming whitespace and converting to lowercase before the case statement to
match the behavior of the JavaScript runtime code and ensure consistent gating
between the hook and UI.
---
Outside diff comments:
In `@port-integrations/copilot-reasoning-effort/README.md`:
- Around line 12-21: Update the file path references in the opt-out instructions
section to use the standardized canonical configuration filenames. Replace the
legacy path `port-integrations/integrations.json` with the canonical path
`port-integrations.json` in the instruction text that describes copying from the
example file. This ensures consistency with the PR's documentation standards and
follows the coding guidelines that specify `port-integrations.json` as the
canonical config filename for source paths and file paths.
🪄 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: 90d92a37-17c2-4aec-8d11-e46a1e503dbd
📒 Files selected for processing (15)
docs/maintainers/security-best-practices.mddocs/maintainers/threat-model.mddocs/port-integrations-architecture.mdpackaging/linux/codex-packaged-runtime.shport-integrations/agent-workspace/README.mdport-integrations/appshots/README.mdport-integrations/appshots/patch.jsport-integrations/appshots/test.jsport-integrations/codex-wrapper-updater/README.mdport-integrations/codex-wrapper-updater/apply-pending.shport-integrations/codex-wrapper-updater/patch.jsport-integrations/codex-wrapper-updater/test.jsport-integrations/copilot-reasoning-effort/README.mdport-integrations/copilot-reasoning-effort/test.jsscripts/bootstrap-wizard.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af2200a1a8
ℹ️ 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".
1 participant
Summary
Changes
disabledids so setup-time disables override manifest defaults.CODEX_PACKAGE_HAS_UPDATER=0, hide the in-app wrapper updater, and clear stale pending markers.browser-client.mjsprofile-root shape and treats the current metadata/instance matching code as already compatible.Verification
bash -n launcher/start.sh.template scripts/bootstrap-wizard.sh packaging/appimage/codex-appimage-runtime.sh packaging/linux/codex-packaged-runtime.sh port-integrations/codex-wrapper-updater/apply-pending.sh tests/scripts_smoke.shpassed.node --test port-integrations/codex-wrapper-updater/test.jspassed: 16 tests.node --test port-integrations/*/test.jspassed: 283 tests.bash tests/scripts_smoke.shpassed.git diff --checkandgit diff --cached --checkpassed.CODEX_DISABLE_PORT_INTEGRATIONS=agent-workspacewrotedisabled: ["agent-workspace"], andscripts/lib/port-integrations.js --enabledexcludedagent-workspacewhile preserving the other defaults.check-wrapper, status visibility, and direct install paths are gated bycodex-linux-wrapper-updates-enabled, which defaults off.npm_config_cache=/tmp/codex-npm-cache npm_config_logs_dir=/tmp/codex-npm-logs make build-app packagepassed after refreshingCodex.dmgbecause cached metadata differed from upstream; rebuiltdist/codex-app-26.609.41114-1-x86_64.pkg.tar.zst.thorium-chrome-pluginenabled with the other default integrations; generated Chrome plugin files include.config/thorium.PACKAGE_WITH_UPDATER=0 make pacmanpassed; the no-updater package omits/usr/bin/codex-app-updaterand the update-builder bundle, and rendersCODEX_PACKAGE_HAS_UPDATER="0"inusr/lib/codex-app/packaged-runtime.sh.codex-app/start.shexports packaged runtime env before feature prelaunch hooks, soCODEX_PACKAGE_HAS_UPDATERis visible to wrapper-updater hooks before they run.make appimageis blocked on this host becauseappimagetoolis not installed;bash tests/scripts_smoke.shcovers the AppImage builder path with a fakeappimagetooland verifiesCODEX_PACKAGE_HAS_UPDATER="0"in the staged AppImage runtime helper.Follow-up
.binsymlink warnings during DMG extraction.Summary by CodeRabbit
Summary by CodeRabbit