From fcb370edfd23624c83cc7db4ac111ca73e20fa30 Mon Sep 17 00:00:00 2001 From: YikaJ Date: Wed, 22 Jul 2026 20:33:15 +0800 Subject: [PATCH] fix(squads): align parent issue status with agent-managed model (MUL-5156) (#5758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(squads): align parent issue status ownership with agent-managed model Squad leaders now open assigned parents to in_progress on first dispatch, keep them there while members work, and only move to in_review when overall completion is confirmed—matching ordinary agent status semantics without server auto-flips. Co-authored-by: Cursor * fix(squads): scope leader parent-status ownership to squad-assigned issues Review follow-up on the parent-status alignment change. Two boundaries were left ambiguous, both of which the change's own premise ("don't make the model resolve a contradiction in the prompt") argues should be closed in-place. 1. Status ownership was granted too widely. The leader briefing is injected on every leader path, keyed off is_leader_task — including the MUL-3724 case where an issue is assigned to a plain agent and a squad was merely @mentioned for help. The unqualified "Own the parent issue status" responsibility therefore also reached guest leaders, who could push another assignee's in-flight issue to in_review. buildSquadLeaderBriefing now takes ownsIssueStatus and selects between two variants of responsibility 6: the grant only when the issue's assignee is this squad, otherwise an explicit "do NOT change this issue's status". Quick-create passes false — no issue exists on that turn. Everything else in the protocol (roster, delegation, evaluation) is unchanged for both. 2. The comment-triggered path still contradicted itself. The runtime brief says "do not change status unless the comment explicitly asks", and a member's delivery comment never asks. Squads that dispatch by @mention create no child issues, so no child-done system comment exists to carry the explicit ask either — that parent would sit in in_progress indefinitely. writeWorkflowComment now names the protocol responsibility as the one exception for squad leaders. It is safe to state unconditionally because the grant is only present in the instructions when the server decided this squad owns the issue; for a guest leader the sentence has nothing to activate. Tests: two composition tests assemble both halves (server-side briefing + daemon-side CLAUDE.md) for one real scenario each, since asserting each half alone is how the original contradiction shipped. Plus execenv coverage that the carve-out appears only for leaders and the ordinary-agent rule stays absolute. Docs and the multica-squads skill / source map record the narrower contract. Co-authored-by: multica-agent --------- Co-authored-by: Cursor Co-authored-by: J Co-authored-by: multica-agent --- apps/docs/content/docs/assigning-issues.mdx | 4 +- .../docs/content/docs/assigning-issues.zh.mdx | 4 +- apps/docs/content/docs/issues.mdx | 4 +- apps/docs/content/docs/issues.zh.mdx | 4 +- apps/docs/content/docs/squads.mdx | 11 +- apps/docs/content/docs/squads.zh.mdx | 11 +- .../daemon/execenv/runtime_config_sections.go | 33 ++++- .../daemon/execenv/runtime_config_test.go | 72 ++++++++++ server/internal/handler/daemon.go | 20 ++- server/internal/handler/issue_child_done.go | 10 +- .../handler/issue_child_done_stage_test.go | 6 +- server/internal/handler/squad_briefing.go | 68 +++++++-- .../internal/handler/squad_briefing_test.go | 85 ++++++++++- .../squad_parent_status_contract_test.go | 135 ++++++++++++++++++ .../builtin_skills/multica-squads/SKILL.md | 20 ++- .../references/squad-source-map.md | 24 +++- .../multica-working-on-issues/SKILL.md | 10 ++ .../working-on-issues-source-map.md | 8 +- 18 files changed, 487 insertions(+), 42 deletions(-) create mode 100644 server/internal/handler/squad_parent_status_contract_test.go diff --git a/apps/docs/content/docs/assigning-issues.mdx b/apps/docs/content/docs/assigning-issues.mdx index 7b3cc928f..2197f7ffc 100644 --- a/apps/docs/content/docs/assigning-issues.mdx +++ b/apps/docs/content/docs/assigning-issues.mdx @@ -50,7 +50,9 @@ When a non-Backlog issue is assigned to an agent, Multica immediately does the f 1. Enqueues a `queued` `task` with priority inherited from the issue, routed to the runtime where the agent lives. 2. The agent's daemon picks up the `task` on its next poll and transitions it to `dispatched`. 3. The agent starts working and the `task` moves to `running`; on completion it becomes `completed` or `failed`. -4. During execution the agent can change the issue's status, post comments, and edit fields — these actions appear under the agent's identity. +4. During execution the agent is expected to manage the issue's status (typically `todo` → `in_progress`, then `in_review` when it has delivered), post comments, and edit fields — these actions appear under the agent's identity. Task lifecycle and issue status are separate: completing a `task` does not by itself change the issue status. + +The same status expectations apply when the assignee is a [squad](/squads): Multica enqueues the **leader**, who should move the parent issue to `in_progress` on the first turn and keep it there while members work. The leader only moves the parent to `in_review` once the overall goal is met — a successful first dispatch is not completion. **If the agent is offline**, the `task` waits in the queue — **it times out and fails after 5 minutes** with reason `runtime_offline`. For retryable sources (assign, @-mention, chat), Multica automatically re-enqueues it. See [**Tasks**](/tasks) for the full retry rules. diff --git a/apps/docs/content/docs/assigning-issues.zh.mdx b/apps/docs/content/docs/assigning-issues.zh.mdx index 89ed29a6c..df0a25da9 100644 --- a/apps/docs/content/docs/assigning-issues.zh.mdx +++ b/apps/docs/content/docs/assigning-issues.zh.mdx @@ -50,7 +50,9 @@ multica issue assign MUL-42 --unassign 1. 入队一个 `queued` 状态的 `task`，优先级继承自 issue，路由到该智能体所在的运行时 2. 该智能体的守护进程下次轮询时把 `task` 领走，状态变成 `dispatched` 3. 智能体开始执行，`task` 转成 `running`；完成后转成 `completed` / `failed` -4. 执行过程中智能体可以改 issue 状态、发评论、改字段——这些动作以智能体的身份出现 +4. 执行过程中智能体应按约定管理 issue 状态（通常是 `todo` → `in_progress`，交付后再到 `in_review`）、发评论、改字段——这些动作以智能体的身份出现。`task` 生命周期和 issue 状态是分开的：`task` 完成本身不会自动改 issue 状态 + +当 assignee 是 [小队](/squads) 时，同样的状态约定适用：Multica 入队的是**队长**，队长首次接单时应把父 issue 推到 `in_progress`，并在成员干活期间保持该状态。只有整体目标达成后，队长才把父 issue 推到 `in_review`——第一次成功分发不等于完成。 **如果智能体离线**，`task` 会在队列里等——**5 分钟没被领走就超时失败**，失败原因 `runtime_offline`。对可重试的来源（分配、@ 提及、对话），Multica 会自动重新排队；完整重试规则见 [**执行任务**](/tasks)。 diff --git a/apps/docs/content/docs/issues.mdx b/apps/docs/content/docs/issues.mdx index ba339d10c..2eeca477d 100644 --- a/apps/docs/content/docs/issues.mdx +++ b/apps/docs/content/docs/issues.mdx @@ -11,7 +11,7 @@ An issue is a self-contained unit of work in Multica — a bug, a new feature, a ## Assigning an issue to an agent -[Assigning](/assigning-issues) an issue to an agent hands that work over to it. The agent **starts automatically** — executing within seconds, reporting progress in comments, and flipping the status to done when finished. The only difference from handing work to a teammate is that an agent doesn't go offline, doesn't need reminders, and is available 24/7. +[Assigning](/assigning-issues) an issue to an agent hands that work over to it. The agent **starts automatically** — Multica enqueues a `task` within seconds, the agent reports progress in comments, and during the run it is expected to update the issue status (typically `todo` → `in_progress` → `in_review`). The only difference from handing work to a teammate is that an agent doesn't go offline, doesn't need reminders, and is available 24/7. For agent identity, configuration, and where they run, see [Agents](/agents). @@ -33,7 +33,7 @@ Multica has seven statuses. **Any status can move directly to any other** — Mu | `blocked` | Stuck on an external factor | | `cancelled` | Cancelled | -Once an issue is assigned to an agent, the agent automatically moves the status from `backlog` / `todo` to `in_progress`, then to `done` on completion. You can also change it manually at any time. +Once an issue is assigned to an agent, the agent is expected to move the status from `backlog` / `todo` to `in_progress` when it starts, then to `in_review` when it has delivered the work. Status updates are made by the agent through the Multica CLI during the run — the server does **not** flip issue status when a `task` starts or completes. `done` is usually a human confirmation or an integration (for example a PR with close intent that merges). You can also change status manually at any time. ## Priority diff --git a/apps/docs/content/docs/issues.zh.mdx b/apps/docs/content/docs/issues.zh.mdx index f5c56af63..862d903df 100644 --- a/apps/docs/content/docs/issues.zh.mdx +++ b/apps/docs/content/docs/issues.zh.mdx @@ -11,7 +11,7 @@ Issue（工作项）是 Multica 里一个独立工作的单位——一条 bug ## 把 issue 分配给智能体 -把 issue [分配](/assigning-issues) 给某个智能体等于把这项工作交给它。智能体会**自动开工**——在几秒内开始执行、在评论里汇报进展、完成后把状态改到 done。和给同事派活的区别只在于：它不下线、不需要你提醒、7×24 可用。 +把 issue [分配](/assigning-issues) 给某个智能体等于把这项工作交给它。智能体会**自动开工**——Multica 在几秒内入队一个 `task`，智能体在评论里汇报进展，并在执行过程中按约定更新 issue 状态（通常是 `todo` → `in_progress` → `in_review`）。和给同事派活的区别只在于：它不下线、不需要你提醒、7×24 可用。 智能体的身份、配置、运行位置详见 [智能体](/agents)。 @@ -33,7 +33,7 @@ Multica 提供七种状态。**任何状态可以直接改到任何状态**— | `blocked` | 被外部因素卡住 | | `cancelled` | 已取消 | -把 issue 分配给智能体后，智能体会自动把状态从 `backlog` / `todo` 推到 `in_progress`，完成后推到 `done`。你也可以随时手动改。 +把 issue 分配给智能体后，智能体开工时应把状态从 `backlog` / `todo` 推到 `in_progress`，交付工作后再推到 `in_review`。这些状态变更由智能体在运行中通过 Multica CLI 显式写入——`task` 开始或完成时，服务端**不会**自动改 issue 状态。`done` 通常留给人工确认，或由集成完成（例如带 close intent 的 PR merge）。你也可以随时手动改。 ## 优先级 diff --git a/apps/docs/content/docs/squads.mdx b/apps/docs/content/docs/squads.mdx index f8eb8323b..2753f22a0 100644 --- a/apps/docs/content/docs/squads.mdx +++ b/apps/docs/content/docs/squads.mdx @@ -62,9 +62,10 @@ When a non-Backlog issue is assigned to a squad, Multica immediately enqueues a 1. **Leader claims the task.** The agent runtime picks up the task on its next poll, same as any other agent assignment. 2. **Leader is briefed.** On claim, Multica appends three sections to the leader's system prompt — see [What the leader sees on every turn](#what-the-leader-sees-on-every-turn) below. -3. **Leader posts one delegation comment.** The comment `@`-mentions the chosen member(s) using the exact mention markdown from the roster — that mention triggers a new `task` for each mentioned agent. -4. **Leader records its evaluation** via `multica squad activity action --reason "..."`. This writes an entry to the issue's activity timeline so humans can see the leader actually evaluated the trigger. -5. **Leader stops.** The leader does not do the implementation itself. When the delegated member posts back, the leader is re-triggered to read the update and either delegate the next step, escalate, or stay silent. +3. **Leader moves the parent to `in_progress`.** Same agent-managed status contract as a direct agent assignment: the first turn should leave `todo` / leave `backlog`-promoted work as `in_progress`. Dispatching members is not completion — the parent stays `in_progress` while the squad works. +4. **Leader posts one delegation comment.** The comment `@`-mentions the chosen member(s) using the exact mention markdown from the roster — that mention triggers a new `task` for each mentioned agent. +5. **Leader records its evaluation** via `multica squad activity action --reason "..."`. This writes an entry to the issue's activity timeline so humans can see the leader actually evaluated the trigger. +6. **Leader stops.** The leader does not do the implementation itself. When the delegated member posts back — or when a sub-issue / stage barrier closes — the leader is re-triggered to read the update and either delegate the next step, escalate, move the parent to `in_review` once the overall goal is met, or stay silent. `done` is left to a human reviewer or existing integrations (for example a PR with close intent that merges). If the issue is in **Backlog**, the leader is not triggered — Backlog is a parking lot, same rule as for direct agent assignment. @@ -72,7 +73,9 @@ If the issue is in **Backlog**, the leader is not triggered — Backlog is a par On each squad-leader run, three blocks are appended to the leader's instructions: -- **Squad Operating Protocol** — a hard-coded rule set: read the issue, delegate by `@`-mention, be terse (don't restate the issue body — the assignee can read it), record an evaluation every turn, and **stop after dispatching**. This protocol is system-managed and not editable. +- **Squad Operating Protocol** — a hard-coded rule set: read the issue, move the parent to `in_progress` on the first turn, delegate by `@`-mention, be terse (don't restate the issue body — the assignee can read it), record an evaluation every turn, **stop after dispatching**, and only move the parent to `in_review` once the overall goal is met. This protocol is system-managed and not editable. + + The status half of that protocol is **scoped to issues actually assigned to this squad**. A leader woken by an `@squad` mention on someone else's issue gets the same roster and delegation rules, but is told explicitly **not** to touch that issue's status — status stays with the issue's own assignee. - **Squad Roster** — the leader's self-row plus one row per non-archived member. Each row carries the exact mention markdown (`[@Name](mention://agent/)` or `[@Name](mention://member/)`) the leader should paste — typing a plain `@name` won't trigger anyone. - **Squad Instructions** — your custom guidance for this squad (set on the squad detail page or via `multica squad update --instructions`). Use this for routing rules ("send DB work to Alice, frontend to Bob"), escalation policies, or anything else the leader needs to know that isn't already in the issue. diff --git a/apps/docs/content/docs/squads.zh.mdx b/apps/docs/content/docs/squads.zh.mdx index b13a54d33..b378cd78c 100644 --- a/apps/docs/content/docs/squads.zh.mdx +++ b/apps/docs/content/docs/squads.zh.mdx @@ -62,9 +62,10 @@ multica squad member add --member-id --type agen 1. **队长领走 task。** 队长所在的 daemon 在下次轮询时把 task 领走，和普通智能体的分配流程一样。 2. **队长拿到 briefing。** 领走的瞬间，Multica 会在队长的系统提示后面追加三段内容——详见下文 [队长每次执行看到的内容](#队长每次执行看到的内容)。 -3. **队长发一条"派活"评论。** 评论里用 roster 里给好的 mention markdown `@` 选中的成员——这个 `@` 会触发被派的成员入队新 `task`。 -4. **队长记录 evaluation：** `multica squad activity action --reason "..."`。这一行会写进 issue 的 activity 时间线，方便人类回溯队长确实评估过这一次触发。 -5. **队长停下。** 派完活，队长**不动手干活**。当被派的成员有回复时，队长会被自动唤醒，决定下一步：继续派活、上抛给人类、还是保持沉默。 +3. **队长把父 issue 推到 `in_progress`。** 和直接分配给智能体同一套 Agent-managed 状态约定：首次接单应离开 `todo`（或从 backlog 提升后的状态），进入 `in_progress`。分发成员不等于完成——小队干活期间父 issue 保持 `in_progress`。 +4. **队长发一条"派活"评论。** 评论里用 roster 里给好的 mention markdown `@` 选中的成员——这个 `@` 会触发被派的成员入队新 `task`。 +5. **队长记录 evaluation：** `multica squad activity action --reason "..."`。这一行会写进 issue 的 activity 时间线，方便人类回溯队长确实评估过这一次触发。 +6. **队长停下。** 派完活，队长**不动手干活**。当被派的成员有回复——或子 issue / stage 屏障关闭——时，队长会被自动唤醒，决定下一步：继续派活、上抛给人类、在整体目标达成后把父 issue 推到 `in_review`、还是保持沉默。`done` 留给人工确认或既有集成（例如带 close intent 的 PR merge）。 如果 issue 是 **Backlog** 状态，队长不会被触发——Backlog 是停泊场，规则和直接分配给智能体一样。 @@ -72,7 +73,9 @@ multica squad member add --member-id --type agen 每次队长被触发，三段内容会被附加到它的 instructions 上： -- **Squad Operating Protocol（小队工作规范）** —— 一段硬编码的规则集：读 issue → 用 `@` 派活 → 简洁（**不要**复述 issue 内容，被派的成员自己能读）→ 每次都记 evaluation → **派完就停**。这段是系统管理的，不可编辑。 +- **Squad Operating Protocol（小队工作规范）** —— 一段硬编码的规则集：读 issue → 首次接单把父 issue 推到 `in_progress` → 用 `@` 派活 → 简洁（**不要**复述 issue 内容，被派的成员自己能读）→ 每次都记 evaluation → **派完就停** → 只有整体目标达成后才把父 issue 推到 `in_review`。这段是系统管理的，不可编辑。 + + 其中**状态那一半只对「确实分配给本小队」的 issue 生效**。如果队长是被别人 issue 里的 `@squad` 喊醒的，他照样拿到花名册和派活规则，但会被明确告知**不要碰那个 issue 的状态**——状态仍归该 issue 自己的 assignee。 - **Squad Roster（小队花名册）** —— 队长自己一行 + 每个未归档成员一行。每一行带上**确切可用**的 mention markdown（`[@Name](mention://agent/)` 或 `[@Name](mention://member/)`）让队长直接复制——纯文本 `@name` 是**不会**触发任何人的。 - **Squad Instructions（小队自定义指令）** —— 你为这个小队写的私货（在详情页里编辑，或用 `multica squad update --instructions`）。用来写路由规则（"DB 相关派给 Alice，前端派给 Bob"）、上报策略，或者任何 issue 本身不会有的背景。 diff --git a/server/internal/daemon/execenv/runtime_config_sections.go b/server/internal/daemon/execenv/runtime_config_sections.go index 68a131de8..096e86b6c 100644 --- a/server/internal/daemon/execenv/runtime_config_sections.go +++ b/server/internal/daemon/execenv/runtime_config_sections.go @@ -403,10 +403,35 @@ func writeWorkflowComment(b *strings.Builder, provider string, ctx TaskContextFo b.WriteString(buildCommentReplyInstructionsSlim(provider, ctx.IssueID, ctx.TriggerCommentID)) } b.WriteString("8. Before exiting: only if this run produced a fact that clears the high bar (important AND likely to be re-read by future runs on this same issue, e.g. a new PR URL or deploy URL), or you noticed a metadata key from entry that is now stale, pin or clear it via `multica issue metadata set`/`delete`. Most runs write nothing here — that is the expected outcome, not a gap. When in doubt, do not write. See the `## Issue Metadata` section above for the full bar.\n") - b.WriteString("9. Do NOT change the issue status unless the comment explicitly asks for it\n\n") + if ctx.IsSquadLeader { + // The default rule below and the Squad Operating Protocol's + // "Own the parent issue status" responsibility would otherwise + // contradict each other on the squad's most common shape: + // @mention dispatch with no child issues, where the member's + // delivery comment never "explicitly asks" for a status change and + // no child-done system comment exists to carry that ask. Naming the + // protocol section as the exception resolves it in one direction. + // + // The exception is safe to state unconditionally here because the + // grant only exists in the instructions when the server determined + // this issue is assigned to this squad (see buildSquadLeaderBriefing); + // a guest leader gets the opposite text and this stays a no-op. + b.WriteString("9. Do NOT change the issue status unless the comment explicitly asks for it — **or** a section in your instructions explicitly grants you ownership of this issue's status (the Squad Operating Protocol's \"Own the parent issue status\" responsibility). That section only appears when this issue is assigned to your squad; when it is there, treat it as a standing instruction and move the parent to `in_review` on the turn you confirm the overall goal is met, without waiting to be asked. When it is absent, the rule above is absolute.\n\n") + } else { + b.WriteString("9. Do NOT change the issue status unless the comment explicitly asks for it\n\n") + } } // writeWorkflowAssignment emits the assignment-triggered workflow. +// +// Ordinary agents own the full status arc for their issue: open with +// in_progress, deliver with in_review. Squad leaders share the opening +// in_progress step so the parent leaves todo as soon as coordination +// starts, but their first assignment turn is only a dispatch — flipping +// the parent to in_review there would mark unfinished multi-stage work +// as ready for review. Leaders move the parent to in_review later, when +// a re-trigger (member update / stage barrier) confirms the overall goal +// is met; see the Squad Operating Protocol and child-done system comments. func writeWorkflowAssignment(b *strings.Builder, ctx TaskContextForEnv) { b.WriteString("You are responsible for managing the issue status throughout your work, unless your Agent Identity forbids issue status changes.\n\n") fmt.Fprintf(b, "1. Run `multica issue get %s --output json` to understand your task\n", ctx.IssueID) @@ -420,7 +445,11 @@ func writeWorkflowAssignment(b *strings.Builder, ctx TaskContextForEnv) { fmt.Fprintf(b, "6. **Post your final results as a comment — this step is mandatory**: post it with `multica issue comment add %s` using the platform-correct non-inline mode from ## Comment Formatting (never inline `--content`). Your results are only visible to the user if posted via this CLI call; text in your terminal or run logs is NOT delivered.\n", ctx.IssueID) } b.WriteString("7. Before exiting: only if this run produced a fact that clears the high bar (important AND likely to be re-read by future runs on this same issue, e.g. a new PR URL or deploy URL), or you noticed a metadata key from entry that is now stale, pin or clear it via `multica issue metadata set`/`delete`. Most runs write nothing here — that is the expected outcome, not a gap. When in doubt, do not write. See the `## Issue Metadata` section above for the full bar.\n") - fmt.Fprintf(b, "8. When done, run `multica issue status %s in_review` unless your Agent Identity forbids issue status changes; if it does, skip this step.\n", ctx.IssueID) + if ctx.IsSquadLeader { + fmt.Fprintf(b, "8. After this initial dispatch, leave the parent issue `in_progress` — do NOT run `multica issue status %s in_review` or `done` on this turn. Dispatching members is not completion. You will be re-triggered when members post updates or a stage closes; only then, if the overall goal is met, move the parent to `in_review`.\n", ctx.IssueID) + } else { + fmt.Fprintf(b, "8. When done, run `multica issue status %s in_review` unless your Agent Identity forbids issue status changes; if it does, skip this step.\n", ctx.IssueID) + } fmt.Fprintf(b, "9. If blocked, run `multica issue status %s blocked` unless your Agent Identity forbids issue status changes. Post a comment explaining the blocker unless your Agent Identity forbids issue comments.\n\n", ctx.IssueID) } diff --git a/server/internal/daemon/execenv/runtime_config_test.go b/server/internal/daemon/execenv/runtime_config_test.go index 0e3a9e2dc..6307dcf4a 100644 --- a/server/internal/daemon/execenv/runtime_config_test.go +++ b/server/internal/daemon/execenv/runtime_config_test.go @@ -162,6 +162,45 @@ func TestCommentTriggeredProtocolDoesNotForceInReview(t *testing.T) { if !strings.Contains(out, guardrail) { t.Errorf("expected the comment-triggered workflow guardrail %q to be present", guardrail) } + + // For an ordinary agent the guardrail is absolute — the squad-leader + // carve-out below must not leak into this path. + if strings.Contains(out, "Own the parent issue status") { + t.Errorf("ordinary-agent comment brief must not reference the squad status grant:\n%s", out) + } +} + +// A squad leader on a comment-triggered turn gets the same guardrail plus a +// named exception. Without it the guardrail and the Squad Operating Protocol's +// "Own the parent issue status" responsibility contradict each other on the +// @mention-dispatch shape, where the member's delivery comment never asks for +// a status change and no child-done system comment exists to ask on its +// behalf — so the parent would sit in in_progress forever. +func TestCommentTriggeredSquadLeaderDefersToStatusOwnershipGrant(t *testing.T) { + t.Parallel() + out := buildMetaSkillContent("claude", TaskContextForEnv{ + IssueID: "55555555-6666-7777-8888-999999999999", + TriggerCommentID: "66666666-7777-8888-9999-aaaaaaaaaaaa", + IsSquadLeader: true, + }) + + for _, want := range []string{ + "Do NOT change the issue status unless the comment explicitly asks for it", + `Squad Operating Protocol's "Own the parent issue status"`, + "only appears when this issue is assigned to your squad", + "without waiting to be asked", + "When it is absent, the rule above is absolute.", + } { + if !strings.Contains(out, want) { + t.Errorf("squad-leader comment brief missing %q\n---\n%s", want, out) + } + } + + // The unqualified sentence must be gone: its presence alongside the grant + // is the contradiction this branch exists to remove. + if strings.Contains(out, "explicitly asks for it\n") { + t.Errorf("squad-leader comment brief still ends the guardrail unqualified\n---\n%s", out) + } } // The CLAUDE.md workflow surface must carry the same issue-wide since-delta @@ -327,6 +366,39 @@ func TestAssignmentTriggeredProtocolHonorsAgentIdentity(t *testing.T) { } } +// Squad-leader assignment briefs must open the parent with in_progress, but +// must not treat the first dispatch turn as completion (no unconditional +// in_review). Leaders move the parent to in_review only on a later re-trigger +// once the overall goal is met. +func TestSquadLeaderAssignmentProtocolKeepsParentInProgress(t *testing.T) { + t.Parallel() + const issueID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + out := buildMetaSkillContent("claude", TaskContextForEnv{ + IssueID: issueID, + IsSquadLeader: true, + }) + + for _, want := range []string{ + "Run `multica issue status " + issueID + " in_progress` unless your Agent Identity forbids issue status changes; if it does, skip this step.", + "After this initial dispatch, leave the parent issue `in_progress`", + "do NOT run `multica issue status " + issueID + " in_review` or `done` on this turn", + "only then, if the overall goal is met, move the parent to `in_review`", + } { + if !strings.Contains(out, want) { + t.Errorf("squad-leader assignment brief missing %q\n---\n%s", want, out) + } + } + + for _, banned := range []string{ + "When done, run `multica issue status " + issueID + " in_review`", + "8. When done, run `multica issue status " + issueID + " in_review`", + } { + if strings.Contains(out, banned) { + t.Errorf("squad-leader assignment brief must not contain ordinary-agent completion step %q\n---\n%s", banned, out) + } + } +} + func TestInstructionPrecedenceOnlyAppliesToAssignmentWorkflow(t *testing.T) { t.Parallel() cases := []struct { diff --git a/server/internal/handler/daemon.go b/server/internal/handler/daemon.go index 6b1b6985a..41fae7aeb 100644 --- a/server/internal/handler/daemon.go +++ b/server/internal/handler/daemon.go @@ -1752,7 +1752,18 @@ func (h *Handler) buildClaimedTaskResponse(r *http.Request, task *db.AgentTaskQu ID: task.SquadID, WorkspaceID: issue.WorkspaceID, }); err == nil && uuidToString(squad.LeaderID) == resp.Agent.ID { - briefing := buildSquadLeaderBriefing(r.Context(), h.Queries, squad) + // Parent-status authority is deliberately NARROWER than + // briefing injection. Injection is keyed off is_leader_task + // (see above) and therefore also fires on the MUL-3724 path, + // where the issue belongs to a plain agent and this squad was + // only @mentioned for help. Granting status ownership there + // would let a guest squad push someone else's in-flight issue + // to in_review, so we gate it on the issue actually being + // assigned to this squad. + ownsIssueStatus := issue.AssigneeType.Valid && + issue.AssigneeType.String == "squad" && + uuidToString(issue.AssigneeID) == uuidToString(squad.ID) + briefing := buildSquadLeaderBriefing(r.Context(), h.Queries, squad, ownsIssueStatus) if strings.TrimSpace(resp.Agent.Instructions) == "" { resp.Agent.Instructions = briefing } else { @@ -1762,6 +1773,7 @@ func (h *Handler) buildClaimedTaskResponse(r *http.Request, task *db.AgentTaskQu "squad_id", uuidToString(squad.ID), "squad_name", squad.Name, "leader_agent_id", resp.Agent.ID, + "owns_issue_status", ownsIssueStatus, ) } } @@ -2336,7 +2348,11 @@ func (h *Handler) buildClaimedTaskResponse(r *http.Request, task *db.AgentTaskQu ID: squadUUID, WorkspaceID: wsUUID, }); err == nil && uuidToString(squad.LeaderID) == resp.Agent.ID { - briefing := buildSquadLeaderBriefing(r.Context(), h.Queries, squad) + // Quick-create has no issue yet — there is no parent + // status to own on this turn. Once the leader opens the + // issue with the squad as assignee, the issue-bound + // claim path above grants ownership. + briefing := buildSquadLeaderBriefing(r.Context(), h.Queries, squad, false) if strings.TrimSpace(resp.Agent.Instructions) == "" { resp.Agent.Instructions = briefing } else { diff --git a/server/internal/handler/issue_child_done.go b/server/internal/handler/issue_child_done.go index b9f0be80c..2df0736e8 100644 --- a/server/internal/handler/issue_child_done.go +++ b/server/internal/handler/issue_child_done.go @@ -286,13 +286,13 @@ func (h *Handler) postChildDoneComment(ctx context.Context, parent, completed db } else { if batch { content = fmt.Sprintf( - "%sAll sub-issues are complete — they just finished together in a batch update, most recently [%s](mention://issue/%s) — \"%s\". Continue the parent: synthesize the children's results and move it forward, or close it out if nothing remains.", - mentionPrefix, identifier, childID, title, + "%sAll sub-issues are complete — they just finished together in a batch update, most recently [%s](mention://issue/%s) — \"%s\". Continue the parent: synthesize the children's results and move it forward, or — if nothing remains — run `multica issue status %s in_review` to mark the parent ready for review.", + mentionPrefix, identifier, childID, title, parentID, ) } else { content = fmt.Sprintf( - "%sAll sub-issues are complete — the last one, [%s](mention://issue/%s) — \"%s\", just finished. Continue the parent: synthesize the children's results and move it forward, or close it out if nothing remains.", - mentionPrefix, identifier, childID, title, + "%sAll sub-issues are complete — the last one, [%s](mention://issue/%s) — \"%s\", just finished. Continue the parent: synthesize the children's results and move it forward, or — if nothing remains — run `multica issue status %s in_review` to mark the parent ready for review.", + mentionPrefix, identifier, childID, title, parentID, ) } } @@ -456,7 +456,7 @@ func stageAdvanceInstruction(nextStage int32, parentID string) string { nextStage, parentID, nextStage, ) } - return " Completing this stage does not mean the whole issue is done. Decide whether the issue is actually complete — if so, wrap up the parent (synthesize the results and move it forward, or close it out) — or whether the next stage still needs to be created, in which case create that stage and its sub-issues now." + return fmt.Sprintf(" Completing this stage does not mean the whole issue is done. Decide whether the issue is actually complete — if so, synthesize the results and run `multica issue status %s in_review` to mark the parent ready for review — or whether the next stage still needs to be created, in which case create that stage and its sub-issues now.", parentID) } // sanitizeChildTitleForSystemComment removes mention-style markdown from a diff --git a/server/internal/handler/issue_child_done_stage_test.go b/server/internal/handler/issue_child_done_stage_test.go index d32c09954..130433e2d 100644 --- a/server/internal/handler/issue_child_done_stage_test.go +++ b/server/internal/handler/issue_child_done_stage_test.go @@ -171,13 +171,17 @@ func TestStageAdvanceInstruction(t *testing.T) { } // It must make clear that finishing the stage != the whole issue is // done, and hand both paths (wrap up / create the next stage) to the - // leader. + // leader. When wrapping up, the explicit ask is in_review so the + // comment-triggered "only change status when asked" rule permits it. if !strings.Contains(got, "does not mean the whole issue is done") { t.Fatalf("expected stage-done != issue-done framing, got %q", got) } if !strings.Contains(got, "next stage") { t.Fatalf("expected create-next-stage guidance, got %q", got) } + if !strings.Contains(got, "multica issue status "+parentID+" in_review") { + t.Fatalf("expected explicit in_review instruction for confirmed completion, got %q", got) + } }) } diff --git a/server/internal/handler/squad_briefing.go b/server/internal/handler/squad_briefing.go index e40693e19..031b35b3c 100644 --- a/server/internal/handler/squad_briefing.go +++ b/server/internal/handler/squad_briefing.go @@ -9,15 +9,18 @@ import ( db "github.com/multica-ai/multica/server/pkg/db/generated" ) -// squadOperatingProtocol is the hard-coded system-level briefing prepended to -// every squad-leader claim. It explains the leader's coordinator role, the -// @mention dispatch mechanism, and the stop-after-dispatch contract. +// squadOperatingProtocolHeader is the hard-coded system-level briefing +// prepended to every squad-leader claim. It explains the leader's coordinator +// role, the @mention dispatch mechanism, and the stop-after-dispatch contract. +// Responsibility 6 (parent issue status) is appended separately by +// squadOperatingProtocolFor — it is the only part that varies by whether this +// squad actually owns the issue. // // Keep this text English-only (matches existing agent-harness conventions) // and keep the mention syntax exactly aligned with util.MentionRe — the // "Squad Roster" block below renders concrete examples that round-trip // through util.ParseMentions, and the protocol text refers to that format. -const squadOperatingProtocol = `## Squad Operating Protocol +const squadOperatingProtocolHeader = `## Squad Operating Protocol **If you are reading this section, you have been activated as a squad LEADER for this task — regardless of how the work reached you (direct assignment, @@ -65,9 +68,43 @@ Your responsibilities, in order: activity and decide whether to delegate the next step, escalate to the human reporter, or close the loop. If no action is needed (e.g. a member posted a progress update that requires no response), - record ` + "`" + `no_action` + "`" + ` and exit silently. + record ` + "`" + `no_action` + "`" + ` and exit silently.` -Hard rules: +// squadParentStatusOwned is responsibility 6 for the case where the issue this +// leader was woken on is assigned to THIS squad. Only then does the leader own +// the parent's status arc. +// +// The "even when no comment asked you to" clause is load-bearing: the comment +// workflow's default rule is "do not change status unless the comment asks", +// and a member's delivery comment never asks. Without an explicit standing +// grant here, the @mention-dispatch squad shape (no child issues, so no +// child-done system comment carrying an explicit ask) would leave the parent +// stuck in in_progress forever. The comment workflow defers to this section by +// name — keep the heading text in sync with writeWorkflowComment. +const squadParentStatusOwned = `6. **Own the parent issue status.** This issue is assigned to your squad, + so its status is yours to manage (unless Agent Identity forbids status + changes). On the first assignment turn, move the parent to + ` + "`" + `in_progress` + "`" + ` and keep it there while members work — a successful + dispatch is not completion. On later turns, do not flip status for + routine progress updates. When you confirm the overall goal is met, run + ` + "`" + `multica issue status in_review` + "`" + ` — this responsibility is + itself the standing instruction that authorizes that change, so do it even + when no comment asked you to. Leave ` + "`" + `done` + "`" + ` to a human reviewer or + existing integrations (for example a PR with close intent that merges).` + +// squadParentStatusNotOwned is responsibility 6 for every other leader path: +// an @squad mention on an issue owned by someone else (MUL-3724), and +// quick-create, where no issue exists yet on this turn. Granting status +// ownership there would let a squad that was merely pulled in to answer a +// question push another assignee's in-flight issue to in_review. +const squadParentStatusNotOwned = `6. **Do NOT change this issue's status.** This issue is not assigned to your + squad — you were pulled in by an @mention (or this is a quick-create turn, + where the issue does not exist yet). Its status belongs to its own + assignee. Answer, delegate, or escalate as usual, but never run + ` + "`" + `multica issue status` + "`" + ` on it, no matter how complete the work looks + to you.` + +const squadOperatingProtocolHardRules = `Hard rules: - EVERY delegation MUST use the full mention markdown syntax ` + "`" + `[@Name](mention:///)` + "`" + ` exactly as shown in the Squad Roster. A plain "@name" or bare name does NOT trigger the agent — @@ -96,6 +133,16 @@ Hard rules: @mention on this issue, or create a ` + "`" + `todo` + "`" + ` child issue assigned to them. Never both for the same work.` +// squadOperatingProtocolFor assembles the protocol, selecting the parent-status +// responsibility that matches this leader's actual authority over the issue. +func squadOperatingProtocolFor(ownsIssueStatus bool) string { + status := squadParentStatusNotOwned + if ownsIssueStatus { + status = squadParentStatusOwned + } + return squadOperatingProtocolHeader + "\n" + status + "\n\n" + squadOperatingProtocolHardRules +} + // buildSquadLeaderBriefing composes the full system briefing appended to a // squad leader's Instructions when it claims a task on a squad-assigned // issue. The returned string contains three sections: @@ -106,12 +153,17 @@ Hard rules: // 3. Squad Instructions (user-defined `squad.instructions`, omitted when // empty so we don't leave a dangling heading). // +// ownsIssueStatus must be true only when the issue this task is bound to is +// assigned to this very squad. The briefing is injected on every leader path, +// including ones where the squad is a guest on someone else's issue, so this +// flag is what keeps status authority from leaking along with the roster. +// // Archived agent members are skipped — there's no point asking the leader // to delegate to a retired agent. Members whose underlying record can't be // loaded (deleted user/agent races, FK weirdness) are also skipped silently. -func buildSquadLeaderBriefing(ctx context.Context, q *db.Queries, squad db.Squad) string { +func buildSquadLeaderBriefing(ctx context.Context, q *db.Queries, squad db.Squad, ownsIssueStatus bool) string { var sb strings.Builder - sb.WriteString(squadOperatingProtocol) + sb.WriteString(squadOperatingProtocolFor(ownsIssueStatus)) sb.WriteString("\n\n") sb.WriteString(buildSquadRoster(ctx, q, squad)) diff --git a/server/internal/handler/squad_briefing_test.go b/server/internal/handler/squad_briefing_test.go index 903d11fbf..a755cab52 100644 --- a/server/internal/handler/squad_briefing_test.go +++ b/server/internal/handler/squad_briefing_test.go @@ -13,6 +13,76 @@ import ( db "github.com/multica-ai/multica/server/pkg/db/generated" ) +// TestSquadOperatingProtocolOwnsParentStatus locks the parent-issue status +// contract: first dispatch moves todo→in_progress and stays there; only a +// later confirmation of overall completion may advance to in_review; done is +// left to humans / integrations. +func TestSquadOperatingProtocolOwnsParentStatus(t *testing.T) { + protocol := squadOperatingProtocolFor(true) + compact := strings.Join(strings.Fields(protocol), " ") + for _, want := range []string{ + "Own the parent issue status", + "move the parent to `in_progress`", + "successful dispatch is not completion", + "multica issue status in_review", + "Leave `done` to a human reviewer", + } { + if !strings.Contains(compact, want) { + t.Errorf("expected squad operating protocol to contain %q\n--- protocol ---\n%s", want, protocol) + } + } +} + +// TestSquadOperatingProtocolScopesParentStatusOwnership is the guard for the +// MUL-5156 review finding: the briefing is injected on every leader path, +// including an @squad mention on an issue assigned to someone else. Status +// ownership must not ride along — a guest leader gets an explicit prohibition +// instead of the grant, so the model never has to infer the boundary. +func TestSquadOperatingProtocolScopesParentStatusOwnership(t *testing.T) { + guest := squadOperatingProtocolFor(false) + compactGuest := strings.Join(strings.Fields(guest), " ") + + for _, want := range []string{ + "Do NOT change this issue's status", + "not assigned to your squad", + "never run `multica issue status` on it", + } { + if !strings.Contains(compactGuest, want) { + t.Errorf("expected guest-leader protocol to contain %q\n--- protocol ---\n%s", want, guest) + } + } + // The grant must be entirely absent — not merely qualified. + for _, forbidden := range []string{ + "Own the parent issue status", + "multica issue status in_review", + } { + if strings.Contains(compactGuest, forbidden) { + t.Errorf("guest-leader protocol must not contain status grant %q\n--- protocol ---\n%s", forbidden, guest) + } + } + + // Everything that is not the status responsibility is identical, so a + // guest leader still coordinates, delegates, and records activity. + owner := squadOperatingProtocolFor(true) + for _, shared := range []string{ + "## Squad Operating Protocol", + "Delegate by @mention", + "Record your evaluation", + "Stop after dispatching", + "Never both for the same work.", + } { + if !strings.Contains(owner, shared) || !strings.Contains(guest, shared) { + t.Errorf("expected %q in both protocol variants", shared) + } + } + + // The IsSquadLeader marker the daemon greps for must survive both ways, + // or a guest leader silently loses its no_action / silent-exit behavior. + if !strings.Contains(guest, "## Squad Operating Protocol") { + t.Error("guest-leader protocol lost the header the daemon keys IsSquadLeader off") + } +} + // TestSquadOperatingProtocolWarnsAgainstDualTrigger locks in the rule // added for #3033: the protocol must tell the squad leader that a `todo` // child issue with an agent assignee already fires that agent, so they @@ -20,13 +90,14 @@ import ( // same work. Asserts behavior, not exact wording — keep the substrings // narrow so harmless rewording doesn't break the test. func TestSquadOperatingProtocolWarnsAgainstDualTrigger(t *testing.T) { - compact := strings.Join(strings.Fields(squadOperatingProtocol), " ") + protocol := squadOperatingProtocolFor(true) + compact := strings.Join(strings.Fields(protocol), " ") for _, want := range []string{ "--status todo` and an agent assignee already fires that agent automatically", "Never both for the same work.", } { if !strings.Contains(compact, want) { - t.Errorf("expected squad operating protocol to contain %q\n--- protocol ---\n%s", want, squadOperatingProtocol) + t.Errorf("expected squad operating protocol to contain %q\n--- protocol ---\n%s", want, protocol) } } } @@ -124,7 +195,7 @@ func TestBuildSquadLeaderBriefing_FullSquad(t *testing.T) { _ = memberRowID addHumanMember(t, squad.ID, userID, "reviewer") - out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad) + out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad, true) for _, want := range []string{ "## Squad Operating Protocol", @@ -200,7 +271,7 @@ func TestBuildSquadLeaderBriefing_MemberSkillsInRoster(t *testing.T) { _ = memberRowID addHumanMember(t, squad.ID, userID, "reviewer") - out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad) + out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad, true) if !strings.Contains(out, "skills: polars, statistical-analysis") { t.Errorf("expected skilled member skills in roster, got:\n%s", out) @@ -219,7 +290,7 @@ func TestBuildSquadLeaderBriefing_OnlyLeader(t *testing.T) { leaderID, _ := seededLeaderAgent(t) squad := seedSquadForBriefing(t, leaderID, "Solo Squad", "") - out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad) + out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad, true) if !strings.Contains(out, "Members: (none — you are the only member of this squad)") { t.Errorf("expected lone-leader fallback line, got:\n%s", out) } @@ -243,7 +314,7 @@ func TestBuildSquadLeaderBriefing_SkipsArchivedAgent(t *testing.T) { t.Fatalf("archive agent: %v", err) } - out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad) + out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad, true) if strings.Contains(out, "Retired Bot") { t.Errorf("archived agent should not appear in roster:\n%s", out) } @@ -268,7 +339,7 @@ func TestBuildSquadLeaderBriefing_MentionsRoundTrip(t *testing.T) { _ = memberRowID addHumanMember(t, squad.ID, userID, "") - out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad) + out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad, true) mentions := util.ParseMentions(out) wantIDs := map[string]string{ diff --git a/server/internal/handler/squad_parent_status_contract_test.go b/server/internal/handler/squad_parent_status_contract_test.go new file mode 100644 index 000000000..d2ceede87 --- /dev/null +++ b/server/internal/handler/squad_parent_status_contract_test.go @@ -0,0 +1,135 @@ +package handler + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/multica-ai/multica/server/internal/daemon/execenv" +) + +// The two tests below are composition tests, not text-presence tests. The +// parent-status contract is split across two systems that never see each +// other: the squad briefing (server-side, appended to the leader's +// Instructions) and the runtime brief (daemon-side CLAUDE.md). Asserting each +// half in isolation is exactly how the original contradiction shipped — the +// briefing said "move it to in_review when the goal is met" while the runtime +// brief said "do not change status unless the comment asks", and a member's +// delivery comment never asks. +// +// So each test assembles both halves for one real scenario and asserts the +// combined instruction set points one way. + +// leaderCommentRuntimeBrief renders the CLAUDE.md a squad leader receives on a +// comment-triggered turn. +func leaderCommentRuntimeBrief(t *testing.T, instructions string) string { + t.Helper() + dir := t.TempDir() + if _, err := execenv.InjectRuntimeConfig(dir, "claude", execenv.TaskContextForEnv{ + IssueID: "issue-1", + TriggerCommentID: "comment-1", + AgentInstructions: instructions, + IsSquadLeader: true, + }); err != nil { + t.Fatalf("InjectRuntimeConfig: %v", err) + } + data, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md")) + if err != nil { + t.Fatalf("read CLAUDE.md: %v", err) + } + return string(data) +} + +// TestSquadAssignedLeaderCanWrapUpOnCommentTurn covers the squad's most common +// shape: work dispatched by @mention with no child issues, so no child-done +// system comment ever arrives to carry an explicit status ask. The member +// simply posts "done". The leader must still be able to close the parent out. +func TestSquadAssignedLeaderCanWrapUpOnCommentTurn(t *testing.T) { + ctx := context.Background() + leaderID, _ := seededLeaderAgent(t) + squad := seedSquadForBriefing(t, leaderID, "Owning Squad", "") + + // The issue is assigned to this squad → the server grants status ownership. + briefing := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad, true) + brief := leaderCommentRuntimeBrief(t, briefing) + + if !strings.Contains(briefing, "Own the parent issue status") { + t.Fatalf("squad-assigned briefing must grant status ownership:\n%s", briefing) + } + + // The runtime brief must not restate the prohibition in its absolute form, + // which is what contradicted the grant. The absolute sentence ends right + // after "explicitly asks for it"; the leader variant continues past it. + if strings.Contains(brief, "explicitly asks for it\n") { + t.Error("leader runtime brief still carries the unqualified no-status-change rule, " + + "which contradicts the Own-the-parent-issue-status grant") + } + for _, want := range []string{ + // The carve-out must name the granting section, not gesture at it — + // the leader has to be able to tell whether it applies to this turn. + `Squad Operating Protocol's "Own the parent issue status"`, + "only appears when this issue is assigned to your squad", + "without waiting to be asked", + } { + if !strings.Contains(brief, want) { + t.Errorf("leader runtime brief missing %q\n--- brief ---\n%s", want, brief) + } + } + + // End to end: both halves must agree that in_review is reachable here. + combined := briefing + "\n" + brief + if !strings.Contains(combined, "multica issue status in_review") { + t.Error("combined instructions never tell the owning leader how to wrap up") + } +} + +// TestGuestLeaderCannotChangeStatusOnCommentTurn is the other half of the +// scope fix (MUL-3724 path): the issue belongs to a plain agent and this squad +// was only @mentioned for help. The briefing still gets injected — the leader +// needs its roster — but no combination of the two halves may authorize a +// status change on someone else's issue. +func TestGuestLeaderCannotChangeStatusOnCommentTurn(t *testing.T) { + ctx := context.Background() + leaderID, _ := seededLeaderAgent(t) + squad := seedSquadForBriefing(t, leaderID, "Guest Squad", "") + + // The issue is assigned to someone else → no status ownership. + briefing := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad, false) + brief := leaderCommentRuntimeBrief(t, briefing) + + // The leader still gets the coordination context it was pulled in for — + // withholding status authority must not withhold the roster too. + for _, want := range []string{ + "## Squad Roster", + "Leader (you):", + "Delegate by @mention", + "Record your evaluation", + } { + if !strings.Contains(briefing, want) { + t.Fatalf("guest leader lost coordination context %q:\n%s", want, briefing) + } + } + + // But the grant is absent, so the runtime brief's carve-out has nothing to + // activate and the default prohibition governs. + if strings.Contains(briefing, "Own the parent issue status") { + t.Errorf("guest leader must not receive the status-ownership grant:\n%s", briefing) + } + combined := briefing + "\n" + brief + if strings.Contains(combined, "multica issue status in_review") { + t.Error("combined instructions hand a guest leader an in_review command for " + + "an issue assigned to someone else") + } + // The prohibition wraps across source lines, so match on compacted text. + compact := strings.Join(strings.Fields(briefing), " ") + for _, want := range []string{ + "Do NOT change this issue's status", + "never run `multica issue status` on it", + } { + if !strings.Contains(compact, want) { + t.Errorf("guest-leader briefing missing %q\n--- briefing ---\n%s", want, briefing) + } + } +} diff --git a/server/internal/service/builtin_skills/multica-squads/SKILL.md b/server/internal/service/builtin_skills/multica-squads/SKILL.md index e5ad28ab7..4fa9b2ac1 100644 --- a/server/internal/service/builtin_skills/multica-squads/SKILL.md +++ b/server/internal/service/builtin_skills/multica-squads/SKILL.md @@ -161,7 +161,18 @@ Current behavior: - assignment while status is `backlog` does not immediately start work; - moving a squad-assigned issue out of `backlog` can trigger the leader; - changing assignee cancels existing tasks for the issue before enqueueing the - new assignee path. + new assignee path; +- parent issue status is agent-managed (same model as direct agent assignment): + the leader's first assignment turn should move the parent to `in_progress` + and keep it there while members work; the leader moves the parent to + `in_review` only when a later re-trigger confirms the overall goal is met. + Completing a leader `task` (including the first dispatch) does not itself + change issue status; +- that status authority is granted only when the issue's `assignee_type` / + `assignee_id` point at THIS squad. The leader briefing is injected on every + leader path, including an `@squad` mention on an issue owned by a plain agent + — on those paths the protocol instead carries an explicit "do not change this + issue's status". Assignment validation rejects a missing type/id pair, non-existent squad, archived squad, archived leader, and private leader when the actor cannot access @@ -245,6 +256,13 @@ authorizes them. - `description` is not proven runtime prompt content. - `role` is roster context, not automatic scheduling. - Backlog assignment does not immediately start work. +- First leader dispatch is not parent completion — parent stays `in_progress` + until the leader later confirms the overall goal and moves it to `in_review`. +- The server does not auto-flip parent status when child issues finish; it only + wakes the leader with an explicit ask (including `in_review` when wrapping up). +- Getting the leader briefing does NOT imply status authority. A squad + `@`-mentioned into an issue assigned to someone else is a guest: roster and + delegation rules yes, `multica issue status` no. ## References diff --git a/server/internal/service/builtin_skills/multica-squads/references/squad-source-map.md b/server/internal/service/builtin_skills/multica-squads/references/squad-source-map.md index 7b48f98bb..bd2352b50 100644 --- a/server/internal/service/builtin_skills/multica-squads/references/squad-source-map.md +++ b/server/internal/service/builtin_skills/multica-squads/references/squad-source-map.md @@ -91,6 +91,14 @@ Contracts: (daemon.go:1187, 1530); - briefing includes operating protocol, roster, and optional instructions (squad_briefing.go:104-117); +- `buildSquadLeaderBriefing` takes an `ownsIssueStatus` argument selecting + responsibility 6 via `squadOperatingProtocolFor`: the status grant + (`squadParentStatusOwned`) only when `issue.assignee_type == "squad"` and + `issue.assignee_id == squad.id`, otherwise an explicit prohibition + (`squadParentStatusNotOwned`). Quick-create passes `false` — no issue exists + yet. Injection is broader than authority on purpose: it is keyed off + `is_leader_task`, which also fires for `@squad` mentions on issues owned by + someone else (MUL-3724); - `instructions` section appears only when non-empty (squad_briefing.go:110-112); - archived agent members are skipped from roster (squad_briefing.go:178-179); - agent member roster rows list assigned workspace skills via @@ -119,7 +127,16 @@ Contracts: - private leader access is checked at assign-time (issue.go:2629-2632) and at enqueue-time via `canEnqueueSquadLeader` (squad.go:1037); - archived squad / archived leader rejected at assign-time (issue.go:2622-2627); -- pending task dedup is applied (squad.go:1042-1048). +- pending task dedup is applied (squad.go:1042-1048); +- parent status is agent-managed: assignment brief (`writeWorkflowAssignment` with + `IsSquadLeader`) requires `in_progress` on the first turn and forbids + unconditional `in_review` on that dispatch turn; Squad Operating Protocol + (`squad_briefing.go`) owns the ongoing `in_progress` → later `in_review` + contract. `StartTask` / `CompleteTask` do not write issue status. On + comment-triggered leader turns `writeWorkflowComment` names that protocol + responsibility as the one exception to "do not change status unless the + comment asks" — without it the @mention-dispatch shape (no child issues, so + no child-done ask) would strand the parent in `in_progress`. ## Comment / Mention @@ -198,6 +215,11 @@ Contracts: process-squad pipeline after stage 1 while direct-to-leader-agent parents advanced fine (MUL-4063 / GH #4928). Agent and squad child-done now share one ungated path; any future invocation gate must be added to BOTH together. +- parent status is not auto-advanced by the barrier: the system comment asks the + leader to continue or — when the overall goal is met — run + `multica issue status in_review`. That explicit ask is what lets a + comment-triggered leader turn change status (the comment workflow otherwise + forbids status flips unless asked). `done` remains human / integration owned. ## Private Leader Access diff --git a/server/internal/service/builtin_skills/multica-working-on-issues/SKILL.md b/server/internal/service/builtin_skills/multica-working-on-issues/SKILL.md index f7241e577..75db901ef 100644 --- a/server/internal/service/builtin_skills/multica-working-on-issues/SKILL.md +++ b/server/internal/service/builtin_skills/multica-working-on-issues/SKILL.md @@ -180,6 +180,13 @@ on it. These are the contracts, not advice: - **`backlog`** parks an agent-assigned issue: the assignee is set but no task fires. Moving `backlog → todo` (or any non-done/non-cancelled status) enqueues the assigned agent then. +- **`in_progress` / `in_review` on assignment runs** are agent-managed CLI + mutations, not `StartTask` / `CompleteTask` side effects. The assignment + runtime brief asks ordinary agents for `todo`/`backlog` → `in_progress` then + `in_review` when they have delivered. Squad leaders share the opening + `in_progress` step on the first assignment turn, keep the parent there while + members work, and only move to `in_review` when a later re-trigger confirms + the overall goal is met. - **`in_review`** is an accepted issue status. Some workflows use it while a PR is open and awaiting review; moving to it is an explicit mutation. - **`done`** on a child issue posts a system comment on its parent. If a PR @@ -189,6 +196,9 @@ on it. These are the contracts, not advice: `done` it enqueues no new agent work, but it does **not** stop tasks already in flight — a run in progress keeps going (MUL-4465). To stop a running task, cancel the task itself. +- **Failed issue-triggered tasks** may roll an issue from `in_progress` back to + `todo` when no active task / retry remains — that is the main server-owned + status write on the agent-run path. ## Sub-issues: `todo` starts work now, `backlog` parks it diff --git a/server/internal/service/builtin_skills/multica-working-on-issues/references/working-on-issues-source-map.md b/server/internal/service/builtin_skills/multica-working-on-issues/references/working-on-issues-source-map.md index 4b862f9ac..2484f8e0a 100644 --- a/server/internal/service/builtin_skills/multica-working-on-issues/references/working-on-issues-source-map.md +++ b/server/internal/service/builtin_skills/multica-working-on-issues/references/working-on-issues-source-map.md @@ -121,6 +121,9 @@ and is hidden from the PR list. | Same contract in batch update | `server/internal/handler/issue.go:3021-3024` | new citation | | Child → `done` notifies + wakes the parent, gated by the stage barrier | `server/internal/handler/issue_child_done.go:66` (`notifyParentOfChildDone`; doc comment at `:15`; barrier gate at `:115`) | func def `:51` | | Status change (incl. → `cancelled`) does NOT cancel in-flight tasks; only issue deletion does (MUL-4465) | no-cancel note in `server/internal/handler/issue.go:2652-2658` (`UpdateIssue`) and `:3170-3171` (`BatchUpdateIssues`); deletion still cancels at `:2863` (`DeleteIssue`) / `:3239` (`BatchDeleteIssues`) via `CancelTasksForIssue` (`server/internal/service/task.go:1229`) | new citation | +| `StartTask` / `CompleteTask` do not write issue status (agent CLI owns progress) | `server/internal/service/task.go` (`StartTask` / `CompleteTask` comments) | new citation | +| Assignment brief: ordinary agent `in_progress` then `in_review`; squad leader `in_progress` only on first dispatch | `server/internal/daemon/execenv/runtime_config_sections.go` (`writeWorkflowAssignment`) | new citation | +| Failed task may roll `in_progress` → `todo` when no active task remains | `server/internal/service/task.go` (`HandleFailedTasks`) | new citation | Creation with `--status todo` (or any non-backlog status) on an agent-assigned issue fires the agent immediately; `--status backlog` parks it with the assignee @@ -146,7 +149,10 @@ away, so no task is left orphaned. Advancement is agent-driven: the server only detects the closed barrier and wakes the parent assignee. Promoting the next stage's `backlog` sub-issues to -`todo` is the woken agent's decision, not a server side effect. +`todo` is the woken agent's decision, not a server side effect. When the woken +assignee (often a squad leader) decides the parent is complete, the system +comment explicitly asks for `multica issue status in_review` — +comment-triggered runs otherwise must not change status unless asked. ## Metadata CLI