Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: blocked before merge. Reviewed September 28, 2026, 12:08 AM ET / 04:08 UTC. ClawSweeper reviewWhat this changesThe 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 provenancePossible 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 Review scores
Verification
How this fits togetherCodexBar’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]
Decision needed
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
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest 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:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 579f68406855. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
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.
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!
ChatGPT's nested
codex-cli/CodexCLI.app/Contents/MacOS/codex app-serverwas 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 outercom.openai.codexbundle through the existing signature/Gatekeeper preflight. The nested bundle's own identifier iscodex, 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, andCODEXBAR_TEST_SESSION_FILE_ISOLATION=1on an isolated macOS worker. No running CodexBar app was relaunched.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.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.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
renameoperations and unrelated process-cleanup fixtures, so the macOS worker with matching source hashes supplied the completed proof.Fixes #4069