[malibio]

Summary

Refs #3784 (one of its five scenarios; the issue stays open).

When Creating a Task is a candidate and the model makes a top-level create_node, the system searches decision nodes for the title before the write runs and returns what it finds as the tool result. The retry of the same title goes through, so the model keeps the judgement. A turn that ends with the hold unresolved asks the user, naming the decision and offering to leave it or reopen it (declined_work_backstop, the sibling of the duplicate-entity backstop).

Why

The procedure's second step is "was it declined before". On E4B the model skips it, the decision is in no prompt, and a forced read the model chose itself searched open tasks instead. Rendering the procedure first and offering it alone were both measured and changed nothing. Findings are on the issue.

Verified

• Declined request, E4B, production stack in-process: before, one bare create_node; now no node is created and the user is asked.
• "Add a task to call the roofer on Friday": created, not held.
• "Add a task to build a REST API" against a GraphQL decision: asks. A false positive by design: the search has no relevance bar and the user can say go ahead.
• 363 agent_loop unit tests pass, 5 new. test:changed passes.

Not covered

The other four eval:tasks scenarios (routing: the lane's procedure, the lookup path dropping it) and the confusion matrix / eval:agent / eval:tasks runs. This change is a create-time guard and moves no retrieval rank.

🤖 Generated with Claude Code

[malibio]

Code Review Summary

Verdict: REQUEST CHANGES (one blocker). The design is sound and small: system-side check keyed to a procedure candidate, a held create returned as is_error: true (correct, so it is not replayed as a write), and a turn-end backstop that only fires when nothing else resolved the hold. AGENTS.md checks are clean: no node_type == comparisons, no lifecycle_status reads, no issue numbers in comments, no sync wording. But the advertised retry path appears to be unreachable in the real loop.

Findings

Critical Issues

• agent_loop.rs (~4706-4722, the per-turn seen_calls duplicate-call guard vs. declined_work_refused_result): the refusal message tells the model to "call create_node again with the same arguments". seen_calls is populated before execution (line ~4722), including the held call. When the model retries with identical args and that is the only call in the iteration, all_duplicate is true and the loop breaks ("Duplicate tool-call loop detected"), so the retry never executes decisions_that_may_settle at all. The result is that "none of them applies" can never create the task; the turn ends with no write and the backstop then asks the user about an unrelated decision. The unit test a_top_level_create_under_creating_a_task_is_held_for_its_decisions_once only exercises the helper in isolation with checked, so it cannot see this. Fix options: remove the held call's key from seen_calls when it is held back (it did not execute), or tell the model to change nothing and exempt via checked-aware dedupe. Then add a loop-level test (mock engine: create_node, same create_node, assert the write executes). Please verify this before merge; if I misread the guard's scope, the loop-level test will show it.

Suggested Improvements

• False-positive cost: the search has no relevance bar, so any decision node in the pool of 50 is returned as "may already settle this", and the backstop names rows.first() in a user-facing question. A workspace with any decisions will hold most task creates once, and a failed retry (see above) will ask about an unrelated decision. Either gate on the score (the search result carries one) or make the backstop wording hedge less ("may" is there, but the options say "Leave X as decided"). Measure with an unrelated-decision case.
• No node_type check on the held create: any top-level create_node under the Creating a Task candidate (a text note, a person) triggers the search. Gate on the task type through the registry (type_is_a), not a literal.
• "node_types": ["decision"] is an exact-type filter; confirm it matches subtypes of decision, otherwise a subtype decision is invisible. If it does not, resolve through the registry.
• Keying on CREATING_A_TASK_SKILL_ID inside the loop is acceptable for now (the same constant is in skill_pipeline), but it hard-codes one procedure into the generic loop. A settles_against_decisions flag on the procedure definition would keep the loop skill-agnostic. Not a blocker.
• parent_id exemption is reasonable, but a task created with a parent_id of a project/collection is also skipped, so a declined request that lands under a parent is not checked. State this in the doc comment.
• Backstop can replace a good reply: the "reply already names the decision" test is a case-insensitive substring match of the first decision's title only. A reply that paraphrases it, or names decision Create design system foundation #2 or Desktop shell with resizable panels #3, is replaced by the composed question. Check all returned decisions (titles and ids), not just the first.
• Search failure is silently fail-open (.ok()?); log at debug so a broken search is diagnosable.

Tests

• Add: loop-level retry test (above); backstop ordering with duplicate_entity_backstop (clarify already set means this one skips, which is right but untested) and cut_off_turn_backstop; a decision row with empty title/id; a route_clarify after the hold leaving clarify alone; search error/empty fail-open.
• The test fixture's result shape (id, title, type) is hand-written; assert it against the real search_semantic output shape or a captured sample.

Nitpicks

• Nit: DECLINED_WORK_ERROR is sound, but duplicate_entity_backstop uses is_duplicate_entity_refusal; a matching is_declined_work_hold helper would keep the two symmetrical.
• Nit: the doc comment for the pool records a measurement (3 vs 50) with no pointer to where it was recorded; reference the ADR/eval instead.

[malibio]

Review verdict: REQUEST CHANGES. One blocker (the held create's same-args retry is likely cut off by the per-turn duplicate-call guard); details in the comment above.

[malibio]

Review addressed

Fixed

• 🔴 The retry of a held create was unreachable: the duplicate-call guard registers a call before it runs, so the identical retry ended the loop. The held call's key is dropped from the seen set. New loop-level test (a_held_create_retried_with_the_same_arguments_runs); verified it fails with the fix removed.
• The backstop now treats a reply naming any returned decision, by title or id, as the user having been told.
• A failed decision search is logged at debug.
• is_declined_work_hold helper.

Skipped, with reasons

• Relevance bar / score gate: the search rows carry no usable score (score: 0.0), and the model is the judge of relevance; a wrong hold costs one question. Revisit if holds on unrelated decisions show up in practice.
• Node-type gate through the registry: the loop has no node-service handle, and the checks that matter (top-level, procedure leading) already bound it. A text note under the procedure being held once is harmless.
• Decision subtypes via node_types: ["decision"]: no decision subtype exists in the registry today; revisit when one does.
• Skill-id flag on the procedure definition: a node-type-sequence change for one consumer.
• parent_id exemption: documented on the function; it is what keeps the brief and checklist from being checked.
• Extra tests (ordering against the other backstops, empty rows, search error): the paths are single early returns covered by the existing ones.

test:changed passes; 364 agent_loop tests pass.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: One small, test-covered fix to the single blocker (confirmed by a test that fails without it), plus three low-risk tidy-ups. No new behaviour.