Repository navigation
feat(helper): update the current session goal through Cindy tools - #8362
randomradio wants to merge 3 commits into
Conversation
Signed-off-by: Ian Chenyang Zhao <chzhao8904@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
[Medium impact] The PR adds goal access for agents, limited to the current task on its Cindy host. 建议修复目标写入的权限复核窗口后再合并。 Findings
Fix with agent prompt### Issue 1
apps/desktop/src/main/mcp-integrations/sessionGoal.ts:29-34
`update_goal` 只在进入 `updateGoal` 前调用一次 `assertTaskActionCurrent()`。伙伴自动轮获准后,控制器还会等待已有写入并读取目标。如果用户此时开启 Plan 模式,旧许可已经失效,但控制器仍会保存新目标。返回后的检查只核对账号、实例和控制器,因此还可能报告成功。
请把本次调用的权限检查传到控制器,在等待结束后、提交目标及恢复执行前重新检查。
**How this was verified:** 获准调用的 Plan 代次会在切换开始时失效,但 `updateGoal` 等待后直接写入,未再次检查该许可。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
| if (objective !== undefined) await assertTaskActionCurrent(); | ||
| assertSessionCurrent(); | ||
| if (deps.getController() !== controller) throw new GoalControllerDisposedError(); | ||
| const goal = objective === undefined | ||
| ? await controller.getStatus(sessionId) | ||
| : await controller.updateGoal(sessionId, { objective }); |
There was a problem hiding this comment.
update_goal 只在进入 updateGoal 前调用一次 assertTaskActionCurrent()。伙伴自动轮获准后,控制器还会等待已有写入并读取目标。如果用户此时开启 Plan 模式,旧许可已经失效,但控制器仍会保存新目标。返回后的检查只核对账号、实例和控制器,因此还可能报告成功。
请把本次调用的权限检查传到控制器,在等待结束后、提交目标及恢复执行前重新检查。
How this was verified: 获准调用的 Plan 代次会在切换开始时失效,但 updateGoal 等待后直接写入,未再次检查该许可。
Knowledge Base Used: Agent tool and computer integrations
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/mcp-integrations/sessionGoal.ts
Line: 29-34
Comment:
**权限收紧后仍修改目标**
`update_goal` 只在进入 `updateGoal` 前调用一次 `assertTaskActionCurrent()`。伙伴自动轮获准后,控制器还会等待已有写入并读取目标。如果用户此时开启 Plan 模式,旧许可已经失效,但控制器仍会保存新目标。返回后的检查只核对账号、实例和控制器,因此还可能报告成功。
请把本次调用的权限检查传到控制器,在等待结束后、提交目标及恢复执行前重新检查。
**How this was verified:** 获准调用的 Plan 代次会在切换开始时失效,但 `updateGoal` 等待后直接写入,未再次检查该许可。
**Knowledge Base Used:** [Agent tool and computer integrations](https://app.greptile.com/-/switch-tenant?tenant=xindong&next=%2Fcustom-context%2Fknowledge-base%2Fmakecindy%2Fcindy%2F-%2Fdocs%2Fagent-tool-and-computer-integrations.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cae2cff3be
ℹ️ 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".
| if (deps.getController() !== controller) throw new GoalControllerDisposedError(); | ||
| const goal = objective === undefined | ||
| ? await controller.getStatus(sessionId) | ||
| : await controller.updateGoal(sessionId, { objective }); |
There was a problem hiding this comment.
Recheck action authorization before committing a Goal edit
If a Bot or automatically triggered call loses its permission, Plan, or input generation while GoalController.updateGoal() awaits lifecycle persistence or storage.get(), it still writes the new objective and reports success. The action guard runs only before entering the controller; the subsequent checks cover account/session identity, not action authorization. An in-memory reproduction committed the edit despite an expired TaskAction scope. This contradicts the post-admission revocation contract in agent-user-parity.md; pass the captured action guard into the controller and recheck it at the mutation/resume boundaries.
AGENTS.md reference: AGENTS.md:L55-L58
Useful? React with 👍 / 👎.
| 'steer_session', | ||
| 'stop_session_turn', | ||
| 'set_session_runtime', | ||
| 'update_goal', |
There was a problem hiding this comment.
Require confirmation before autonomously resuming a Goal
For an autonomous companion turn with an existing paused Goal, changing the objective queues a Goal resume, but registering update_goal only in TASK_ACTION_TOOLS leaves taskActionRisk() returning undefined. Consequently, a Full Access autonomous turn can restart the user's paused execution without owner confirmation. An in-memory reproduction resumed and dispatched the Goal with zero confirmations, whereas the equivalent message_session_task(mode='resume') requested confirmation and was denied. teammate-authority.md requires that additional confirmation for resuming paused execution; capture the current Goal state and route this effect through the existing resume risk gate before editing.
AGENTS.md reference: AGENTS.md:L73-L74
Useful? React with 👍 / 👎.
Signed-off-by: Ian Chenyang Zhao <chzhao8904@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24d4a03376
ℹ️ 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".
| const assertCurrent = this.deferredManualResumeContexts.get(sessionId)?.assertCurrent; | ||
| void this.resumeGoal(sessionId, { assertCurrent }).catch((error) => { | ||
| this.cancelDeferredManualResume(sessionId, { restoreUsageResume: true }); |
There was a problem hiding this comment.
Preserve deferred Goal resume across normal turn completion
When a Bot—including an owner-requested turn—changes a paused or blocked Goal from its currently running session, the resume is deferred. The saved assertCurrent checks the live task-action input key, which getTaskActionInputState() derives from { input, executing }; normal turn completion clears that input in agent-input-coordinator.ts:4280. Consequently, this retry throws TASK_ACTION_AUTHORIZATION_EXPIRED and cancels the resume even when the account and permissions are unchanged and no replacement input arrived. An in-memory run of the real authorizer, callback and controller returned ok: true but left the edited Goal paused with zero sends, unlike the editor path. Preserve the admitted resume through expected completion of its originating turn while retaining checks for actual revocation or superseding input.
AGENTS.md reference: AGENTS.md:L57-L58
Useful? React with 👍 / 👎.
|
本 PR 命中维护者确认门(product:feat 类型;rules:改到审查判据 docs/product-rules/agent-user-parity.md;arch:核心路径 450 行 ≥ 150),自动审查流程不会继续审查或合并。 本次改动为 请维护者直接在本 PR 上 Approve 确认(需要修改则 Request Changes)。讨论 issue:#8365 |
…ions Signed-off-by: Ian Chenyang Zhao <chzhao8904@gmail.com>
这次改了什么
摘要
Agents can read and replace their current session’s existing Goal Mode objective through
cindy_helper.get_goal {}returns the current goal, limits and counters, orgoal: null. Reading does not resume execution.update_goal { objective }reuses the goal editor’s controller, persistence, edit marker and status broadcasts.变更类型
feat新功能fix缺陷修复refactor/perf重构或性能优化docs/test/chore文档、测试或工程维护范围
UI 变化
不涉及。The API reuses existing status broadcasts, editor behavior and permission cards.
怎么验证的
自动验证
Desktop: 1,162 tests passed across 10 files. The MCP package’s 177 tests passed during the API implementation.
The MCP package has no typecheck script;
tsc --noEmitsupplies its type check.测试价值
All modified test files use the unit tier with injected memory dependencies or in-memory MCP transport. Each file also passed alone with one worker. Times include command startup.
goal-host/__tests__/controller.test.tscreateSessionGoalCallbacksand the controller persist edits, broadcast state and dispatch the new objective.agent-user-parity.md,teammate-authority.md: earlier tests omitted MCP lifecycle revocation and confirmed goal snapshots.GOAL_NOT_FOUND.maker-ipc/__tests__/taskActionAuthorization.test.tsteammate-authority.md, reviews r4436235377 and r4436295232: previous cases lacked goal risk facts and the completion transition.maker-ipc/__tests__/agent-input-coordinator.test.tsteammate-authority.md, r4436295232: clearing active input previously lost the distinction between completion and supersession.maker-host/__tests__/mcpToolApprovalPolicy.test.tsagent-user-parity.md: the existing mutation matrix omitted the new tool.update_goalcalls.mcp-integrations/__tests__/collabSendOutcome.test.tslizi-mcps/src/__tests__/lizi_xdtHelperMcpServer.test.tslist_tools/call_toolmessages expose tools, bind the live caller and enforce arguments/authorization.agent-user-parity.md: no goal tool protocol coverage existed.Regression cases reproduced the authorization and confirmation failures before their fixes. Both owner and autonomous deferred resumes also failed under the old input-bound guard and passed with the completion guard.
手工验证
Reviewed the complete diff. Independent review checked confirmation binding, delayed state checks, completion and superseding input.
未执行的验证
No live Desktop, Windows or Mobile app verification. The full repository suite was not run. CI remains required.
风险
风险分类
影响与回滚
PRECONDITION_FAILEDdoes not guarantee unchanged state; callers must useget_goalbefore retrying.提交前检查
git commit -s)