Repository navigation
feat: ask before handing a requirement to an SOP - #157
crispinhu27 wants to merge 5 commits into
Conversation
The assistant could start an SOP the user never asked for, and a timeout could run the same draft twice.
The view tour treats a nav entry without a title as a broken page.
Robert0622
left a comment
There was a problem hiding this comment.
Reviewed 57be7b4. I reproduced a blocking proposal-decision race in chatbot/src/useChat.js (applyDraft):
[P1] A stale proposal card can execute after the stored decision is declined, muted or expired. applyDraft fetches the fresh session, but only calls the /proposals acceptance endpoint when !draft.decision. If the server now returns decision: "declined", "muted" or "expired", it skips acceptance and proceeds directly to /mcp. For example, one tab lets the proposal expire (or another participant declines it), while a second tab still displays the old executable card. Clicking that card starts the SOP even though the decision already rejected it, potentially alongside the timeout's Main Bot continuation.
Local reproduction: I added three temporary hook regression cases to the existing Vitest fixture. Each returned a freshly fetched draft with one of those decisions, called applyDraft, and asserted zero /mcp calls. All three failed: each made one /mcp call. The existing test only covers the different race where /proposals itself returns 409.
Please reject all terminal negative decisions before executing any write, refresh the displayed receipt, and add these regression cases. Ideally validate the proposal decision and consume its execution atomically on the server as well; accepting an already accepted but unsaved card in two clients currently has no single execution claim. I did not exercise a paid-model turn for this finding: the reproduction uses the real React hook with mocked HTTP responses and no external writes.
|
LoopForge PR Review — head Verdict: ✅ Pass
What I ran (built
Not run here: the Docker/MySQL e2e ( |
Robert0622
left a comment
There was a problem hiding this comment.
复审当前 head 57be7b4:此前的 P1 尚未修复。本次再次运行真实 useChat hook 的回归复现,declined、muted、expired 三个用例全部失败,每种状态都仍发送一次 /mcp 写请求。
旧标签页保留可执行的建议卡片,另一个标签页拒绝建议或等待超时后,旧标签页点击执行,applyDraft 虽读取最新草稿,但在已有负面 decision 时跳过 /proposals 的确认,直接执行写入。这可能同时启动 SOP 和超时后的助手续作。
请在写入前拒绝这些终态、刷新卡片并补上三项回归,最好由服务端原子校验并认领执行。当前保留 Changes requested,不应因后续 Pass 概述而视为已解决。本次复现使用模拟 HTTP 和真实 React hook,没有启动真实 SOP。
|
✅ 结论:Approve(通过) 复审 head 上次 Changes requested 针对的是 此前 P1(旧卡片在 declined/muted/expired 后仍执行)已修复
服务端认领情况
AGENTS.md 检查:新表 验证
|
Robert0622
left a comment
There was a problem hiding this comment.
复审 76c8cbe(含修复 bf8996f):上次的 P1 已修复,approve。合入前需要先解决与 main 的冲突,见文末。
修复确认
applyDraft在执行写入前会拒绝decision不是accepted的建议,并用最新会话刷新卡片;新增的declined/muted/expired三项 hook 回归覆盖了这个路径。- 真实 Chromium 实测(临时平台 + MySQL):打开带建议卡片的会话后,另一端通过 API 把建议改成
declined或muted,再点旧卡片的「走 SOP」:/mcp写请求数为 0- 页面提示「这条建议已经处理过,不能执行。」
- 卡片刷新为「未采纳」/「未采纳,本会话不再建议」
- 无页面错误
expired只能由服务端超时产生,没法通过 API 构造,由单测覆盖。
其他验证
- chatbot vitest 306/306、
npm run build、go vet ./internal/...、go testinternal/httpapi、repo、domain 与./web/...全部通过。 e2e/api/70-assistant-proposals.test.mjs4/4 通过。- CodeBuddy IOA 真实模型用例
api/40-model.test.mjs8/8 通过,包括 AI 助手问答、SSE、草稿确认后续作不重复执行。 - CI 16/16 通过。
非阻塞说明
如果另一个标签页已经 accept、正在执行,旧卡片读到 accepted 且尚未 saved,仍会再执行一次(实测 /mcp 调用 1 次)。普通草稿在 main 上也存在同样的双标签页竞态,不是本 PR 引入的。后续可以考虑由服务端原子认领执行。
合入前
#169 已合入 main,重构了设置导航和资产页。本 PR 在 web/app.js、web/index.html、web/i18n-en.js、e2e/ui/01-views.spec.mjs 有冲突。解决时请把「AI 助手设置」接到新的设置侧边栏,并重跑 ui/01-views。
Keep the AI assistant settings page inside the consolidated settings navigation. Co-authored-by: Cursor <cursoragent@cursor.com>
What changed
The assistant could start an SOP the user never asked for, and a timeout could run the same draft twice. A requirement the assistant chooses to hand to an SOP is now a proposal unless the user @ mentioned an SOP or named one. Roles are still dispatched directly. Full access starts the SOP immediately; confirm and assisted approval ask first. A start proposal waits three minutes, then the assistant does the work itself. Accepting is recorded before the draft runs, so a timeout that already handed the work back cannot run it again.
Settings for this live on a new AI assistant page, after Bot management. Proactive role dispatch and proactive SOP use are off by default. Custom rules are stored per user and added to every turn. They do not override approval mode.
Scope
Compatibility
New table
assistant_preferencesis created by AutoMigrate. No manual SQL. New routes:GET/PUT /api/assistant/preferencesandPOST /api/assistant/sessions/{id}/proposals. Existing conversations and role dispatch are unchanged. Full access still starts an SOP without a card.Validation
The Go tests, the web settings test, and 43 chatbot tests passed. The public-content check passed. The e2e file ran against MySQL 8 in Docker: preferences persisted, an oversized rule was rejected, decline returned the follow-up and a second decision returned 409, accept recorded the decision without a follow-up, and a plain draft was rejected. Signed in on the local platform and saved the AI assistant settings; a reload kept the switch and the rule.
Not run:
go test ./...,go vet ./...,bash local/scripts/validate.sh,bash local/scripts/smoke-install.sh,bash local/scripts/e2e.sh,bash scripts/scan-secrets.sh. No live model turn was used to generate a proposal card.Safety