From 35a19388a0c4ec032707dbffe1fb457185c14c48 Mon Sep 17 00:00:00 2001 From: leader Date: Tue, 10 Nov 2026 21:15:03 +0900 Subject: [PATCH] feat: offer /devup-retro at the end of the final completion report When work that used devup-mcp is complete, the agent ends its final report with one question in the user's language offering the retrospective. - contract identifier offer-devup-retro-in-final-report (v5), stated in delivery-quality, devfive-frontend, devup, devup-implementation-plan, the OpenCode command and the usage guide - nextSuggestedAction on results that mark completion: runtime-audit completion reportAllowed, screen-plan status reportAllowed, stack-diff zero drift on every layer; never on exports, validation or failures - invocation is /devup-retro when the skill is installed, otherwise the MCP prompt form Co-Authored-By: Claude Fable 5.2 --- .../changepack_log_Hk3vT9pQw5LnZ2xJc7RmB.json | 7 + README.md | 2 + .../src/server/commands/opencode-devup.md | 4 +- .../devup-mcp/src/server/completion_tests.rs | 3 + .../src/server/development_contract.rs | 12 +- .../server/handlers/devup_runtime_audit.rs | 9 +- .../src/server/handlers/devup_screen_plan.rs | 3 + .../src/server/handlers/devup_skills.rs | 13 ++ .../src/server/handlers/devup_stack_diff.rs | 4 +- .../server/skills/devfive-frontend/SKILL.md | 5 + .../references/delivery-quality.md | 21 +++ .../skills/devup-implementation-plan/SKILL.md | 6 + .../src/server/skills/devup/SKILL.md | 6 + .../devup-mcp/src/server/skills/manifest.json | 16 +-- crates/devup-mcp/src/server/skills/retro.rs | 64 +++++++++ .../src/server/skills/retro_tests.rs | 125 ++++++++++++++++++ .../server/stack_diff/contracts_and_layers.rs | 13 ++ .../devup-mcp/src/server/text/completion.md | 4 + .../src/server/text/guide-GUIDE-c682b0c5.md | 7 + .../devup-mcp/tests/export_project_theme.rs | 4 +- crates/devup-mcp/tests/retro_command.rs | 27 ++++ 21 files changed, 340 insertions(+), 15 deletions(-) create mode 100644 .changepacks/changepack_log_Hk3vT9pQw5LnZ2xJc7RmB.json diff --git a/.changepacks/changepack_log_Hk3vT9pQw5LnZ2xJc7RmB.json b/.changepacks/changepack_log_Hk3vT9pQw5LnZ2xJc7RmB.json new file mode 100644 index 00000000..44f66d6b --- /dev/null +++ b/.changepacks/changepack_log_Hk3vT9pQw5LnZ2xJc7RmB.json @@ -0,0 +1,7 @@ +{ + "changes": { + "Cargo.toml": "Minor" + }, + "note": "Offer /devup-retro when devup-mcp work is complete: the final completion report ends with one question in the user's language (\"작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?\"). The rule is contract identifier offer-devup-retro-in-final-report (developmentContract v5) and is stated in delivery-quality, the devfive-frontend, devup and devup-implementation-plan skills and the usage guide; results that mark completion (runtime-audit completion reportAllowed, screen-plan status reportAllowed, stack-diff zero drift on every layer) carry nextSuggestedAction with the ready message and the invocation that works in the workspace. Never on intermediate or failing results, never run unasked.", + "date": "2026-11-10T00:00:00.000000000Z" +} diff --git a/README.md b/README.md index 3d555226..aad393aa 100644 --- a/README.md +++ b/README.md @@ -1115,6 +1115,8 @@ DevupUI가 audit 속성을 기본으로 제거하며 `DEVUP_AUDIT=1`이면 유 `/devup`과 같은 경로로 배포됩니다: `devup_skills install`이 `devup-retro` 스킬을 쓰고(Claude Code에서는 `.claude/skills/devup-retro/SKILL.md`가 곧 `/devup-retro`, Codex 등은 `.agents/skills`에서 이름으로 로드), OpenCode에는 스킬 본문으로 만든 `.opencode/command/devup-retro.md`도 씁니다. 설치하지 않은 워크스페이스에서는 MCP 프롬프트 `devup-retro`(인자 `scope`, `language`, 둘 다 선택; Claude Code에서는 `/mcp__<서버 이름>__devup-retro`)로 같은 본문을 받습니다. `devup_skills`의 `retro` 액션은 읽기 전용으로 설치된 빌드(`server`), 이 서버 프로세스가 처리한 devup-mcp 호출 기록(`callLog`: 도구·action·outputs·시각·성공 여부, 최대 200개, 인자 내용과 결과는 보관하지 않으며 재연결하면 비워짐), surface 목록, `.feedback/` 상태(`nextNumber`, index, 최근 기록)를 돌려줍니다. 명령 이름은 `crates/devup-mcp/src/server/skills/commands.rs`의 `retro_name!` 한 곳에서 정합니다. +완료 시 회고 제안: devup-mcp를 쓴 작업이 끝나면 에이전트는 최종 완료 보고의 마지막에 사용자 언어로 한 번 묻습니다. 한국어는 “작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?”, 그 외에는 같은 뜻의 영어 문장입니다. 스킬이 설치돼 있으면 `/devup-retro`, 아니면 MCP 프롬프트 형태 `/mcp__<서버 이름>__devup-retro`를 씁니다. 이 규칙은 계약 식별자 `offer-devup-retro-in-final-report`(developmentContract v5)와 delivery-quality의 “Final report: offer the retrospective”, devfive-frontend·devup·devup-implementation-plan 스킬의 최종 보고 단계, `devup://guide/usage`에 있습니다. 완료를 뜻하는 도구 결과에만 `nextSuggestedAction`(id, invocation, `message.ko`/`message.en`, 규칙)이 붙습니다: `devup_runtime_audit` completion의 `reportAllowed: true`, `devup_screen_plan` status의 `completion.reportAllowed: true`, `devup_stack_diff`의 네 layer 전부 checked + drift 0. export의 `deliverable.isFinal`과 `devup_ui_validate` 통과는 작업 중 여러 번 나오는 중간 결과라 붙지 않습니다. 완료된 작업당 한 번만 제안하고, 중간 호출·실패·미해결 상태에서는 제안하지 않으며, 사용자가 수락하기 전에는 실행하지 않습니다. + 질문 도구에 호출당 개수 제한이 없으면(OpenCode `question`) 평평한 `questions` 목록을 한 번에 묻고, 제한이 있으면(Claude Code `AskUserQuestion`: 4) `maxQuestionsPerRound`(기본 4)로 나눈 `questionRounds`를 연달아 묻습니다. ## 읽기 전용·개인정보 보호 diff --git a/crates/devup-mcp/src/server/commands/opencode-devup.md b/crates/devup-mcp/src/server/commands/opencode-devup.md index bdbed9db..ad1cb63f 100644 --- a/crates/devup-mcp/src/server/commands/opencode-devup.md +++ b/crates/devup-mcp/src/server/commands/opencode-devup.md @@ -159,7 +159,9 @@ For Figma work, continue with the following branch after project intake: --deny-warnings`, `bun tsc --noEmit` in each changed app and `bun test` pass (`bun run test` adds cargo tarpaulin, which does not work on Windows), devup_ui_validate has no errors, site completeness filled. -13. Finish with the status finalReport and summary. +13. Finish with the status finalReport and summary. When the work is complete, + end by offering /devup-retro once in the user's language ("작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?"); + never after a failure, never run it unasked. User-owned decisions, before or after confirm, use the runtime question tool. Show the evidence table first. Keep table approval and individual decisions as diff --git a/crates/devup-mcp/src/server/completion_tests.rs b/crates/devup-mcp/src/server/completion_tests.rs index 4f10fdbc..230facac 100644 --- a/crates/devup-mcp/src/server/completion_tests.rs +++ b/crates/devup-mcp/src/server/completion_tests.rs @@ -372,6 +372,9 @@ async fn source_bound_static_checks_cannot_replace_figma_visuals() { input.visual_environments.insert(comparison_id, serde_json::from_value(json!({"tsxResource":{"uri":"devup://artifact/test/tsx","sha256":"a".repeat(64)},"themePath":"devup.json","assetDirectory":"public/images","devupUiVersion":"test","fontManifest":[{"family":"Test","sha256":"b".repeat(64)}]})).unwrap()); let result = write(&input, &policy, &store, None).await.unwrap(); assert_eq!(result["stage"], "data-verified", "{result}"); + // The one receipt that may be reported is the one that carries the offer. + assert!(super::super::skills::marks_completion("devup_runtime_audit", "completion", &result)); + assert!(!super::super::skills::marks_completion("devup_runtime_audit", "ingest", &result)); assert_eq!(refresh(&input.project_root, &policy, &store).await.unwrap()["done"], true); assert_eq!(result["dataVerificationScope"], "not-required"); // A syntactically present but unsigned empty inventory cannot remove all diff --git a/crates/devup-mcp/src/server/development_contract.rs b/crates/devup-mcp/src/server/development_contract.rs index 712f0f51..e30dd750 100644 --- a/crates/devup-mcp/src/server/development_contract.rs +++ b/crates/devup-mcp/src/server/development_contract.rs @@ -32,6 +32,7 @@ pub(super) const REQUIREMENTS: &[&str] = &[ "canonical-semantic-assets", "landing-detail-api-db-single-source", "runtime-evidence-before-completion", + "offer-devup-retro-in-final-report", ]; pub(super) const COMPLETION_EVIDENCE: &[&str] = &[ @@ -48,7 +49,7 @@ pub(super) const COMPLETION_EVIDENCE: &[&str] = &[ pub(super) fn attach(result: &mut Value) { if !result.is_object() { return; } result["developmentContract"] = json!({ - "version": 4, + "version": 5, "guide": "devup://guide/usage#behavior-and-data-contracts", "alwaysApplies": true, "requirements": REQUIREMENTS, @@ -66,11 +67,16 @@ mod tests { attach(&mut result); assert_eq!(result["developmentContract"]["alwaysApplies"], true); assert_eq!(result["developmentContract"]["staticExportProvesApplicationBehavior"], false); - assert_eq!(result["developmentContract"]["version"], 4); + assert_eq!(result["developmentContract"]["version"], 5); let requirements = result["developmentContract"]["requirements"].as_array().unwrap(); for required in ["route-owned-page-body", "devup-api-only", "stop-affected-api-gaps", "paired-e2e-and-native-figma-visuals"] { assert!(requirements.contains(&json!(required))); } + assert!(requirements.contains(&json!("offer-devup-retro-in-final-report"))); + for term in ["Final report: offer the retrospective", "Ask once per completed task", "Never run it +yourself", "carries no offer", super::super::skills::RETRO_OFFER_FIELD] { + assert!(RULES.contains(term), "missing completion offer: {term}"); + } for term in ["keep-all", "grayscale", "layout.tsx", "count", "footer", "DB", "aria-current", "sidecar"] { assert!(RULES.contains(term), "missing delivery policy: {term}"); } @@ -136,6 +142,6 @@ mod tests { unique.sort(); unique.dedup(); assert_eq!(unique.len(), REQUIREMENTS.len() + COMPLETION_EVIDENCE.len()); - assert_eq!((REQUIREMENTS.len(), COMPLETION_EVIDENCE.len()), (20, 8)); + assert_eq!((REQUIREMENTS.len(), COMPLETION_EVIDENCE.len()), (21, 8)); } } diff --git a/crates/devup-mcp/src/server/handlers/devup_runtime_audit.rs b/crates/devup-mcp/src/server/handlers/devup_runtime_audit.rs index 9a5b8026..976389e6 100644 --- a/crates/devup-mcp/src/server/handlers/devup_runtime_audit.rs +++ b/crates/devup-mcp/src/server/handlers/devup_runtime_audit.rs @@ -17,7 +17,11 @@ impl DevupServer { false, )) }; - let result = match input.action.as_str() { + let completion_root = input + .completion + .as_ref() + .map(|completion| completion.project_root.clone()); + let mut result = match input.action.as_str() { "plan" => { let mut plan = input .plan @@ -77,6 +81,9 @@ impl DevupServer { _ => return Err(invalid("action must be plan, ingest or completion.")), } .map_err(to_mcp_error)?; + // Only the completion receipt can say the application is finished. + let complete = skills::marks_completion("devup_runtime_audit", &input.action, &result); + self.offer_retro(&mut result, completion_root.as_deref(), complete); Ok(tool_result(result)) } } diff --git a/crates/devup-mcp/src/server/handlers/devup_screen_plan.rs b/crates/devup-mcp/src/server/handlers/devup_screen_plan.rs index 21139d77..c6495291 100644 --- a/crates/devup-mcp/src/server/handlers/devup_screen_plan.rs +++ b/crates/devup-mcp/src/server/handlers/devup_screen_plan.rs @@ -47,6 +47,9 @@ impl DevupServer { { prefer_bridge_reference_calls(&mut result); } + // Status is done only after application completion was re-evaluated. + let complete = skills::marks_completion("devup_screen_plan", &input.action, &result); + self.offer_retro(&mut result, input.project_root.as_deref(), complete); Ok(tool_result(result)) } } diff --git a/crates/devup-mcp/src/server/handlers/devup_skills.rs b/crates/devup-mcp/src/server/handlers/devup_skills.rs index 071c83e6..7e81c9ef 100644 --- a/crates/devup-mcp/src/server/handlers/devup_skills.rs +++ b/crates/devup-mcp/src/server/handlers/devup_skills.rs @@ -1,5 +1,18 @@ use super::*; +impl DevupServer { + /// Adds the /devup-retro offer to a result that marks completion. Called + /// only by the tools whose result can mean "the work is finished". + pub(super) fn offer_retro(&self, result: &mut Value, project_root: Option<&str>, complete: bool) { + let lookup = skills::Lookup::new( + project_root, + self.output_policy.primary_root(), + self.client_name(), + ); + skills::offer_retro(result, &lookup, complete); + } +} + #[tool_router(router = devup_skills_router, vis = "pub(super)")] impl DevupServer { #[tool( diff --git a/crates/devup-mcp/src/server/handlers/devup_stack_diff.rs b/crates/devup-mcp/src/server/handlers/devup_stack_diff.rs index 938cf09a..bed5dc1e 100644 --- a/crates/devup-mcp/src/server/handlers/devup_stack_diff.rs +++ b/crates/devup-mcp/src/server/handlers/devup_stack_diff.rs @@ -10,9 +10,11 @@ impl DevupServer { &self, Parameters(input): Parameters, ) -> Result { - let result = stack_diff::run(input.project_root.as_deref(), &input.layers) + let mut result = stack_diff::run(input.project_root.as_deref(), &input.layers) .await .map_err(to_mcp_error)?; + let complete = skills::marks_completion("devup_stack_diff", "", &result); + self.offer_retro(&mut result, input.project_root.as_deref(), complete); Ok(tool_result(result)) } } diff --git a/crates/devup-mcp/src/server/skills/devfive-frontend/SKILL.md b/crates/devup-mcp/src/server/skills/devfive-frontend/SKILL.md index 62927caf..049f5fdd 100644 --- a/crates/devup-mcp/src/server/skills/devfive-frontend/SKILL.md +++ b/crates/devup-mcp/src/server/skills/devfive-frontend/SKILL.md @@ -77,6 +77,11 @@ Common coding conventions for DevFive frontend projects (puzzle-fit, girok-space ## Links, controls and completion evidence +End the final completion report by offering the retrospective once, in the user's +language ("작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?" / "The work is complete. Shall I run /devup-retro to turn this session's follow-up prompts into devup-mcp improvements?"; `/mcp____devup-retro` when the skill is not installed). +Ask once per completed task, never after an intermediate call or a failure, and never +run it unasked. A completion-state tool result carries the sentence as `nextSuggestedAction`. + `devup_figma_export` produces static visual markup. Its `interactiveCandidates` are a review list, and `deliverable.isFinal` never proves working navigation or forms. Resolve GNB, logo, footer, CTA, cards, “view all”, tabs and breadcrumbs diff --git a/crates/devup-mcp/src/server/skills/devfive-frontend/references/delivery-quality.md b/crates/devup-mcp/src/server/skills/devfive-frontend/references/delivery-quality.md index 12b06f6d..fc716a51 100644 --- a/crates/devup-mcp/src/server/skills/devfive-frontend/references/delivery-quality.md +++ b/crates/devup-mcp/src/server/skills/devfive-frontend/references/delivery-quality.md @@ -297,6 +297,26 @@ replace the agreed CSS viewport, DPR or zoom. If the required capture viewport cannot fit onscreen, use an independent automated browser context at that size; do not change application layout, CSS zoom or thresholds to make a pane match. +## Final report: offer the retrospective + +When the work is complete, end the final completion report with one question +offering the session retrospective, in the user's language: + +- Korean: "작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?" +- otherwise: "The work is complete. Shall I run /devup-retro to turn this session's follow-up prompts into devup-mcp improvements?" + +Use the invocation that works in this client: `/devup-retro` when the +`devup-retro` skill is installed, otherwise the MCP prompt form +`/mcp____devup-retro`. A tool result that marks completion carries the +ready sentence as `nextSuggestedAction` (`devup_runtime_audit` completion with +`reportAllowed`, `devup_screen_plan` status with `completion.reportAllowed`, +`devup_stack_diff` with every layer checked and zero drift). An export's +`deliverable.isFinal` grades generated source and carries no offer. + +Ask once per completed task. Not after an intermediate tool call, a failed or +unresolved result, or a partial report, and not again once asked. Never run it +yourself: it starts only when the user says yes. + ## Contract identifiers Export replies carry `developmentContract` as identifiers only, to stay inside @@ -329,6 +349,7 @@ before claiming completion. The installed copy of this document is | `canonical-semantic-assets` | One asset registry, semantic file names, no duplicates or hash names | | `landing-detail-api-db-single-source` | Landing and detail values come from the same API and DB records | | `runtime-evidence-before-completion` | Completion is claimed from a runtime audit, not from the export or a screenshot | +| `offer-devup-retro-in-final-report` | The final completion report ends by offering `/devup-retro` once, in the user's language; it is never run unasked | `completionEvidence`: diff --git a/crates/devup-mcp/src/server/skills/devup-implementation-plan/SKILL.md b/crates/devup-mcp/src/server/skills/devup-implementation-plan/SKILL.md index dfa9a1f9..834c2c35 100644 --- a/crates/devup-mcp/src/server/skills/devup-implementation-plan/SKILL.md +++ b/crates/devup-mcp/src/server/skills/devup-implementation-plan/SKILL.md @@ -539,6 +539,12 @@ its computer-use setup yourself. Verify status and a small read before export. design fixes requested, the excluded frames, the assumptions and the dependencies line - and its one-line `summary`. Report every `fail`, `not-run`, unresolved finding and assumption as what it is. + Then offer the retrospective once, in the user's language: + "작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?" + (otherwise "The work is complete. Shall I run /devup-retro to turn this session's follow-up prompts into devup-mcp improvements?"; + `/mcp____devup-retro` when the skill is not installed). Only when + the work is complete, never after an intermediate call or a failure, and + never run it unasked. Hard rules, after confirm: diff --git a/crates/devup-mcp/src/server/skills/devup/SKILL.md b/crates/devup-mcp/src/server/skills/devup/SKILL.md index 26fa227b..248bb937 100644 --- a/crates/devup-mcp/src/server/skills/devup/SKILL.md +++ b/crates/devup-mcp/src/server/skills/devup/SKILL.md @@ -267,6 +267,12 @@ Then run `bun run build`, `bun run lint`, `bun tsc --noEmit` width (health, geometry, page, section, asset, interaction, screenshot, diffs), unresolved findings with evidence, pending decisions taken, assumptions, design fixes, exclusions, defaults and the dependencies line. + Then offer the retrospective once, in the user's language: + "작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?" + (otherwise "The work is complete. Shall I run /devup-retro to turn this session's follow-up prompts into devup-mcp improvements?"; + `/mcp____devup-retro` when the skill is not installed). Only when + the work is complete, never after an intermediate call or a failure, and + never run it unasked. Hard rules: visually verify every screen, section and asset at every drawn width; never stop to wait, reorder the work; done = every required cell passes; diff --git a/crates/devup-mcp/src/server/skills/manifest.json b/crates/devup-mcp/src/server/skills/manifest.json index a6e6abc1..3a5213b2 100644 --- a/crates/devup-mcp/src/server/skills/manifest.json +++ b/crates/devup-mcp/src/server/skills/manifest.json @@ -51,8 +51,8 @@ "documents": [ { "path": "SKILL.md", - "bytes": 31428, - "sha256": "e4729816c355ee065c1a5908516a68c95c9bca59131c62958404ce94db30d320" + "bytes": 32009, + "sha256": "3b40ffaecfcab15bf531bf84197a5c0c377392ed57ca8ad888f237e415acf57d" }, { "path": "references/critical-rules.md", @@ -76,8 +76,8 @@ }, { "path": "references/delivery-quality.md", - "bytes": 25551, - "sha256": "b5e6013167c80f785b93ae4c80b4ad2f300a381610065a7fcf183524f79e48a3" + "bytes": 26870, + "sha256": "ee1dd0b19c1a4bff42709af86e19e15496ccc2f22fa99b60ffa9490db2574362" }, { "path": "references/animation.md", @@ -99,8 +99,8 @@ "documents": [ { "path": "SKILL.md", - "bytes": 45398, - "sha256": "90ced638e1f3ad023bf991bc69872fae18933431cb3f5302f023cb716156bbe4" + "bytes": 45897, + "sha256": "ddbfcb3588c4c61c1e867abae24d062f4a88ed838fd47e7917d98edcfd498e2c" } ], "sourceUrl": "https://github.com/dev-five-git/devup-mcp/blob/HEAD/crates/devup-mcp/src/server/skills/devup-implementation-plan", @@ -117,8 +117,8 @@ "documents": [ { "path": "SKILL.md", - "bytes": 24646, - "sha256": "41e42e8aa489c4d8b51fcfbe02fa6cc53a72dfdab2713cd284b656666c41c58d" + "bytes": 25151, + "sha256": "52c926b03727b9b05bd09a2a8875f1dbbb71c253ef60cb4054434f4d600b5fdc" } ], "sourceUrl": "https://github.com/dev-five-git/devup-mcp/blob/HEAD/crates/devup-mcp/src/server/skills/devup", diff --git a/crates/devup-mcp/src/server/skills/retro.rs b/crates/devup-mcp/src/server/skills/retro.rs index e8ee8366..6ac55650 100644 --- a/crates/devup-mcp/src/server/skills/retro.rs +++ b/crates/devup-mcp/src/server/skills/retro.rs @@ -367,3 +367,67 @@ pub fn retro_prompt_result( let title = find_by_name(RETRO_NAME).map_or(RETRO_NAME, |skill| &skill.record.title); Some(rmcp::model::GetPromptResult::new(vec![message]).with_description(title).into()) } + +/// What the agent says at the end of its final completion report, once. +/// +/// The retrospective only improves anything if it is run, and the moment it +/// is worth running is the one where the user is about to leave. The server +/// cannot speak to the user, so a tool result that marks completion carries +/// the sentence for the agent to say - with the command, never instead of the +/// user's decision to run it. +pub const RETRO_OFFER_KO: &str = + "작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 {} 를 실행할까요?"; +pub const RETRO_OFFER_EN: &str = + "The work is complete. Shall I run {} to turn this session's follow-up prompts into devup-mcp improvements?"; +pub const RETRO_OFFER_FIELD: &str = "nextSuggestedAction"; + +/// How the command is typed in this workspace: the slash command when the +/// skill is installed, otherwise the MCP prompt as Claude Code names it. +pub fn retro_invocation(lookup: &Lookup) -> String { + if lookup.installed_paths(RETRO_NAME).is_empty() { + format!("/mcp__devup-mcp__{RETRO_NAME}") + } else { + format!("/{RETRO_NAME}") + } +} + +/// Attaches the offer to a result that marks completion, and only to one. +/// `complete` is the caller's own completion flag; an intermediate or failing +/// result gets nothing, so the offer cannot become a line on every reply. +pub fn offer_retro(result: &mut serde_json::Value, lookup: &Lookup, complete: bool) { + if !complete || !result.is_object() { + return; + } + let invocation = retro_invocation(lookup); + result[RETRO_OFFER_FIELD] = serde_json::json!({ + "id": RETRO_NAME, + "invocation": invocation, + "skillInstalled": invocation == format!("/{RETRO_NAME}"), + "message": { + "ko": RETRO_OFFER_KO.replace("{}", &invocation), + "en": RETRO_OFFER_EN.replace("{}", &invocation), + }, + "when": "final-completion-report", + "how": "End the final completion report for this task with this one question, in the \ + user's language. Ask once per completed task: not after an intermediate call, a \ + failure or an unresolved item, and not again if already asked. Never run it \ + yourself; run it only when the user says yes. /mcp__devup-mcp__ assumes the \ + server is registered as devup-mcp; use the client's own server name.", + }); +} + +/// Whether a tool result says the work is finished: the one place the three +/// completion signals are spelled, so a handler cannot offer on anything else. +/// +/// `devup_figma_export` is deliberately absent. `deliverable.isFinal` grades +/// generated source and is the first step of the work, not its end. +pub fn marks_completion(tool: &str, action: &str, result: &serde_json::Value) -> bool { + match (tool, action) { + ("devup_runtime_audit", "completion") => { + result["kind"] == "devup-completion-receipt" && result["reportAllowed"] == true + } + ("devup_screen_plan", "status") => result["completion"]["reportAllowed"] == true, + ("devup_stack_diff", _) => super::stack_diff::is_zero_drift(result), + _ => false, + } +} diff --git a/crates/devup-mcp/src/server/skills/retro_tests.rs b/crates/devup-mcp/src/server/skills/retro_tests.rs index ec37d366..ae38b40c 100644 --- a/crates/devup-mcp/src/server/skills/retro_tests.rs +++ b/crates/devup-mcp/src/server/skills/retro_tests.rs @@ -244,3 +244,128 @@ language:"), "a blank argument is not passed on"); std::fs::remove_dir_all(project).unwrap(); } } + +#[cfg(test)] +mod retro_offer_tests { + use super::*; + + fn scratch(label: &str) -> PathBuf { + let path = std::env::temp_dir().join(format!("devup-retro-offer-{label}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).expect("scratch directory"); + path + } + + /// The offer is for a finished task. A result that does not mark + /// completion gets nothing, so the sentence cannot ride on every reply. + #[test] + fn only_a_completion_result_carries_the_offer() { + let project = scratch("gate"); + let lookup = Lookup::project_only(project.clone(), Runtime::ClaudeCode); + let mut pending = serde_json::json!({"done": false}); + offer_retro(&mut pending, &lookup, false); + assert!(pending.get(RETRO_OFFER_FIELD).is_none(), "{pending}"); + let mut not_an_object = serde_json::json!([]); + offer_retro(&mut not_an_object, &lookup, true); + assert_eq!(not_an_object, serde_json::json!([])); + + let mut done = serde_json::json!({"done": true}); + offer_retro(&mut done, &lookup, true); + let offer = &done[RETRO_OFFER_FIELD]; + assert_eq!(offer["id"], RETRO_NAME); + assert_eq!(offer["when"], "final-completion-report"); + // Not installed here: the MCP prompt form is the one that works. + assert_eq!(offer["skillInstalled"], false); + assert_eq!(offer["invocation"], format!("/mcp__devup-mcp__{RETRO_NAME}")); + assert_eq!( + offer["message"]["ko"], + format!("작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /mcp__devup-mcp__{RETRO_NAME} 를 실행할까요?") + ); + let how = offer["how"].as_str().unwrap(); + for rule in ["once per completed task", "not after an intermediate call, a failure", "Never run it yourself", "user's language"] { + assert!(how.split_whitespace().collect::>().join(" ").contains(rule), "{rule}"); + } + std::fs::remove_dir_all(project).unwrap(); + } + + #[tokio::test] + async fn an_installed_skill_is_offered_as_the_slash_command() { + let project = scratch("installed"); + let policy = super::super::output::OutputPolicy::from_roots(vec![project.clone()]).unwrap(); + let lookup = Lookup::project_only(project.clone(), Runtime::ClaudeCode); + install(&policy, &lookup, &[RETRO_NAME.to_owned()]).await.unwrap(); + let mut done = serde_json::json!({}); + offer_retro(&mut done, &lookup, true); + let offer = &done[RETRO_OFFER_FIELD]; + assert_eq!(offer["skillInstalled"], true); + assert_eq!(offer["invocation"], format!("/{RETRO_NAME}")); + assert!(offer["message"]["ko"].as_str().unwrap().contains(&format!("/{RETRO_NAME} 를 실행할까요?"))); + assert!(offer["message"]["en"].as_str().unwrap().contains(&format!("Shall I run /{RETRO_NAME} "))); + drop(policy); + std::fs::remove_dir_all(project).unwrap(); + } + + #[test] + fn only_three_results_mark_completion() { + use serde_json::json; + let receipt = |allowed: bool| json!({"kind": "devup-completion-receipt", "stage": if allowed {"data-verified"} else {"browser-verified"}, "done": allowed, "reportAllowed": allowed}); + assert!(marks_completion("devup_runtime_audit", "completion", &receipt(true))); + assert!(!marks_completion("devup_runtime_audit", "completion", &receipt(false))); + assert!(!marks_completion("devup_runtime_audit", "plan", &receipt(true))); + assert!(!marks_completion("devup_runtime_audit", "ingest", &json!({"verdict": "pass"}))); + + let status = |allowed: bool| json!({"done": allowed, "completion": {"done": allowed, "reportAllowed": allowed}}); + assert!(marks_completion("devup_screen_plan", "status", &status(true))); + assert!(!marks_completion("devup_screen_plan", "status", &status(false))); + assert!(!marks_completion("devup_screen_plan", "auditPlan", &status(true))); + + let layer = |checked: bool, drifts: serde_json::Value| json!({"checked": checked, "drifts": drifts}); + let stack = |layers: serde_json::Value| json!({"found": true, "layers": layers}); + let clean = || layer(true, json!([])); + let all = stack(json!({"db-entity": clean(), "entity-route": clean(), "route-openapi": clean(), "openapi-client": clean()})); + assert!(marks_completion("devup_stack_diff", "", &all)); + let drifted = stack(json!({"db-entity": clean(), "entity-route": layer(true, json!([{"kind": "missing"}])), "route-openapi": clean(), "openapi-client": clean()})); + assert!(!marks_completion("devup_stack_diff", "", &drifted)); + let skipped = stack(json!({"db-entity": layer(false, json!([])), "entity-route": clean(), "route-openapi": clean(), "openapi-client": clean()})); + assert!(!marks_completion("devup_stack_diff", "", &skipped), "an unchecked layer is not zero drift"); + let partial = stack(json!({"db-entity": clean()})); + assert!(!marks_completion("devup_stack_diff", "", &partial), "one layer is not the stack"); + assert!(!marks_completion("devup_stack_diff", "", &json!({"found": false}))); + + // The export never marks completion, whatever its deliverable says, + // and neither does a clean validation: both happen many times a task. + let export = json!({"deliverable": {"isFinal": true}, "reportAllowed": true, "completion": {"reportAllowed": true}}); + assert!(!marks_completion("devup_figma_export", "", &export)); + assert!(!marks_completion("devup_ui_validate", "", &json!({"errors": [], "completion": {"reportAllowed": true}}))); + } + + /// Wherever an agent reads how to finish, it reads the same question. + #[test] + fn every_completion_text_states_the_offer_and_its_limits() { + let ko = RETRO_OFFER_KO.replace("{}", &format!("/{RETRO_NAME}")); + let en = RETRO_OFFER_EN.replace("{}", &format!("/{RETRO_NAME}")); + let flat = |text: &str| text.split_whitespace().collect::>().join(" "); + let embedded = |name: &str, path: &str| { + EMBEDDED.iter().find(|(key, _)| *key == name).unwrap().1.iter().find(|(file, _)| *file == path).unwrap().1 + }; + let guide = super::super::guide::GUIDE; + for (label, text) in [ + ("delivery-quality", embedded("devfive-frontend", "references/delivery-quality.md")), + ("devfive-frontend", embedded("devfive-frontend", "SKILL.md")), + ("devup", embedded("devup", "SKILL.md")), + ("devup-implementation-plan", embedded("devup-implementation-plan", "SKILL.md")), + ("guide", guide), + ] { + let text = flat(text); + assert!(text.contains(&ko), "{label}: Korean offer"); + assert!(text.contains(&flat(&en)), "{label}: English offer"); + assert!(text.contains(&format!("/mcp____{RETRO_NAME}")), "{label}: prompt form"); + assert!(text.to_lowercase().contains("once"), "{label}: once"); + assert!(text.to_lowercase().contains("never run it") || text.contains("never run unasked"), "{label}: never auto-run"); + } + let opencode = flat(COMMANDS[0].opencode.unwrap()); + assert!(opencode.contains(&ko) && opencode.contains("never run it unasked")); + assert!(guide.contains("`reportAllowed: true` carries `nextSuggestedAction`")); + assert!(!super::super::guide::INSTRUCTIONS.contains(RETRO_NAME), "the bootstrap budget is spent elsewhere"); + } +} diff --git a/crates/devup-mcp/src/server/stack_diff/contracts_and_layers.rs b/crates/devup-mcp/src/server/stack_diff/contracts_and_layers.rs index 79d4eac6..368d8027 100644 --- a/crates/devup-mcp/src/server/stack_diff/contracts_and_layers.rs +++ b/crates/devup-mcp/src/server/stack_diff/contracts_and_layers.rs @@ -82,6 +82,19 @@ pub async fn run(project_root: Option<&str>, layers: &[String]) -> Result bool { + result["found"] == true + && ALL_LAYERS.iter().all(|layer| { + let layer = &result["layers"][*layer]; + layer.is_object() + && layer["checked"] == true + && layer["drifts"].as_array().is_some_and(Vec::is_empty) + }) +} + /// Ownership is part of every finding, including scan self-checks. Generated /// artifacts are evidence to compare, never a destination for manual repairs. pub(super) fn attach_source_ownership(result: &mut Value, layer: &str) { diff --git a/crates/devup-mcp/src/server/text/completion.md b/crates/devup-mcp/src/server/text/completion.md index ca777e4e..20f0571e 100644 --- a/crates/devup-mcp/src/server/text/completion.md +++ b/crates/devup-mcp/src/server/text/completion.md @@ -74,3 +74,7 @@ Internal clipping checks never waive an entire section merely because a child animates. `allowedMarquees` names exact selectors and requires one visible, horizontal-only clipped, infinitely translating nowrap track without controls. Vertical clipping, unrelated siblings and invalid exemptions still fail. + +A completion receipt with `reportAllowed: true` carries `nextSuggestedAction`: +end the final completion report by offering `/devup-retro` once, in the user's +language. Do not offer it for any other stage, and never run it unasked. diff --git a/crates/devup-mcp/src/server/text/guide-GUIDE-c682b0c5.md b/crates/devup-mcp/src/server/text/guide-GUIDE-c682b0c5.md index e45031e3..8dbc7d94 100644 --- a/crates/devup-mcp/src/server/text/guide-GUIDE-c682b0c5.md +++ b/crates/devup-mcp/src/server/text/guide-GUIDE-c682b0c5.md @@ -435,3 +435,10 @@ the list; only then print one ready-to-paste prompt per surface. Start with `devup_skills` action `retro`: it returns this build, the calls this process served with timestamps, the surface list and the project's `.feedback/` state, and writes nothing. Nothing is written before the user confirms. + +Offer it: end the final completion report with one question in the user's +language, "작업이 완료되었습니다. 지금까지 진행한 내용을 개선하기 위해 /devup-retro 를 실행할까요?" or +"The work is complete. Shall I run /devup-retro to turn this session's follow-up prompts into devup-mcp improvements?" +(`/mcp____devup-retro` when the skill is not installed). Once per +completed task, only on completion, never run unasked; completion-state tool +results carry the sentence as `nextSuggestedAction`. diff --git a/crates/devup-mcp/tests/export_project_theme.rs b/crates/devup-mcp/tests/export_project_theme.rs index 6bd1140b..a18d4428 100644 --- a/crates/devup-mcp/tests/export_project_theme.rs +++ b/crates/devup-mcp/tests/export_project_theme.rs @@ -184,6 +184,8 @@ async fn export_context_summarizes_a_large_project_under_fifteen_kib() -> anyhow .is_none() ); assert!(bytes < 15 * 1024, "reply is {bytes} bytes"); + // An export is the first step, not the end of the work: no retro offer. + assert!(result.get("nextSuggestedAction").is_none()); // The budget is a wall only if nothing can be added before hitting it. // #124 had 31 bytes left and renamed two contract identifiers instead of // adding them. Standing prose now lives in the guide (contract meanings, @@ -200,7 +202,7 @@ async fn export_context_summarizes_a_large_project_under_fifteen_kib() -> anyhow let contract = &result["developmentContract"]; assert_eq!( contract["requirements"].as_array().map(Vec::len), - Some(20), + Some(21), "{contract}" ); assert_eq!( diff --git a/crates/devup-mcp/tests/retro_command.rs b/crates/devup-mcp/tests/retro_command.rs index 0b447ca4..236fbc3f 100644 --- a/crates/devup-mcp/tests/retro_command.rs +++ b/crates/devup-mcp/tests/retro_command.rs @@ -239,3 +239,30 @@ async fn retro_reports_the_served_calls_and_the_feedback_folder_and_writes_nothi std::fs::remove_dir_all(workspace)?; Ok(()) } + +/// The offer rides only on a result that marks completion. Through the real +/// process, a stack diff that found nothing to compare, a skills status and a +/// retro report are not that, and must not carry it. +#[tokio::test] +async fn results_that_do_not_mark_completion_carry_no_offer() -> anyhow::Result<()> { + let workspace = scratch("no-offer"); + let root = workspace.display().to_string(); + session(&workspace, async |client| { + let arguments: Map = + json!({"projectRoot": root}).as_object().cloned().unwrap(); + let diff = client + .call_tool(CallToolRequestParams::new("devup_stack_diff").with_arguments(arguments)) + .await? + .structured_content + .expect("a structured response"); + assert!(diff.get("nextSuggestedAction").is_none(), "{diff}"); + for action in ["status", "retro"] { + let result = call(client, json!({"action": action, "projectRoot": root})).await?; + assert!(result.get("nextSuggestedAction").is_none(), "{action}"); + } + Ok(()) + }) + .await?; + std::fs::remove_dir_all(workspace)?; + Ok(()) +}