Skip to content

refactor(codex): remove code the ~/.codex hook rework left unused - #25744

Merged
Jinwoo-H merged 6 commits into
mainfrom
jinwoo/codex-hook-d-dead-code
Oct 6, 2026
Merged

Jinwoo-H merged 6 commits into
mainfrom
jinwoo/codex-hook-d-dead-code

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 18 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​100 $\color{#cf222e}{\Huge{\mathbf{−}}}$​184 $\color{#cf222e}{\Huge{\mathbf{−}}}$​84
Prod 19 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​57 $\color{#cf222e}{\Huge{\mathbf{−}}}$​190 $\color{#cf222e}{\Huge{\mathbf{−}}}$​133

Stack: #25741 (merged) → #25742 (merged) → #25743 (~/.codex written only on change) → #25744 (cleanup). This is part 4 of 4; review and merge in order. Supersedes #24234.

ELI5

The previous PR stopped Orca from running a background Codex approval session for ~/.codex. That left some code with nothing calling it. This PR deletes it. Nothing a user sees changes.

What Changed

  • Before: the Codex hook-approval code still carried options and helpers that only the old ~/.codex background approval used: a "background" mode (longer timeout, no retry cooldown, its own probe lane), a "use the default Codex home" mode, a ledger-only lookup, a shared-cache method for unshared probes, a startup check for whether to install the ~/.codex hook, and a native (non-WSL) approval host that, after the earlier PRs in this stack, only tests reached.
  • After: those are gone. The remaining caller, the WSL hook installer, runs exactly the path it ran before: the normal timeout, the retry cooldown and the shared probe.
  • Tests that only covered the removed code are removed. The approval-session tests now run on a WSL host, the only real one.
  • Approval records an older build wrote for a native host still load.
  • The switch that could move the system-default account to Orca's own Codex home is gone. The previous PR stopped anything from setting it, so the "is this the system default on ~/.codex?" question is now one function instead of three.

Why

Each removal was checked with a search showing no remaining production caller. Keeping them would leave untested branches that look live. The WSL path and the SSH installer are untouched.

Linked Issue

Fixes #

Visual Proof

N/A: no UI or behavior change.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated
    • Typecheck (node and CLI), lint and the changed-code quality gate pass.
    • Full src/main + src/cli run: the only 3 failures also fail on the base commit (machine-specific shell tests).

Notes

  • Cross-platform / SSH / WSL: WSL keeps its approval session unchanged; nothing here touches SSH.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — the removals are complete and verified; one stale comment example worth a tweak.

Reviewed changes

  • background / useDefaultCodexHome plan modes removed — the CodexManagedTrustGrantPlan fields, CODEX_BACKGROUND_TRUST_GRANT_TIMEOUT_MS, findCurrentManagedCodexHookTrust, and every plan.background guard in codex-hook-trust-grant.ts and codex-trust-grant-cooldown.ts. The surviving managed path runs the same cooldown, probe and timeout as before.
  • Request/host surface narrowed — CodexTrustGrantRequestInput.timeoutMs and the native buildRequest default-home branch dropped; native now always sets CODEX_HOME + NATIVE_GRANT_TIMEOUT_MS, WSL keeps WSL_GRANT_TIMEOUT_MS.
  • Shared cache helper removed — CapabilityProbeCache.runUnshared deleted; runWithFallback is the only path.
  • Startup helper removed — shouldInstallStartupManagedAgentHook deleted; resolveStartupManagedHookAction and shouldContinueManagedHookStartup are untouched.
  • Test cleanup — imports and tests that only exercised the removed branches deleted.

No remaining references to any removed symbol exist in the repo (verified by search), pnpm tc passes, and the four touched/relevant test files pass (48 tests).

ℹ️ Nitpicks

  • src/main/codex/codex-app-server-session.ts:34-35 — the envToDelete doc example still reads "an inherited CODEX_HOME, so a default-home grant runs against the real ~/.codex". This PR removed the only grant that produced envToDelete; the live caller is now listCodexHooks in codex-hook-trust-derivation.ts:130 when codexHome is null. The mechanism is live, but the "grant" example is stale — reword it to the derivation case.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch from 8a2e9e0 to fd95a2b Compare October 6, 2026 02:45
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch 2 times, most recently from b4a574a to 225491e Compare October 6, 2026 04:11
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch 2 times, most recently from 2b1578c to f3ff6b4 Compare October 6, 2026 04:44
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from 225491e to 4afb766 Compare October 6, 2026 04:44
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch 2 times, most recently from 17ae487 to ef4388a Compare October 6, 2026 07:08
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from 5da068f to ff3652c Compare October 6, 2026 07:08
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch from ef4388a to 8cdba24 Compare October 6, 2026 07:39
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from ff3652c to 2cc4cc2 Compare October 6, 2026 07:39
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch from 8cdba24 to f552966 Compare October 6, 2026 17:40
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from 2cc4cc2 to ed88c6d Compare October 6, 2026 17:40
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch from f552966 to c491f5d Compare October 6, 2026 18:19
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from ed88c6d to 26945c1 Compare October 6, 2026 18:19
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch from c491f5d to 44ba5ec Compare October 6, 2026 18:38
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from 5eeb083 to c0266f3 Compare October 6, 2026 19:24
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch from 44ba5ec to 23ae714 Compare October 6, 2026 19:24
Only WSL runs a grant session, so resolveNativeCodexTrustGrantHost, its
timeout, the native host arm and buildNativeCodexBinaryStamp go. The ledger
still parses a native stamp, so files an older build wrote keep loading. The
grant suites now run on a WSL host with the guest probe stubbed.
… one host left

Only WSL reaches the grant, always on the managed lane; the telemetry event keeps its shape.
@Jinwoo-H
Jinwoo-H changed the base branch from jinwoo/codex-hook-c-real-home-reconcile to main October 6, 2026 20:12
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-d-dead-code branch from 23ae714 to fa2a059 Compare October 6, 2026 20:12
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c469f33e-e013-4b3d-9191-b61f708d9c8d
📥 Commits

Reviewing files that changed from the base of the PR and between ac8ea9f and fa2a059.

📒 Files selected for processing (37)
  • src/main/agent-hooks/managed-agent-hook-controls.test.ts
  • src/main/agent-hooks/managed-agent-hook-controls.ts
  • src/main/codex-accounts/runtime-home-managed-auth-recovery.test.ts
  • src/main/codex-accounts/runtime-home-per-account-homes.test.ts
  • src/main/codex-accounts/runtime-home-real-home-lane-routing.test.ts
  • src/main/codex-accounts/runtime-home-retained-auth-provenance.test.ts
  • src/main/codex-accounts/runtime-home-service-home-routing.ts
  • src/main/codex-accounts/runtime-home-service-launch.ts
  • src/main/codex-accounts/runtime-home-service-paths.ts
  • src/main/codex-accounts/runtime-home-service-state.ts
  • src/main/codex-accounts/runtime-home-service-sync.ts
  • src/main/codex-accounts/runtime-home-status-hook-independence.test.ts
  • src/main/codex-accounts/runtime-home-system-default-snapshot.test.ts
  • src/main/codex/codex-ai-vault-session-resume.test.ts
  • src/main/codex/codex-ai-vault-session-resume.ts
  • src/main/codex/codex-app-server-capability-cache.test.ts
  • src/main/codex/codex-app-server-capability-cache.ts
  • src/main/codex/codex-app-server-session.ts
  • src/main/codex/codex-hook-trust-grant.test.ts
  • src/main/codex/codex-hook-trust-grant.ts
  • src/main/codex/codex-hook-wsl-runtime.ts
  • src/main/codex/codex-legacy-session-resume.test.ts
  • src/main/codex/codex-legacy-session-resume.ts
  • src/main/codex/codex-managed-trust-grant-plan.ts
  • src/main/codex/codex-real-home-shared-entry.test.ts
  • src/main/codex/codex-short-lived-app-server-spawn.test.ts
  • src/main/codex/codex-trust-config-concurrent-launch.test.ts
  • src/main/codex/codex-trust-grant-cooldown.ts
  • src/main/codex/codex-trust-grant-host.test.ts
  • src/main/codex/codex-trust-grant-host.ts
  • src/main/codex/codex-trust-grant-ledger.ts
  • src/main/codex/codex-trust-grant-telemetry.ts
  • src/main/startup/codex-account-restart-resume.test.ts
  • src/main/startup/codex-launch-per-agent-hook-opt-out.test.ts
  • src/main/startup/codex-session-resume-daemon-guard.test.ts
  • src/main/startup/codex-session-resume-launch.ts
  • src/shared/capability-probe-cache.ts
💤 Files with no reviewable changes (10)
  • src/main/codex/codex-trust-grant-cooldown.ts
  • src/main/startup/codex-launch-per-agent-hook-opt-out.test.ts
  • src/main/agent-hooks/managed-agent-hook-controls.ts
  • src/main/agent-hooks/managed-agent-hook-controls.test.ts
  • src/main/codex-accounts/runtime-home-system-default-snapshot.test.ts
  • src/shared/capability-probe-cache.ts
  • src/main/codex/codex-short-lived-app-server-spawn.test.ts
  • src/main/codex-accounts/runtime-home-retained-auth-provenance.test.ts
  • src/main/codex/codex-managed-trust-grant-plan.ts
  • src/main/codex-accounts/runtime-home-service-state.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Runtime-home routing now uses the host system-default home selection check, and the lane-gate API is removed. Codex trust-grant host handling and capability keys now use WSL distro identities; grant plans use shared fallback handling and apply cooldown checks to all plans. Managed hook startup continuation now also checks whether the application is quitting.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to fa2a0

No identified issue blocks merging after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 27 files. 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 clearly identifies the cleanup of unused code from the Codex hook rework.
Description check ✅ Passed The description explains the changes, rationale, testing, and lack of user-facing impact. The linked issue is blank, and some checklist items remain unchecked, but the description is otherwise mostly …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

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

@Jinwoo-H
Jinwoo-H merged commit 58b4633 into main Oct 6, 2026
40 checks passed
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
…de-probes

codex-hook-trust-grant.ts takes main's side: #25743/#25744 removed the background
trust-grant timeout whose comment PR 2 had updated, so that comment change is
dropped. tsconfig.cli.json keeps main's edits plus PR 2's teardown-chain entries,
without the two real-home grant files main deleted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant