diff --git a/agent-team/agent_team/dashboard.py b/agent-team/agent_team/dashboard.py index 69f5711..2fd5179 100644 --- a/agent-team/agent_team/dashboard.py +++ b/agent-team/agent_team/dashboard.py @@ -55,7 +55,11 @@ _WAITING_STATUSES = frozenset({"waiting_human"}) _PARKED_STATUSES = frozenset({"parked", "failed"}) # Map a topology node id back to the budget_ledger ``stage`` key for cost join. -# Most node ids equal their stage; the P3 build/verify vertices differ. +# Most node ids equal their stage; the P3 build/verify vertices differ. The +# Confluence vertices (conf_draft / conf_gate / conf_write) need NO entry — their +# node id already equals both their Phase value and their ledger stage key, so the +# identity fallback (``_NODE_TO_STAGE.get(node_id, node_id)``) resolves them and +# they render on the timeline like any other stage. _NODE_TO_STAGE = {"build_node": "build", "verify_node": "verify"} diff --git a/agent-team/agent_team/nodes/confluence_writer.py b/agent-team/agent_team/nodes/confluence_writer.py index 688ff6c..e63d7da 100644 --- a/agent-team/agent_team/nodes/confluence_writer.py +++ b/agent-team/agent_team/nodes/confluence_writer.py @@ -94,24 +94,30 @@ except Exception: # noqa: BLE001 - graph may not define it yet on this branch # always terminates. MAX_CONFLUENCE_GATE_VISITS = 3 -# Per-call turn headroom + budget for the AGENTIC draft call. The subscription -# invoker defaults to single-shot (max_turns=1), which an agentic repo-inspecting -# call exhausts before it can finish ("Reached maximum number of turns (1)"; PR -# #61 / issue #60). This call READS the repo to write accurate docs, so it needs -# real turns and a READ-ONLY toolset — it emits the draft as DATA and writes -# nothing, so no Edit/Write/Bash. -_DRAFT_MAX_TURNS = 8 -_DRAFT_ALLOWED_TOOLS: tuple[str, ...] = ("Read", "Grep", "Glob") -_DRAFT_BUDGET_USD = 4.0 +# Per-call turn headroom for the draft call. This is a reasoning->JSON node (it +# turns the task/plan/diff already in state into a documentation draft), so it +# follows the SAME shape as the planner/clarifier: a few turns, NO tools. The +# subscription invoker's single-shot default (max_turns=1) crashes the reasoning +# nodes ("Reached maximum number of turns (1)"), so the planner uses max_turns=4; +# we match it. +# +# Deliberately NOT agentic + NOT tool-using: the merged #60 fix (PR #61) proved +# live that giving a generative node read-only tools + a higher turn cap just +# moves the failure — the tool session exhausts the cap reading files before it +# emits output, or returns narration instead of the artifact (observed at +# max_turns=8). The lesson (memory feedback_claude_sdk_single_shot): reasoning +# nodes run tools-off; any repo context the draft needs is gathered +# deterministically INTO the prompt (build_confluence_prompt folds in plan + +# candidate_diff), never via Claude tool calls. +_DRAFT_MAX_TURNS = 4 -# Phase .value strings this stage routes on. The graph wires vertices that read -# these; kept as Phase members so a future dedicated Phase enum value is a -# one-line swap. Until the foundation adds CONF_* members the stage reuses the -# closest existing phases (VERIFY/BUILD/DONE/PARKED) for routing — the graph-agent -# repoints these if dedicated phases are introduced. -CONF_DRAFT_PHASE = Phase.VERIFY.value -CONF_GATE_PHASE = Phase.REVIEW.value -CONF_WRITE_PHASE = Phase.BUILD.value +# Phase .value strings this stage routes on. Now that task_model defines dedicated +# CONF_* phases, the stage records them directly (durable current_phase reflects +# the real lane in the ledger / transitions / dashboard instead of borrowing +# VERIFY/REVIEW/BUILD). +CONF_DRAFT_PHASE = Phase.CONF_DRAFT.value +CONF_GATE_PHASE = Phase.CONF_GATE.value +CONF_WRITE_PHASE = Phase.CONF_WRITE.value CONF_DONE_PHASE = Phase.DONE.value # Disjoint namespace base for the Confluence gate's stable ``turn`` derivation. @@ -235,17 +241,17 @@ def _conf_approval_question_set( def conf_draft_node( state: PipelineState, config: dict[str, Any] | None = None ) -> PipelineState: - """CONF_DRAFT stage: agentic repo inspection -> Confluence draft (as DATA). + """CONF_DRAFT stage: reasoning over in-state context -> Confluence draft (as DATA). Builds the draft prompt (Flow A from ``state['task']``; Flow B folding in ``plan`` + ``candidate_diff`` + repo name; ``confluence_feedback`` folded in - on a ``request_changes`` loop-back), then makes ONE **agentic** Claude call - that may READ the repository to write accurate docs. The call is pinned to a - READ-ONLY toolset (``Read``/``Grep``/``Glob``), ``max_turns=8``, and a - ``budget_usd`` cap — the agentic-config rationale builders use (PR #61 / - issue #60: single-shot defaults die with "Reached maximum number of turns - (1)"). The node emits the draft as DATA and writes NOTHING to the repo, so no - Edit/Write/Bash tool is granted. + on a ``request_changes`` loop-back), then makes ONE single-shot, **tool-less** + Claude call (``max_turns=4``) that turns that context into a documentation + draft. This is a reasoning->JSON node like the planner: it does NOT read the + repository agentically — the merged #60 fix (PR #61) proved live that handing + a generative node read-only tools just exhausts the turn cap or returns + narration. Any repo context the draft needs is folded into the prompt by + :func:`build_confluence_prompt`, never fetched via tools. Returns a **partial** :class:`PipelineState`: the parsed ``confluence_draft`` plus the phase advanced to the Confluence gate and ``status`` ACTIVE. @@ -259,8 +265,6 @@ def conf_draft_node( prompt, config=config, max_turns=_DRAFT_MAX_TURNS, - allowed_tools=list(_DRAFT_ALLOWED_TOOLS), - budget_usd=_DRAFT_BUDGET_USD, ) draft = parse_confluence_reply(result.text) diff --git a/agent-team/tests/test_confluence_writer.py b/agent-team/tests/test_confluence_writer.py index 49c6407..f448ae4 100644 --- a/agent-team/tests/test_confluence_writer.py +++ b/agent-team/tests/test_confluence_writer.py @@ -2,8 +2,10 @@ Covers the three LangGraph nodes plus the conditional-edge router: -* :func:`conf_draft_node` — the AGENTIC draft call. Pins the PR #61 contract: - ``max_turns > 1`` and a READ-ONLY toolset (Read/Grep/Glob, NO Edit/Write/Bash). +* :func:`conf_draft_node` — the reasoning->JSON draft call. Pins the post-#60 + contract: a small turn headroom (``max_turns == 4``, like the planner) and + TOOLS-OFF (no allowed_tools / budget) — the high-cap + read-only-tools approach + was reverted live in PR #61 (feedback_claude_sdk_single_shot). * :func:`conf_gate_node` — the resumable human gate. Driven through a real in-memory LangGraph (interrupt + ``Command(resume=...)``) to assert the interrupt payload ``kind`` and the approve / request_changes / abandon / @@ -204,28 +206,24 @@ def test_conf_draft_node_builds_draft_and_advances_to_gate() -> None: assert out["confluence_draft"]["page_id"] == "1540098" -def test_conf_draft_node_passes_agentic_max_turns() -> None: - # PR #61: single-shot defaults die with "Reached maximum number of turns (1)"; - # the agentic draft call must request real turn headroom (> 1). +def test_conf_draft_node_passes_reasoning_turn_headroom() -> None: + # The single-shot default (max_turns=1) crashes reasoning nodes; the draft + # asks for the same small headroom the planner uses (4) — NOT a high agentic + # cap (the merged #60 fix / PR #61 reverted the high-cap + tools approach). calls = _bind_invoker(json.dumps(_VALID_DRAFT)) conf_draft_node(_state(task="x")) - assert calls[0]["kw"].get("max_turns", 1) > 1 + assert calls[0]["kw"].get("max_turns", 1) == 4 -def test_conf_draft_node_grants_only_read_only_tools() -> None: - # The draft emits DATA and writes nothing, so it gets a READ-ONLY toolset: - # Read/Grep/Glob present, and NO Edit/Write/Bash (the load-bearing pin). +def test_conf_draft_node_runs_tools_off() -> None: + # Load-bearing pin (memory feedback_claude_sdk_single_shot): the draft is a + # reasoning->JSON node and must run TOOLS-OFF. Giving it repo tools was proven + # live to exhaust the turn cap / return narration (#60). Any repo context goes + # INTO the prompt, never via Claude tools — so no allowed_tools, no budget. calls = _bind_invoker(json.dumps(_VALID_DRAFT)) conf_draft_node(_state(task="x")) - tools = set(calls[0]["kw"].get("allowed_tools") or []) - assert {"Read", "Grep", "Glob"} <= tools - assert tools.isdisjoint({"Edit", "Write", "Bash", "MultiEdit", "NotebookEdit"}) - - -def test_conf_draft_node_passes_budget_cap() -> None: - calls = _bind_invoker(json.dumps(_VALID_DRAFT)) - conf_draft_node(_state(task="x")) - assert isinstance(calls[0]["kw"].get("budget_usd"), (int, float)) + assert not calls[0]["kw"].get("allowed_tools") + assert "budget_usd" not in calls[0]["kw"] def test_conf_draft_node_forwards_config_to_billing_seam() -> None: