Skip to content

fix(refresh): detect nested ChatGPT Codex app-server - #4090

Closed
steipete wants to merge 4 commits into
mainfrom
triage/20260921-adaptive-codex-path
Closed

steipete wants to merge 4 commits into
mainfrom
triage/20260921-adaptive-codex-path

Conversation

@steipete

Copy link
Copy Markdown
Owner

ChatGPT's nested codex-cli/CodexCLI.app/Contents/MacOS/codex app-server was missing from the adaptive activity gate. With Agent Sessions hidden, recent rollouts therefore could not select the five-minute coding cadence. Recognize that layout alongside the existing flat executable, while requiring the running PID's kernel-reported path and OpenAI signature, rejecting symlink redirects, and validating the outer com.openai.codex bundle through the existing signature/Gatekeeper preflight. The nested bundle's own identifier is codex, so validating the outer bundle matters.

Trust is rechecked on every scan instead of cached permanently by pathname. Temporary/home-directory copies, forged command lines, and non-server helpers do not qualify for this app-server gate. Rollout freshness remains authoritative: an idle server alone does not accelerate refresh. The matching rule and 0.68.1 changelog are updated, and the obsolete architecture exception for the removed basename guard is deleted. Sharing equivalent rollout flags and scan-budget setup keeps this lane at 3 fewer production lines.

Verification

Synthetic homes and fixtures were used with CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1, CODEXBAR_TEST_CODEX_FILE_ISOLATION=1, and CODEXBAR_TEST_SESSION_FILE_ISOLATION=1 on an isolated macOS worker. No running CodexBar app was relaunched.

  • Original scanner/parser with the new regressions: swift test --build-system native --jobs 4 --filter CodexSessionRolloutTests — 19 tests, 8 expected assertion issues across 5 tests. This reproduces the nested-path miss, stale pathname trust, and overly broad path acceptance.
  • Final integrated tree: swift test --build-system native --jobs 4 --no-parallel --filter 'ChatGPTCodexProcessTrustTests|CodexSessionRolloutTests|AgentSessionParserTests|AgentSessionMenuDescriptorTests|AgentSessionsStoreSchedulerTests|AdaptiveRefresh.*Tests|PiFamilySessionTests|PiSessionProcessContextTests|PathBuilderTests|ProviderArchitectureGatekeeperTests' — 225 tests in 14 suites passed.
  • make check — passed; 0 SwiftLint violations in 2,668 files.
  • Independent Codex review through P2 — clean. Read-only checks also confirmed the installed nested executable's OpenAI signing identity, canonical path, and accepted outer-bundle Gatekeeper assessment.

The initial parallel focused run hit the existing 250 ms scanner performance assertion (367 ms); the complete set passed with parallelism disabled and the assertion unchanged. The local shared host stalled in filesystem rename operations and unrelated process-cleanup fixtures, so the macOS worker with matching source hashes supplied the completed proof.

Fixes #4069

@clawsweeper

clawsweeper Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 28, 2026
@clawsweeper

clawsweeper Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 28, 2026, 12:08 AM ET / 04:08 UTC.

ClawSweeper review

What this changes

The branch recognizes ChatGPT’s nested Codex app-server for adaptive refresh, replaces cached pathname trust with live process and outer-bundle checks, and updates tests, documentation, and the changelog.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 4 items remain

Keep this PR open. Current main still misses the reported nested ChatGPT executable, and the branch addresses that case, but its compatibility with an existing installation location needs an explicit decision.

Priority: P2
Reviewed head: e7197ba0a0736d860cdfb7b2cebb215174fa3b83
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Focused tests and a narrow implementation provide useful signal, but the shipped installation-path regression limits merge confidence.
Proof confidence 🦐 gold shrimp (3/6) Not applicable: The changed scanner trust gate has synthetic process-and-rollout coverage and the PR reports read-only inspection of a signed installed bundle, but no real after-fix adaptive scan trace; the OWNER proof exemption applies. No stored-data contract changes.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The changed scanner trust gate has synthetic process-and-rollout coverage and the PR reports read-only inspection of a signed installed bundle, but no real after-fix adaptive scan trace; the OWNER proof exemption applies. No stored-data contract changes.
Evidence reviewed 8 items Current-main gap: Current main allows the flat system and user Applications paths but has no nested CodexCLI.app path, so the linked report remains relevant.
Shipped installation path: The v0.68.0 matcher accepted a signed ChatGPT app-server under the user’s Applications directory; the same path remains accepted on current main.
Introduced path restriction: The introduced allowlist contains only two paths beneath /Applications; the prior home-directory path is absent.
Findings 1 actionable finding [P1] Preserve the signed user Applications path
Security None None.

How this fits together

CodexBar’s agent-aware adaptive refresh uses consented local process and rollout scans to detect recent coding activity. That activity determines whether usage refreshes run on the five-minute coding cadence or the longer idle cadence.

flowchart LR
A[Scan consent] --> B[Local process list]
B --> C[ChatGPT app-server trust check]
C -->|trusted| D[Recent Codex rollouts]
C -->|not trusted| F[Idle refresh cadence]
D --> E[Latest coding activity]
E --> G[Refresh cadence]
Loading

Decision needed

Question Recommendation
Should signed ChatGPT app-server installations in ~/Applications remain eligible for agent-aware refresh, as they were in v0.68.0? Preserve user Applications: Validate the running process and its corresponding signed outer bundle at the prior user path, then test both fresh and upgraded installations.

Why: The branch intentionally excludes a shipped installation location, and source inspection cannot decide whether that loss of support is an approved upgrade policy.

Before merge

  • Preserve the signed user Applications path (P1) - The new fixed path set omits the ~/Applications/ChatGPT.app/Contents/Resources/codex path accepted in v0.68.0, and the new test explicitly rejects it. For an existing signed installation there, with Agent Sessions hidden and no other agent process, scanning returns before reading updated rollouts and adaptive refresh falls back toward the idle cadence. Retain this path under equivalent live-process and bundle validation, or obtain explicit approval of the breaking upgrade.
  • Resolve merge risk (P1) - The owner has stated that home-directory installations should no longer qualify, but has not confirmed the upgrade impact on signed ChatGPT installations that v0.68.0 accepted or shown their recovery path.
  • Complete next step (P2) - Decide whether to retain signed ~/Applications installations; then test the chosen upgrade behavior and document any approved break.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P1] Preserve the signed user Applications path — Sources/CodexBarCore/AgentSession.swift:330-333
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Code and test delta production -3 lines; tests +167 lines The production change is small and has focused regression coverage, while the changed installation contract still needs review.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #4069
Summary: This PR is an open candidate fix for the linked nested-path report; it has not merged.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Keep the shipped location (recommended)
    Retain signed ~/Applications matches using the same live-process checks and add a regression for the upgraded setup.
  2. Own the upgrade break
    Approve the narrower installation policy and publish a clear recovery path for affected users.

Technical review

Best possible solution:

Recognize the nested executable while retaining the previously accepted signed user Applications path under equivalent running-process and corresponding outer-bundle validation, with focused fresh-install and upgrade coverage.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main lacks the nested path, so an adaptive-only scan with that app-server and no other agent process cannot read recent rollouts. This review did not run the app on macOS.

Is this the best way to solve the issue?

Not yet: the nested-path and live-trust approach fits the existing design, but its exact-path list drops a previously accepted installation location.

Full review comments:

  • [P1] Preserve the signed user Applications path — Sources/CodexBarCore/AgentSession.swift:330-333
    The new fixed path set omits the ~/Applications/ChatGPT.app/Contents/Resources/codex path accepted in v0.68.0, and the new test explicitly rejects it. For an existing signed installation there, with Agent Sessions hidden and no other agent process, scanning returns before reading updated rollouts and adaptive refresh falls back toward the idle cadence. Retain this path under equivalent live-process and bundle validation, or obtain explicit approval of the breaking upgrade.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 579f68406855.

Labels

Label changes:

  • add P2: This addresses a reported slowdown in an opt-in adaptive refresh mode, with limited user impact.
  • add merge-risk: 🚨 compatibility: Merging would stop recognizing a signed user Applications installation that the latest release accepts.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The changed scanner trust gate has synthetic process-and-rollout coverage and the PR reports read-only inspection of a signed installed bundle, but no real after-fix adaptive scan trace; the OWNER proof exemption applies. No stored-data contract changes.

Label justifications:

  • P2: This addresses a reported slowdown in an opt-in adaptive refresh mode, with limited user impact.
  • merge-risk: 🚨 compatibility: Merging would stop recognizing a signed user Applications installation that the latest release accepts.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The changed scanner trust gate has synthetic process-and-rollout coverage and the PR reports read-only inspection of a signed installed bundle, but no real after-fix adaptive scan trace; the OWNER proof exemption applies. No stored-data contract changes.

Evidence

What I checked:

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Resolve the ~/Applications compatibility decision and cover the chosen upgrade behavior with a focused regression.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Reuse successful outer-bundle preflight while its filesystem identity is unchanged. Keep per-PID signature and exact-path checks live, retry failures, and reject updates during assessment. Deduplicate scanner session construction and CWD lookup to keep production code flat.
steipete added a commit that referenced this pull request Sep 28, 2026
Adaptive agent-aware refresh now recognizes the Codex app-server nested inside the ChatGPT app (both documented executable paths under /Applications/ChatGPT.app), after checking the running PID's kernel-reported path, OpenAI code signature, and symlink redirects on every scan; the outer bundle's Gatekeeper assessment is cached by bundle, Info.plist, executable, and CodeResources identity and retried on failure. Recent rollout activity stays authoritative. Fixes #4069.

Thanks @jaychou0642-create!
@steipete

Copy link
Copy Markdown
Owner Author

Landed on main as 1060f34 via merge train #4099 (one green CI run for the whole train).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adaptive agent-aware refresh misses ChatGPT Codex at nested CodexCLI.app path (0.68.0)

1 participant