fix: normalize public Codex thread refs for live control - #796
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
evaOS review status: completedPR: #796 - fix: normalize public Codex thread refs for live control evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #796 Review URL: #796 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #796 - fix: normalize public Codex thread refs for live control
Head: 92aee2f45a42d07029a059bf9b8dbe575e29591f into main. Review event: COMMENT.
Provider: Unregistered provider id (builtin:zai-coding-plan, zcode (registry miss), model unknown).
Estimated review effort: 3/5 (~48 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +6/-7 | Documentation | Low |
docs/SETUP.md |
modified | +3/-3 | Documentation | Low |
docs/releases/CHANGELOG.md |
modified | +2/-1 | Documentation | Low |
docs/releases/RELEASE_NOTES_1.6.1.md |
added | +31/-0 | Documentation | Low |
package-lock.json |
modified | +2/-2 | Configuration | Low |
package.json |
modified | +1/-1 | Configuration | Low |
packages/mcp-server/src/tools.ts |
modified | +11/-3 | Changed file | Moderate: validated P3 finding |
packages/openclaw-plugin/openclaw.plugin.json |
modified | +1/-1 | Changed file | Low |
tests/control-and-mcp.test.ts |
modified | +71/-0 | Test coverage | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 0, P3: 3).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Release regression: 1
- Runtime correctness: 2
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #795, #673.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92aee2f45a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
evaOS review status: completedPR: #796 - fix: normalize public Codex thread refs for live control evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #796 Review URL: #796 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #796 - fix: normalize public Codex thread refs for live control
Head: 59aa9b68e9730ba4e2409a18d0b7666b7bef9631 into main. Review event: COMMENT.
Provider: Unregistered provider id (builtin:zai-coding-plan, zcode (registry miss), model unknown).
Estimated review effort: 4/5 (~60 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +6/-7 | Documentation | Low |
docs/BETA_RELEASE_RUNBOOK.md |
modified | +1/-1 | Documentation | Low |
docs/CLAIM_AUDIT.md |
modified | +2/-2 | Documentation | Low |
docs/SETUP.md |
modified | +3/-3 | Documentation | Low |
docs/releases/CHANGELOG.md |
modified | +2/-1 | Documentation | Moderate: validated P3 finding |
docs/releases/RELEASE_NOTES_1.6.1.md |
added | +36/-0 | Documentation | Low |
package-lock.json |
modified | +2/-2 | Configuration | Low |
package.json |
modified | +1/-1 | Configuration | Low |
packages/mcp-server/src/tools.ts |
modified | +11/-3 | Changed file | Low |
packages/openclaw-plugin/openclaw.plugin.json |
modified | +1/-1 | Changed file | Low |
tests/control-and-mcp.test.ts |
modified | +71/-0 | Test coverage | Moderate: validated P3 finding |
tests/release-claim-audit.test.ts |
modified | +9/-5 | Test coverage | Low |
Review Signal
Validated inline findings: 2 (P0: 0, P1: 0, P2: 0, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Release regression: 2
Validation and Proof
2 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
- required: CI/release smoke proof - CI, release, launchd, or package metadata changed. Proof: green GitHub check; release-status; coverage-audit; rollback note.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Call out evidence leakage, replay/collision risks, duplicate side effects, and brittle sanitizer logic.
Profile proof expectations: Look for focused sanitizer, signature, orchestration, or fixture proof.
Related Context
Related issues/PRs: #795, #673.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: orchestration, security, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
Outcome
Fix the Eva 1.6.0 live-control failure where a public-safe thread reference from
lco_recent_sessionswas accepted by the LCO dry run but rejected by Codex during the live call.Root cause
Recent-session cards expose
threadIdascodex_thread:<raw-id>. The canonical MCP control entrypoints forwarded that prefixed reference unchanged to Codex app-server and used it unchanged in approval hashing. Codex rejected the target with JSON-RPC-32600.Changes
codex_thread:prefix once at the MCP control-input boundary.Validation
node --test --import tsx --test-name-pattern="MCP Codex control normalizes public codex_thread refs" tests/control-and-mcp.test.tsnode --test --import tsx tests/control-and-mcp.test.ts— 27/27node --test --import tsx tests/public-docs.test.ts— 11/11 after the docs-only correctionnpm run openclaw:manifest:checknpm run buildnpm run typecheckremains red on pre-existing unrelated test typings; this PR does not widen into that backlog. The canonicalnpm run checkand remote CI remain required.Safety boundary
The matching
approval_audit_id, never-approve policy, read-only/no-network sandbox, response redaction, and raw-transcript exclusions are unchanged. This PR does not change Eva’s Hermes profile, gateway, MCP selection, or database and does not add a native Hermes adapter.Agent provenance
fix/1.6.1-thread-ref-controlCloses #795
Related: #673