fix(agent-team): align Confluence node with merged #61 lessons + post-review hardening
Apply review findings from mining PR #61/#63/#64 and the recent merged PRs: - conf_draft_node: drop the agentic invoker config (max_turns=8 + Read/Grep/Glob + budget) that merged #61 reverted live (#60); use max_turns=4 tools-off like the planner. The draft is a reasoning->JSON node — repo context is folded into the prompt, never fetched via Claude tools (feedback_claude_sdk_single_shot). Invert the test that pinned the old tools-on contract. - confluence_writer: record the dedicated Phase.CONF_DRAFT/CONF_GATE/CONF_WRITE values instead of borrowing VERIFY/REVIEW/BUILD, so the ledger/transitions/ dashboard show the real lane. - invoker: note the allowed_tools seam is replicated from merged #61 (drop on a future rebase; byte-equivalent default None->[]). - dashboard: document that conf_* vertices need no _NODE_TO_STAGE remap (node id == stage key, identity fallback resolves them). Verified (no change needed): coordinator confluence gate shares the plan gate's crash isolation (_emit never raises, ledger write fail-soft); confluence_* state channels persist via declared PipelineState channels + checkpointer; Flow B edge hooks VERIFY's approved terminus, orthogonal to #61's plan-gate-approve routing. Full suite 1564 passed; ruff + format clean.
This commit is contained in:
parent
c7f9c1bac2
commit
e9bf016035
3 changed files with 52 additions and 46 deletions
|
|
@ -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"}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Reference in a new issue