Repository navigation
Preserve uncertain goal text and accept distinct controls - #33022
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 15 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the main changes: preserving uncertain goal text and accepting distinct controls. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description provides a clear high-level explanation, detailed changes, rationale, linked issue references, testing results, compatibility notes, follow-up work, and visual evidence. It does not us… Full details: Description check
-
Explanation
The description provides a clear high-level explanation, detailed changes, rationale, linked issue references, testing results, compatibility notes, follow-up work, and visual evidence. It does not use every template heading and omits the explicit checklist, AI disclosure, Review, and Agent skill upstream boundary sections, but the required substantive information is mostly present.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 15 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b4f0ff90-d0f5-45b7-bba2-17f419d19ccc
-
src/main/codex/codex-structured-thread-goal.test.tssrc/main/codex/codex-structured-thread-goal.tssrc/main/native-chat/agent-session-wire/structured-agent-session-ready-operation-order.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-thread-goal.tssrc/renderer/src/components/native-chat/NativeChatThreadGoalBanner.test.tsxsrc/renderer/src/components/native-chat/NativeChatThreadGoalBanner.tsxsrc/renderer/src/components/native-chat/agent-session-write-notice-text.test.tssrc/renderer/src/components/native-chat/agent-session-write-notice-text.tssrc/renderer/src/components/native-chat/use-structured-agent-session-thread-goal.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-thread-goal.tssrc/renderer/src/i18n/en-runtime-required.jsonsrc/renderer/src/i18n/locales/en.jsonsrc/shared/agent-session-refusal-notice.test.tssrc/shared/agent-session-refusal-notice.tssrc/shared/agent-session-write-notice-copy.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
-
brennanb2025
changed the title
|
A lost goal reply could hide the submitted objective even when Codex had applied it. Retrying that old unknown action could override a newer decision, and a renderer pending flag could refuse a distinct goal control. The change keeps uncertain goal text, replays the original recorded outcome without another provider write, identifies explicit controls as user actions, and admits distinct controls while preserving the permission hold. Partial-change copy no longer promises the previous goal survived. The full review included adversarial correctness, architecture challenge, a final whole-mechanism comparison, performance and readiness checks. Review removed the partial Stop compensator, enabled pending Resume, corrected locale fallback, and added persisted recovery and recorded-provider-output proof. No open in-scope P0/P1 remains. Differences from the common pattern are intended for the existing durable operation ledger and direct Pause/Clear, or temporary as linked below. Temporary: STA-11962 owns the direct activation Stop gap and replacement identity/usage race. Removing the renderer latch also exposes rapid two-Set entry in one pane. This PR does not fix either gap or promise counter reset. The response-only compensator was removed before shipping. Verified: real Codex 0.163.0 UI Set/Pause/Resume/Edit/Clear, main-created history carried through a graceful restart, and two real replies afterward. Captured explicit requests carry origin:user. The paused baseline goal finished before Pause arrived, so completed history was restored. Pending controls used a labelled ~1.3-second hold on genuine replies; delivery stayed serialized. Unknown-result recovery and permission holds are covered by tests; the notice screenshot is a labelled renderer fixture. No activation-window Stop, phone goal feature, or Claude goal endpoint pass is claimed. 117 tests across seven named files passed on QA head Real Claude Code 2.1.310 ( For Brennan's review Main-created history, before / after graceful upgrade
Set / Pause / Resume — actual provider
Distinct objective / Clear / two continuing replies
Pending Pause/Clear and paused Resume — timing harness
Truthful notice — labelled renderer-only fixture
Claude shared chat — two real replies on current head
|
A lost goal reply could remove the submitted objective even when Codex applied it. Retrying that old unknown action could later overwrite a Pause, Clear, or replacement. A second distinct control could also be dropped while another request was pending.
Goal controls now keep uncertain objective text, replay only recorded results, and send each distinct action to the host. Set, Resume, Pause, and Clear identify the user as their source. Failure copy says when a change could not finish without promising the previous goal survived.
Changes
Review path: full. This touches saved objective/outcome recovery and ordering. The endpoint and saved formats stay compatible; no saved field or public wire operation was added.
Temporary follow-up
Tracked in STA-11962: the existing direct-control path can still race Stop during activation and can reuse an earlier goal's identity/usage when its notification trails a rapid replacement. Removing the renderer latch exposes that rapid two-Set entry on one pane too. The follow-up will make goal Set and Resume owned turns on capable hosts. The partial response-only Stop compensator was removed from this patch before shipping.
Validation
Real Codex 0.163.0 was exercised on the M4Air at
12bd1463951c: Set, Pause, Resume, a distinct objective, Clear, a graceful upgrade of a main-created conversation, and two normal replies afterward. Captured requests identify the user as their source. Pending Pause/Clear and paused Resume also reached the provider; these timing checks briefly held genuine replies and retained host serialization.Real Claude Code 2.1.310 (
claude-opus-5-5) also completed two file-reading sends and two replies in one native chat on18c42c605ecd, with an available composer and no Codex goal controls. Both isolated QA copies stopped gracefully and real-config hashes matched.117 tests across seven explicit files passed on that QA head. Focused SQLite close/reopen proof retains the unknown objective and original receipt, and old-ID replay cannot override later Pause/Clear/Set. A later tests-only commit replays six recorded Codex notifications through the translator, actual SQLite journal, and banner goal projection; its three-test file passed. Localization catalogs and required pre-commit checks passed.
Evidence limits: the main-created goal completed before Pause arrived, so completed history was carried through the graceful restart. Unknown-outcome recovery and permission holds are established by tests. The failure-copy screenshot is a labelled renderer fixture. No activation-window Stop or replacement identity/counter-reset pass is claimed.
The required PR gate, lint/typecheck, all five unit shards, relay integration, persistence, and both package jobs passed on
18c42c605ecd. The separate WSL workflow also passed on the failed-job retry, including Fedora, Debian and Ubuntu; the first Fedora setup timed out before tests. The earlier main lint blocker was fixed by #33046, and the added test's type annotation was corrected before this run. Visual evidence is linked below.Tracked in #31605.
Visual evidence