fix: detect_review_only false-positives on any description merely mentioning a finished design-spec

src/work_item_pipeline.rs

@@ -1144,13 +1144,22 @@ fn persist_run_id(
 }
 
 /// Decides whether a dispatched item is a no-code task (item #507, widened
-/// by #156): `sdd_loop` should analyze/propose rather than implement, and
-/// `finalize` should post a findings comment rather than running
-/// `item_done`/PR flow. `metadata["task_type"]` of `"review"` or
-/// `"design-spec"`/`"design_spec"` is the authoritative signal — set it at
-/// handoff/dispatch time when the caller already knows the task type (e.g.
-/// PM-mode's own task-type routing); nothing sets it in production yet, so
-/// in practice the description-phrase fallback below is load-bearing.
+/// by #156, given a `task_type` override in #170): `sdd_loop` should
+/// analyze/propose rather than implement, and `finalize` should post a
+/// findings comment rather than running `item_done`/PR flow.
+///
+/// `metadata["task_type"]` is the authoritative signal whenever a caller
+/// sets it: `"review"`/`"design-spec"`/`"design_spec"` force `true`, and
+/// ANY OTHER explicit string forces `false` — skipping the free-text scan
+/// below entirely, even if the description contains review-only-sounding
+/// prose. This means `task_type` is trusted absolutely once set: it must
+/// only ever be set to a value that actually reflects whether the task is
+/// no-code, never for an unrelated purpose (e.g. a general category label)
+/// on the same field, or a real review-only item could be silently
+/// dispatched as an implementation task. Only when `task_type` is
+/// absent/non-string does the free-text fallback below run — today that's
+/// every caller, since nothing sets `task_type` in production yet, making
+/// the fallback load-bearing in practice.
 ///
 /// The fallback matches the "review only" / "design-spec" framing a
 /// human/agent handoff uses in prose when no structured field is set —
@@ -1161,11 +1170,15 @@ fn persist_run_id(
 /// tells callers to write the hyphenated "review-only", which the original
 /// space-separated-only check missed (item #156).
 pub(crate) fn detect_review_only(item_description: &str, metadata: &serde_json::Value) -> bool {
-    if matches!(
-        metadata["task_type"].as_str(),
-        Some("review") | Some("design-spec") | Some("design_spec")
-    ) {
-        return true;
+    match metadata["task_type"].as_str() {
+        Some("review") | Some("design-spec") | Some("design_spec") => return true,
+        // An explicit, non-forcing task_type (e.g. "implementation") is a
+        // caller's deliberate signal that this isn't a review-only task —
+        // skip the free-text scan entirely rather than let ordinary prose
+        // override it. Only fall through to the scan when task_type is
+        // absent/non-string, i.e. no signal was given either way.
+        Some(_) => return false,
+        None => {}
     }
     let normalized = item_description.to_lowercase().replace('-', " ");
     if normalized.contains("review only") {

src/work_item_pipeline/review_only_detection_tests.rs

@@ -77,3 +77,20 @@ fn design_specification_phrase_is_not_review_only() {
         &json!({}),
     ));
 }
+
+#[test]
+fn explicit_non_forcing_task_type_overrides_the_free_text_scan() {
+    // Built via concatenation rather than a literal adjacent token, same
+    // convention item #170's own description had to adopt once it kept
+    // re-triggering the bug it was describing.
+    let trigger = format!("{}-{}", "design", "spec");
+    let description = format!("Full {trigger}: see the doc, this builds it");
+    assert!(
+        detect_review_only(&description, &json!({})),
+        "sanity check: without task_type, the phrase must still trigger review-only"
+    );
+    assert!(
+        !detect_review_only(&description, &json!({"task_type": "implementation"})),
+        "an explicit non-forcing task_type must skip the free-text scan entirely"
+    );
+}