Skip to content

fix(codex): share trigger-only delegation guidance with v1 - #4034

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/v1-delegation-guidance-20260908
Closed

fix(codex): share trigger-only delegation guidance with v1#4034
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:agent/v1-delegation-guidance-20260908

Conversation

@luvs01

@luvs01 luvs01 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

The v1 top-effort path maintains a separate legacy delegation message, so it misses the clearer boundaries already present in the shared proactive recommendation. Use MULTI_AGENT_MODE_HINT_RECOMMENDATION.text through the existing PROACTIVE_MULTI_AGENT_MODE_TEXT export.

V1 guidance at max or ultra now says that only the separate delegation-request trigger changes; user instructions, authority, task scope, and collaboration-tool rules still apply. The existing surface detection, opt-out and effort conditions continue to select this guidance. V2 metadata and native preset settings retain their current behavior. This updates advisory wording, without claiming to enforce agent behavior or revoke instructions already in conversation history.

The regression covers the exact recommendation and an old-wording → new-wording → repeated-new-wording continuation. The policy invariant and the existing v1 paragraph in all eight language guides describe the same contract. Released legacy preset bytes in the policy module remain unchanged.

Verification

  • Head eb835fe335c3449d08cb3183606d1cefc2230bc4, based on dev 29bb221c3cfad89e4920ac411c9681073929c152; Bun 1.4.0 on Windows.
  • Before the runtime change, the max, ultra, and historical-wording regression cases all failed because the old text was emitted.
  • bun run test -- --timeout 60000 --parallel=1 tests/codex-integration/multi-agent-compat.test.ts: 63 tests / 241 assertions passed, 9.24 seconds. This includes v1 opt-out/lower-effort and non-agent exclusions, v2 guidance, placement, and replay deduplication. Expected current and historical text are independent literals.
  • bun run typecheck, bun run privacy:scan, and git diff --check passed. The existing recommendation and byte-exact legacy preset module have no diff.
  • Documentation build passed: 425 pages, 21.32 seconds. The new paragraph was verified in the generated HTML for all eight languages without opening previews. Existing dependency assets were reused after matching lock hashes and dependency declarations.
  • Full contributor CI passed all 26 jobs on eb835fe33. CodeRabbit's current-head review covers all 11 files and reports no actionable comments or outstanding material risk. Its first request returned a service error; one retry completed successfully. The docstring-coverage warning is advisory; no production function was added. Independent read-only review also found no required corrections.

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.

Review readiness checklist

  • All CI tests are green on my local testing.
  • I pushed my PR to the latest dev commit.
  • I resolved all correct Codex and CodeRabbit findings.
  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cf6a5789-d04b-43fd-9ead-11470e4701bf

📥 Commits

Reviewing files that changed from the base of the PR and between 29bb221 and eb835fe.

📒 Files selected for processing (11)
  • docs-site/src/content/docs/fr/guides/sub-agent-surface.md
  • docs-site/src/content/docs/guides/sub-agent-surface.md
  • docs-site/src/content/docs/ja/guides/sub-agent-surface.md
  • docs-site/src/content/docs/ko/guides/sub-agent-surface.md
  • docs-site/src/content/docs/ru/guides/sub-agent-surface.md
  • docs-site/src/content/docs/tr/guides/sub-agent-surface.md
  • docs-site/src/content/docs/zh-cn/guides/sub-agent-surface.md
  • docs-site/src/content/docs/zh-tw/guides/sub-agent-surface.md
  • src/server/responses/collaboration.ts
  • structure/03_catalog-and-subagents.md
  • tests/codex-integration/multi-agent-compat.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

V1 proactive delegation guidance now uses the shared v2 recommendation text at max and ultra. Tests validate exact output and replay behavior. The catalog and localized guides describe the updated trigger conditions and unchanged rules.

Changes

V1 proactive delegation guidance

Layer / File(s) Summary
Shared recommendation wiring
src/server/responses/collaboration.ts
PROACTIVE_MULTI_AGENT_MODE_TEXT now reads from MULTI_AGENT_MODE_HINT_RECOMMENDATION.text. The V1 comment describes the trigger-only difference.
Compatibility validation
tests/codex-integration/multi-agent-compat.test.ts
Parameterized tests validate exact guidance at max and ultra. Replay tests verify that legacy and new guidance remain present once without duplication.
Documentation alignment
structure/03_catalog-and-subagents.md, docs-site/src/content/docs/*/guides/sub-agent-surface.md
The catalog and localized guides describe the shared guidance, trigger condition, applicable rules, and unchanged V1 configuration limits.

Priority: ⬇️ Low — Defer this change because it clarifies advisory v1 delegation guidance and localized documentation without changing runtime triggers, permissions, or product behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to eb835

V1 now uses the shared proactive delegation guidance at max and ultra effort while retaining existing authority, scope, and collaboration rules. The documented behavior and compatibility coverage support merging without an outstanding material risk.

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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. Analyzed 2 functions across 2 files. (9 skipped: 9 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: sharing trigger-only delegation guidance with v1. It matches the implementation and documentation updates.
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. Analyzed 2 functions across 2 files. (9 skipped: 9 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


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.

@luvs01

luvs01 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review head eb835fe335c3449d08cb3183606d1cefc2230bc4 against 29bb221c3cfad89e4920ac411c9681073929c152.

The runtime change aliases the existing v1 PROACTIVE_MULTI_AGENT_MODE_TEXT export to the already adopted MULTI_AGENT_MODE_HINT_RECOMMENDATION.text constant. The v1 max/ultra trigger, opt-out, surface detection, and existing v2 metadata/settings behavior stay intact. This is advisory wording; it does not enforce tool authority or erase historical developer messages. The released legacy preset literals are unchanged.

Independent-literal regressions fail on the baseline and pass after the change. The full compatibility file passed 63 tests / 241 assertions, including old v1 wording followed by one new guidance item and deduplication on the next replay. Typecheck, privacy scan, diff check, and the 425-page documentation build passed; all eight changed language paragraphs were checked in generated HTML. An independent read-only review found no required corrections.

The pull request remains draft pending full contributor CI and current-head review.

@github-actions github-actions Bot added the bug Something isn't working label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Oops, something went wrong! Please try again later. 🐰 💔

@luvs01

luvs01 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

The previous review request returned a service error; no formal review or inline findings were produced. Please retry that review for the unchanged head eb835fe335c3449d08cb3183606d1cefc2230bc4 against 29bb221c3cfad89e4920ac411c9681073929c152.

The focused compatibility file passed 63 tests / 241 assertions; typecheck, privacy scan, diff check, and the 425-page documentation build passed. Full contributor CI is running at https://github.com/luvs01/opencodex/actions/runs/34232583711. The existing recommendation module and released legacy literals remain unchanged; only v1's export now uses the shared recommendation, with regression and localized documentation updates.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

@luvs01 I will retry the review for #4034 against the specified head and base commits.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request has been marked Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

Hygiene

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 41 / 80

이 PR은 v1 다중 에이전트 안내 문장을, 이미 정책 모듈에 있는 공유 추천 문구와 같게 맞추려는 초안(draft) 변경입니다. 지금 dev HEAD 29bb221c3에서 src/server/responses/collaboration.tsPROACTIVE_MULTI_AGENT_MODE_TEXT는 여전히 예전 문장 배열을 직접 join합니다(“Any earlier instruction requiring an explicit user request…”, “do not serialize…”, “prefer spawning specialists…”). 한편 src/codex/multi-agent-mode-policy.ts에는 이미 MULTI_AGENT_MODE_HINT_RECOMMENDATION이 있고, 문구는 “Only the delegation trigger changes…”, “All existing user, authority, task-scope, and collaboration-tool rules continue to apply.”, “User requests override this hint.”처럼 트리거만 바꾸고 나머지 규칙은 유지한다고 더 또렷합니다. 이 PR은 v1의 PROACTIVE_MULTI_AGENT_MODE_TEXT를 그 공유 .text를 가리키게 바꿉니다. effort가 max/ultra일 때만 <multi_agent_mode>…를 넣는 조건, 표면 감지·opt-out, v2 메타/네이티브 프리셋 동작은 그대로 두고 조언 문구만 통일합니다.

왜 지금 dev 기준으로 보면 가치가 있을까요. tip 쪽에는 이미 #3993 등 proactive delegation 관련 랜딩이 있었고, 정책 모듈의 추천/레거시 바이트(LEGACY_OPENCODEX_MODE_HINTS, canonicalizeOpenCodexModeHint)는 HEAD에 있습니다. 그런데 v1 주입 경로만 옛 문장을 따로 들고 있어서, 가이드/구조 문서가 말하는 계약과 실제 v1 와이어 문구가 어긋날 수 있습니다. PR은 8개 언어 sub-agent-surface 가이드와 structure/03_catalog-and-subagents.md의 v1 문단을 같은 계약으로 고치고, tests/codex-integration/multi-agent-compat.test.ts에 max/ultra 정확 일치와 “옛 문구 → 새 문구 → 새 문구 재주입” 연속(replay) 회귀를 넣었습니다. 작성자 검증(해당 테스트 63/241, typecheck, privacy:scan, docs 425페이지)과 CodeRabbit/enforce-target/hygiene 통과는 보이지만, PR 본문도 full contributor CI·ready 체크는 아직이라고 했고 GitHub상 draft·mergeable_state blocked입니다.

범위는 작습니다. 런타임 파일은 collaboration.ts 한 곳의 export 연결 + 주석, 테스트, 문서입니다. 레거시 preset 바이트는 정책 모듈에 그대로 두어 이미 대화에 남은 옛 developer 메시지를 지우거나 강제 덮어쓰지 않습니다. inject 쪽 회귀도 히스토리의 legacy 한 번 + 새 문구 한 번을 유지한 채 재주입하지 않는지 확인합니다. types.ts/config.ts 분할 캠페인과도 무관해서 close-don't-rebase 대상이 아닙니다. 우선순위가 중간 아래인 이유는, 제품 tip(#4002 등)이나 런타임/할당량 버그보다 문서·조언 정렬이고, draft라 머지 트레인에 바로 넣기 이르기 때문입니다. 그래도 v1/v2 안내가 같은 문장을 쓰게 되면 사용자·메인테이너가 “트리거만 바뀌고 권한/범위 규칙은 남는다”를 오해할 여지가 줄어듭니다.

라인 collaboration.ts PROACTIVE_MULTI_AGENT_MODE_TEXT - HEAD의 로컬 문장 배열을 MULTI_AGENT_MODE_HINT_RECOMMENDATION.text로 바꾸면 v1/v2 추천이 한 출처를 쓰게 되어 계약이 맞는다(방향 좋음)
경로/심볼 multi-agent-mode-policy.ts - 레거시 바이트와 canonicalize는 그대로 두었다고 하니, 이미 나간 대화 히스토리와의 호환 의도는 유지된다
경로/심볼 multi-agent-compat.test.ts - 기댓값을 정책 모듈과 같은 리터럴로 고정하고 replay 업그레이드를 커버한 점은 유지할 것
경로/심볼 8-locale sub-agent-surface + structure doc - 문서 계약과 코드가 같이 움직이는 것은 맞지만, draft 체크리스트의 CI/ready가 비어 있으면 머지 전에 한 번 더 확인이 필요하다
경로/심볼 draft 상태 - mergeable_state blocked·ready 미체크이므로 지금 바로 landing PR에 넣지 말 것

메인테이너의 판단이 필요한 지점

  • 문구 변경만으로 “에이전트가 더 적극적으로 위임한다”는 체감/회귀가 있는지, 아니면 순수 문서·힌트 정렬로 볼지
  • fix(gui): use server-owned proactive delegation presets #3993 등 이미 올라간 proactive delegation 작업과 이 PR을 같은 스토리로 묶을지, 단독 소규모 docs/compat로 둘지
  • draft를 ready로 올리기 전에 Cross-platform CI 전체 그린을 필수로 둘지

너의 추천
초안 유지한 채 작성자가 checklist·전체 CI를 채운 뒤 ready로 전환하면, 충돌 없으면 dev에 작은 정렬 PR로 머지해도 됩니다. 지금은 머지하지 말고, 리뷰만 남겨 두세요. 닫을 중복도 아닙니다.

이 댓글은 grok-bot이 작성했습니다

@github-actions
github-actions Bot marked this pull request as ready for review September 8, 2026 14:01
lidge-jun added a commit that referenced this pull request Sep 8, 2026
* test(lib): make idle deadline reset timing deterministic

Carried from #4041 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* test: stabilize Windows fixture waits and case cleanup

Carried from #4015 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* test(codex): verify timeout termination without racing child timers

Carried from #4012 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* test(codex): hold prompt-probe admission through document edits

Carried from #4014 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* test(clients): bound transaction fixture child completion

Carried from #4004 (author luvs01) for maintainer-side CI and sequential integration.
Closes #4003

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(codex): retain overlapping multiline TOML terminators

Carried from #4039 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(cli): reject unsupported caps and report ignored legacy values

Carried from #4043 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(codex): share trigger-only delegation guidance with v1

Carried from #4034 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(codex): preserve settings when journal injection hashes are missing

Carried from #4006 (author luvs01) for maintainer-side CI and sequential integration.
Closes #4005

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* docs(cli): note that --injection still accepts none and minimal

CodeRabbit review on #4049: the effort reference documents caps only, so the
new rejection of none/minimal reads as a global removal. --injection keeps
both values because it sets the injection effort rather than a cap.

---------

Co-authored-by: t <a@b.com>
Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev in cfff026 via #4049, which carried all nine PRs of this train as one sequentially reviewed integration branch. Your commit is preserved in the squash body with a Co-authored-by trailer, so the contribution stays attributed to you.

Verification at the exact merged head 8c58bac0863f1f2ca2b430e1177a0e417b474c85: Cross-platform CI run 34256853348 finished with 19 jobs successful and 0 failures, bun x tsc --noEmit exit 0, and 312 focused tests passing across the ten affected files. An independent review confirmed the stacked tree is byte-identical to the union of the nine PR diffs.

Closing this PR because the change is already on dev. Thank you.

@lidge-jun lidge-jun closed this Sep 8, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Landed via #4049 at cfff026

@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants