Refuse starting a task without an approved spec; requires_spec light lane (closes #3710)

packages/agent/src/seeds/skills/breaking-a-plan-into-tasks.md

@@ -1,6 +1,6 @@
 Tasks are how a plan gets done: one task per unit of work someone can pick up and finish, each with a checklist that says when it is finished. A task under a plan is an ordinary task. What ties it to the plan is two links and its checklist.
 
-A SMALL CHANGE NEEDS NO SPEC AND NO PLAN: a task with a checklist of its own is ready to work on as it is. Write the task and its checklist and stop there. Never write a spec or a plan only to have something to link a task to.
+A SMALL CHANGE NEEDS NO SPEC AND NO PLAN: a task with a checklist of its own and `requires_spec` set to false is ready to work on as it is, and without that field a task cannot be started until it links an approved spec. Write the task and its checklist, set `requires_spec` to false once the user agrees it is small, and stop there. Never write a spec or a plan only to have something to link a task to.
 
 READ THE PLAN AND ITS SPEC FIRST: <!-- include: task-breakdown-read --> Read the plan's `approach` and every criterion of the spec. If the plan is not approved, say so: its tasks can be written now, and none can start until the user approves the plan.
 

packages/agent/src/seeds/skills/implementing-a-task.md

@@ -1,6 +1,6 @@
 This is the whole procedure for doing a task, from taking it to handing it over for review. Reviewing it is separate work with its own procedure: do not review a task you implemented, and do not mark it done.
 
-STEP 1, TAKE A READY TASK: <!-- include: task-implement-take --> A ready task is open, has a checklist, is not blocked, and has no plan waiting on approval. A small change needs no spec or plan: a task with a checklist of its own is taken and worked the same way. When the user names a task, or you are resuming one already in progress, read that task. <!-- include: task-context-read -->
+STEP 1, TAKE A READY TASK: <!-- include: task-implement-take --> A ready task is open, has a checklist, is not blocked, has no plan waiting on approval, and links an approved spec or is marked `requires_spec` false. A small change needs no spec or plan: a task with a checklist of its own and `requires_spec` set to false is taken and worked the same way. When the user names a task, or you are resuming one already in progress, read that task. <!-- include: task-context-read -->
 
 STEP 2, START IT WITH THE VERSION YOU READ: <!-- include: task-implement-start --> Starting is how a task is claimed: of two sessions that read the same task, the second start is refused.
 

packages/agent/tests/it/workflow_seeds.rs

@@ -372,7 +372,15 @@ async fn a_task_carries_the_procedure_of_the_stage_it_is_in() {
 async fn a_skill_attached_to_a_project_comes_with_each_of_its_tasks() {
     let h = start().await;
     let work = planned_task(&h).await;
-    let second = h.create("task", "Document the export", json!({})).await;
+    // A small change: ready with a checklist of its own once it is marked as
+    // not needing a spec.
+    let second = h
+        .create(
+            "task",
+            "Document the export",
+            json!({ "requires_spec": false }),
+        )
+        .await;
     h.child(&second, "checkbox", "- [ ] The guide covers it")
         .await;
     h.link(&work.project, "tasks", &second).await;
@@ -392,7 +400,9 @@ async fn a_skill_attached_to_a_project_comes_with_each_of_its_tasks() {
         assert_eq!(skill_names(&read), [IMPLEMENTING, "Apollo Naming"]);
     }
     // A task of no project does not.
-    let other = h.create("task", "Unrelated", json!({})).await;
+    let other = h
+        .create("task", "Unrelated", json!({ "requires_spec": false }))
+        .await;
     h.child(&other, "checkbox", "- [ ] Done").await;
     let read = h.ok("get_node_context", json!({ "id": uri(&other) })).await;
     assert_eq!(skill_names(&read), [IMPLEMENTING]);

packages/core/src/models/core_schemas.rs

@@ -362,6 +362,34 @@ pub fn get_core_schemas() -> Vec<SchemaNode> {
                     "The commits that delivered the task, each a title and URL. Written as the \
                      whole list.",
                 ),
+                // The light lane (ADR-097 §6). The task-spec Play reads it, so
+                // it is a core field: removing it would leave the rule with
+                // nothing to ask.
+                SchemaField {
+                    name: "requires_spec".to_string(),
+                    friendly_name: "Requires spec".to_string(),
+                    field_type: crate::models::SchemaFieldType::Boolean,
+                    local_only: false,
+                    protection: SchemaProtectionLevel::Core,
+                    core_values: None,
+                    user_values: None,
+                    indexed: false,
+                    required: Some(false),
+                    extensible: None,
+                    default: Some(serde_json::json!(true)),
+                    description: Some(
+                        "Whether the task must link an approved spec before it can be started. \
+                         True by default. Set to false for work that needs no spec, such as a \
+                         chore or a small fix: the task can then be started with no spec \
+                         linked."
+                            .to_string(),
+                    ),
+                    item_type: None,
+                    fields: None,
+                    item_fields: None,
+                    unique: None,
+                    unique_case_insensitive: None,
+                },
             ],
             // A task's assignee is the derived inverse of person's `tasks`
             // relationship declaration below (mirrors project ↔ task), and its
@@ -2656,7 +2684,7 @@ mod tests {
         let schemas = get_core_schemas();
         let task = schemas.iter().find(|s| s.envelope.id == "task").unwrap();
 
-        assert_eq!(task.fields.len(), 7);
+        assert_eq!(task.fields.len(), 8);
         assert!(task.get_field("status").is_some());
         assert!(task.get_field("priority").is_some());
         assert!(task.get_field("due_date").is_some());
@@ -2915,6 +2943,19 @@ mod tests {
         assert_eq!(count("decision"), 1);
     }
 
+    /// ADR-097 §6: the light lane is a declared boolean whose schema default
+    /// is that a spec is required, so a task nobody marked is held to the rule.
+    #[test]
+    fn task_declares_the_light_lane_field() {
+        let schemas = get_core_schemas();
+        let task = schemas.iter().find(|s| s.envelope.id == "task").unwrap();
+        let field = task.get_field("requires_spec").expect("task declares it");
+        assert_eq!(field.field_type, SchemaFieldType::Boolean);
+        assert_eq!(field.default, Some(serde_json::json!(true)));
+        assert_eq!(field.required, Some(false));
+        assert_eq!(field.protection, SchemaProtectionLevel::Core);
+    }
+
     /// ADR-092 §4: the three link fields, all optional.
     #[test]
     fn project_and_task_declare_their_link_fields() {

packages/core/src/playbook/core_plays.rs

@@ -41,6 +41,9 @@ pub const TASK_CRITERIA_PLAY_ID: &str = "95796b4d-f078-44f8-807b-8a2e92167a2d";
 /// Node id of the superseded lock Play.
 pub const SUPERSEDED_LOCK_PLAY_ID: &str = "54159daf-f37d-47f9-a337-bf644d1f0db2";
 
+/// Node id of the task spec Play (ADR-097 §2, §6).
+pub const TASK_SPEC_PLAY_ID: &str = "c4a8e1b6-2f73-4d95-a0b8-7e5d3c91f2a4";
+
 /// The id of every Play that ships with the product: the core play table, in
 /// the order the plays are seeded.
 pub const CORE_PLAY_IDS: &[&str] = &[
@@ -51,6 +54,7 @@ pub const CORE_PLAY_IDS: &[&str] = &[
     TASK_BLOCKERS_PLAY_ID,
     TASK_CRITERIA_PLAY_ID,
     SUPERSEDED_LOCK_PLAY_ID,
+    TASK_SPEC_PLAY_ID,
 ];
 
 /// When a task's parent is finished by its sub-tasks (ADR-079 §8). Two
@@ -368,6 +372,57 @@ pub fn task_lineage_rules() -> serde_json::Value {
     )])
 }
 
+/// Refuse to start a task that is not governed by an approved spec.
+///
+/// The lineage rule above only looks at a task that has a plan, so a task
+/// with a draft spec and no plan, or no spec at all, would start without
+/// objection. This rule closes that: moving to `in_progress` needs a linked
+/// spec whose `spec_status` is `approved`, unless the task is marked
+/// `requires_spec: false`, the light lane for chores and small fixes
+/// (ADR-097 §6).
+///
+/// Only `in_progress` is guarded. That is the move that claims the work, and
+/// the question the gate asks of a session's task through the dry run
+/// (ADR-097 §2); a task already started is not re-checked when it later moves
+/// to review or done. A task with no `requires_spec` value reads as requiring
+/// one, the schema default, so removing the value never opens the lane.
+///
+/// Both reads are `has()`-guarded: a spec that has no `spec_status` is not
+/// approved, and a missing key would otherwise fail the condition and allow.
+pub fn task_spec_rules() -> serde_json::Value {
+    json!([reject_change(
+        "reject-starting-a-task-without-an-approved-spec",
+        "Refuse to start a task that links no approved spec, unless it is marked as not \
+         needing one",
+        "task",
+        "status",
+        json!([
+            {
+                "expr": "node.status == 'in_progress'",
+                "description": "The task is being started",
+            },
+            {
+                "expr": "!has(node.requires_spec) || node.requires_spec == true",
+                "description": "The task is not marked as not needing a spec",
+            },
+            {
+                "expr":
+                    "!has(node.spec) \
+                     || !node.spec.exists(s, has(s.spec_status) && s.spec_status == 'approved')",
+                "description": "The task links no approved spec",
+            },
+        ]),
+        "Refuse the change and say the task needs an approved spec, or the light lane",
+        "This task was not started: it is not linked to an approved spec, and nothing was \
+         changed, so do not retry the same change. Follow the Writing a Spec skill: write the \
+         spec as a draft and ask the user to approve it, then link it with `nodespace \
+         relationship create --from <spec-id> --type tasks --to <task-id>` and start the task \
+         again. If the user confirms this is a chore or a small fix that needs no spec, mark \
+         the task with `nodespace node update <task-id> --property requires_spec=false` \
+         instead.",
+    )])
+}
+
 /// Refuse to start a task while something blocking it is unfinished.
 ///
 /// Applies to every task, with or without a plan or spec. Uses the
@@ -571,6 +626,14 @@ pub fn core_play_templates() -> Vec<NodeTemplate> {
              referenced the old one still reads what it was built against.",
             superseded_lock_rules(),
         ),
+        seeded_play(
+            TASK_SPEC_PLAY_ID,
+            "Require an approved spec before starting a task",
+            "Rejects moving a task to in_progress unless it is linked to an approved spec. A \
+             task marked as not requiring a spec (the light lane, for chores and small fixes) \
+             is never checked. Applies to every task, with or without a plan.",
+            task_spec_rules(),
+        ),
     ]
 }
 
@@ -611,6 +674,7 @@ mod tests {
             // Two fields and a status for a spec and for a plan, and a
             // status for a decision.
             (superseded_lock_rules(), 7),
+            (task_spec_rules(), 1),
         ] {
             let props = json!({ "play": { "rules": rules } });
             let rules = parse_rules_from_properties(&props).expect("shipped rules must parse");
@@ -724,6 +788,32 @@ mod tests {
         }
     }
 
+    /// The spec rule guards only the move that claims the work, reads the
+    /// light lane field, and tells the agent what to do next (ADR-097 §8).
+    #[test]
+    fn the_spec_rule_guards_starting_reads_the_lane_and_names_the_next_step() {
+        let rules = task_spec_rules();
+        let rule = &rules[0];
+        let conditions = rule["conditions"].as_array().unwrap();
+        let exprs: Vec<&str> = conditions
+            .iter()
+            .map(|c| c["expr"].as_str().unwrap())
+            .collect();
+        assert!(exprs[0].contains("'in_progress'"));
+        assert!(!exprs[0].contains("'in_review'"), "{}", exprs[0]);
+        assert!(exprs[1].contains("requires_spec"));
+        assert!(exprs[2].contains("'approved'"));
+
+        let message = rule["actions"][0]["params"]["message"].as_str().unwrap();
+        assert!(message.contains("Writing a Spec"), "names the skill");
+        assert!(message.contains("nodespace "), "names a nodespace command");
+        assert!(message.contains("do not retry"), "says not to retry");
+        assert!(
+            message.contains("requires_spec=false"),
+            "names the light lane"
+        );
+    }
+
     /// Every core play ships a description on each rule, condition and action
     /// (ADR-090 §1), in its live rules and in its reset target alike.
     #[test]
@@ -762,6 +852,7 @@ mod tests {
                 "d9ea23d8-9e20-4cfa-9abe-6c86f2164765",
                 "95796b4d-f078-44f8-807b-8a2e92167a2d",
                 "54159daf-f37d-47f9-a337-bf644d1f0db2",
+                "c4a8e1b6-2f73-4d95-a0b8-7e5d3c91f2a4",
             ]
         );
         let unique: std::collections::HashSet<_> = CORE_PLAY_IDS.iter().collect();

packages/core/src/services/query_service/core_queries.rs

@@ -116,17 +116,22 @@ fn by_priority_then_age() -> Value {
 /// checkbox child, no unfinished `blocked_by` task and no plan that is not
 /// approved.
 ///
-/// The query states the conditions the task Plays enforce, until a filter can
-/// ask whether a change would be allowed (ADR-094 §9). Each of the three
+/// A task the Plays would refuse to start is not ready: the last filter asks
+/// whether the change to `in_progress` would be allowed (ADR-094 §9), so a
+/// task with no approved spec that is not marked `requires_spec: false`
+/// (ADR-097 §6) is left out however its checklist looks. The conditions
+/// before it are statements, not a second rule: a checklist is not a Play's
+/// concern, and the blocker and plan conditions are kept as cheap
+/// prefilters that bound the dry runs, one per candidate. Switching off a
+/// gate Play therefore loosens the dry run and not these. Each of the three
 /// relationship conditions is a negation or an existence over the nodes a
 /// path reaches:
 ///
 /// - A checkbox is the only type that derives `checked`, and on any other
 ///   child it reads as absent, so a child whose `checked` exists is a
 ///   checkbox, ticked or not. Only direct children count.
 /// - "No unfinished blocker" keeps a task with no blocker at all.
-/// - "No plan that is not approved" keeps a task with no plan, which is what
-///   lets a small change with a checklist of its own skip the spec and plan.
+/// - "No plan that is not approved" keeps a task with no plan.
 pub fn ready_tasks_filters() -> Value {
     json!([
         { "type": "property", "operator": "equals", "property": "status", "value": "open" },
@@ -147,7 +152,8 @@ pub fn ready_tasks_filters() -> Value {
                 "type": "property", "operator": "equals", "property": "plan_status",
                 "value": "approved", "negate": true
             }
-        }
+        },
+        { "type": "permitted", "operator": "equals", "property": "status", "value": "in_progress" }
     ])
 }