Skip to content

fix(cli): preserve server-issued batch for role suggestion --apply - #8022

Merged
lidge-jun merged 4 commits into
lidge-jun:devfrom
luvs01:codex/fix-cli-auto-assign-batch-history-issue
Oct 31, 2026
Merged

lidge-jun merged 4 commits into
lidge-jun:devfrom
luvs01:codex/fix-cli-auto-assign-batch-history-issue

Conversation

@luvs01

@luvs01 luvs01 commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Preserve the server-issued batch ID across ocx agent roles suggest --apply writes so one Apply-all operation can be restored as one history batch. Refuse more than 50 applicable changed roles before any write; exclude unchanged, unsized and unassigned proposals. Preserve original nonzero errors for partial or unconfirmed outcomes without retrying or rolling back uncertain writes.

Japanese, Korean, Russian and Simplified Chinese guides now document the 50-change cap, exit 5 no-write refusal, exclusions, per-role set fallback and canonical recovery contract.

Migrated from luvs01/opencodex#798 without changing HEAD a92afee2fba53ce2115b428294ada7013e198c94. This includes observed current dev c34d26ff2bde9516517fe7fd58c2bcda70ca5e03; unique delta is 12 files. No GUI changes or live role/config updates are included in this submission.

Verification

  • Current native Windows, pinned Bun 1.4.4 and isolated homes: two related files 116 pass / 639 assertions, typecheck 2,252 files, structure/privacy/file-size gates and docs build 633 pages / 91,440 links passed. Independent read-only review found no blocker.
  • Related files: tests/cli/cli-role-suggestion-history.test.ts and tests/cli/cli-headless-parity.test.ts. Coverage exercises real management handlers/private on-disk role history: one batch across 50 writes and restore, 51 applicable changes refused before writes, excluded no-op proposals, partial failure preserving the original error, and lost acknowledgement without retries/rollback.
  • Two 50-write cases exceeded the default 5s on USB and SSD. Only those two cases use the repository's existing 30s STORE budget; durable writes/restoration and every assertion remain. A missing-batch mutation failed the exact-batch assertion; counting proposals instead of changes failed the exit-code assertion. Production ablations were restored by hash. Final measured bulk cases were about 23.25s (apply and restore) and 11.95s. Other deadlines are unchanged; this is not a claim that the original 5s executions passed.
  • Exact-head fork CI41124844162 and Catalog41124844119 succeeded, source readiness was confirmed and no unresolved fork threads remain. These are fork results, not upstream CI.
  • Earlier Linux validation and original source commits remain in the source PR as historical evidence, not a repeat on this head. Full local repository execution is not claimed; focused tests and listed gates are local evidence, broader execution is covered by exact-head hosted CI. Native platform-policy skips do not establish native runtime coverage. No installed app, real account, service, permissions or user role data was changed.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. Independent scoped source review is recorded; maintainer acceptance remains separate.

Upstream readiness

Initially submitted Draft. Exact upstream CI and Catalog succeeded; no merge, deployment or fork closure is requested.

  • Required local validation passed with historical failures, test-budget rationale and remaining coverage documented.
  • Branch includes observed current dev c34d26f.
  • All current correct Codex and CodeRabbit findings are addressed; no open inline findings at readback. Draft review skip is not approval.
  • Ready for review after exact upstream CI and final review/base readback.

Exact upstream validation — 2026-10-30 21:13 UTC

Exact head a92afee2fba53ce2115b428294ada7013e198c94: Cross-platform CI41127490376 and Catalog41127490241 succeeded. Hygiene passed; current dev remains c34d26f and branch is zero behind, mergeable, with zero inline threads. CodeRabbit initially skipped while Draft, not approval. The subsequent Ready-event review completed on this exact head with no actionable comments and minimal assessed risk; zero inline threads at 2026-10-30 21:37 UTC. Its docstring-coverage warning remains recorded and is not an approval by maintainers. Native-policy skips are not native runtime validation, and the local test-budget/coverage limits above remain.

Summary by CodeRabbit

  • New Features
    • Applying suggested role changes now supports up to 50 changed roles per history batch. Larger batches are refused before any updates are sent.
    • Interrupted operations report confirmed, unconfirmed, and unattempted changes. Earlier confirmed updates remain in place; the CLI does not automatically retry or roll back.
    • Documentation explains how to preview suggestions and apply individual role changes.

Refuse more than ROLE_HISTORY_LIMIT changed suggestions before any role
PUT so a recorded operation remains one listable history batch. Preserve
server-issued batch reuse for accepted operations.

Report confirmed writes, an unconfirmed role, unattempted roles and the
known batch before propagating the original failure. Do not replay or
roll back a request whose response may have been lost.

Add real-handler coverage for the 50/51 boundary, whole-batch restore,
no-op exclusions, partial refusal and lost acknowledgements. Document
the apply limit and recovery contract.
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c34d26f and a92afee.

Files selected for processing: 12
  • docs-site/src/content/docs/guides/integrations.md
  • docs-site/src/content/docs/ja/guides/integrations.md
  • docs-site/src/content/docs/ko/guides/integrations.md
  • docs-site/src/content/docs/reference/cli/agents.md
  • docs-site/src/content/docs/ru/guides/integrations.md
  • docs-site/src/content/docs/zh-cn/guides/integrations.md
  • scripts/test-layout/layout.json
  • src/cli/agent.ts
  • structure/cli-management.md
  • tests/cli/cli-headless-parity.test.ts
  • tests/cli/cli-role-suggestion-history.test.ts
  • tests/fixtures/test-layout-expected.json

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

Walkthrough

Changes

The CLI now limits roles suggest --apply to 50 changed roles, applies accepted changes sequentially with a server-issued history batch, and reports write progress if an operation is interrupted. Documentation and tests describe and verify these behaviors.

Role suggestion apply

Layer / File(s) Summary
Apply safeguards and reporting
src/cli/agent.ts, docs-site/src/content/docs/reference/cli/agents.md, structure/cli-management.md, docs-site/src/content/docs/guides/integrations.md, docs-site/src/content/docs/ja/guides/integrations.md, docs-site/src/content/docs/ko/guides/integrations.md, docs-site/src/content/docs/ru/guides/integrations.md, docs-site/src/content/docs/zh-cn/guides/integrations.md
The CLI rejects applies with more than 50 changed roles before sending updates. Accepted changes use the server-issued batch. On interruption, the CLI reports confirmed, unconfirmed, and unattempted roles and preserves the original error. The documentation describes the limit and recovery behavior.
History apply validation
tests/cli/cli-role-suggestion-history.test.ts, tests/cli/cli-headless-parity.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
Tests cover the limit, excluded roles, previews, batch restoration, and partial or uncertain writes. The headless parity fixture expects the batch in role updates. Test-layout mappings include the new history test.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AgentCLI
  participant ManagementAPI
  AgentCLI->>ManagementAPI: Submit changed role update
  ManagementAPI-->>AgentCLI: Return server-issued history batch
  loop Remaining changed roles
    AgentCLI->>ManagementAPI: Submit next update with batch
    ManagementAPI-->>AgentCLI: Return write result
  end
Loading

Possibly related PRs

  • lidge-jun/opencodex#7693: Adds the role-model history journal and batch grouping that this change uses for role-suggestion applies.

Suggested reviewers: lidge-jun, ingwannu

Merge Risk: Minimal · up to a92af

No merge-blocking issue was identified. Proceed with normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: preserving the server-issued batch for ocx agent roles suggest --apply. This matches the implementation and the PR objective. The additional limit and r…
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.
Full details: Title check

Explanation

The title clearly identifies the main change: preserving the server-issued batch for ocx agent roles suggest --apply. This matches the implementation and the PR objective. The additional limit and recovery behavior does not need to appear in the title.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 3 files. (9 skipped: 9 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • 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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 30, 2026
@luvs01
luvs01 marked this pull request as ready for review October 30, 2026 21:14
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer dev-integration decision: the owner (@lidge-jun) authorized admin integration of this PR into dev as part of the post-v2.95 batch.

Cross-platform coverage comes from the batched dev lane=all run after the batch.

@lidge-jun
lidge-jun merged commit afafe6d into lidge-jun:dev Oct 31, 2026
43 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants