fix(team-mode): clarify team tool call semantics#4694
Conversation
|
All contributors have signed the CLA. Thank you! ✅ |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e74293af0
ℹ️ 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".
There was a problem hiding this comment.
1 issue found across 6 files
Confidence score: 3/5
- There is a concrete behavior mismatch in
src/features/team-mode/tools/tasks.ts: the tool advertisesreassign, but the current logic does not apply reassignment (claimedignoresargs.owner, and the non-claimed flow only validates current ownership). - Because this is a medium-severity, reasonably confident issue (6/10, 7/10) with user-facing impact, there is some regression risk if consumers rely on reassignment semantics.
- Pay close attention to
src/features/team-mode/tools/tasks.ts- reassignment behavior appears incomplete and may cause task ownership updates to be skipped.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
4e74293 to
0408cff
Compare
|
I have read the CLA Document and I hereby sign the CLA |
|
Thanks @WarGloom, and for the careful write-up. There are two distinct things in here: a small parser fix (normalizing empty optional strings to undefined in The description rewrites change the model-facing tool semantics (what the LLM reads to decide how to call these tools), and the diff also drops the strict empty- I am not closing this, just holding it for the maintainer's review. If you would like to land the parser fix sooner, splitting the empty-string normalization into its own small PR (with the |
0408cff to
ce4541c
Compare
|
Updated this PR after review feedback. Changes in the new push:
Verification:
|
Summary
team_createtolerate empty optional strings as omitted before enforcing the exact-oneteamName/inline_specrule.Changes
team_create, task, message, query, and shutdown tool descriptions with concise model-facing semantics.undefinedinteam_createargument parsing.team_createcalls where tool callers include empty optional fields around validteamNameorinline_specinputs.Screenshots
N/A - tool schema/parser behavior only.
Testing
bun test src/features/team-mode/tools/tasks.test.ts src/features/team-mode/tools/query.test.ts src/features/team-mode/tools/messaging.test.ts src/features/team-mode/tools/lifecycle.test.ts src/features/team-mode/tools/lifecycle-inline-spec.test.ts bun run typecheck bun run buildRelated Issues
leadSessionIdfallback, but does not coverteamName/inline_specempty optional pollution or result/message visibility descriptions.Summary by cubic
Clarified team-mode semantics, especially task ownership and async messaging, so callers know what to pass and what each tool returns.
team_createnow treats empty optional strings as omitted while keeping the exactly-one-ofteamName/inline_specrule.Bug Fixes
team_create: normalize empty strings forteamName,inline_spec, andleadSessionIdto undefined before validation; tests cover both named and inline specs.{ output: string }.Refactors
team_task_updatederives ownership from the caller session and ignores/removes theownerarg. Claim by settingstatus: "claimed"or"in_progress"; guidance and tests updated.team_send_messageis async and returns delivery metadata (not replies/history).team_status/task tools do not return message bodies. Clarifiedteam_deletecleanup and shutdown request/approve/reject flow, and when to omit empty optional args;team_createnotes returnedteamRunIdand monitoring viateam_status/team_task_list.Written for commit ce4541c. Summary will update on new commits.