From 0612493cc06d1243e86b08e55ab65f43a7e621ea Mon Sep 17 00:00:00 2001 From: sprooty Date: Wed, 5 Aug 2026 04:35:38 +0000 Subject: [PATCH] feat: point the harness at a project, and let an item produce an answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps found by a run that delivered six items of worthless work. ## survey: the first run can generate the plan (#181) The harness could be told exactly one thing — here is a PLAN.md, execute it. A user may instead want to say "review and generate a plan to upgrade to Node v22" and have the first run produce the plan, in the form the harness consumes. Mostly assembly rather than invention, because the deliverable is a file the parser already understands: - **The gate is `parse_plan`**, the same reader a hand-written plan goes through. A generated plan the queue would read differently from how it looks is caught here rather than three commands later, which beats a model's opinion of its own output. - **The output is a PLAN.md, never queue rows.** Writing to the queue would fork the pipeline into a generated path and a hand-written one that diverge forever. - **Nothing external happens.** No issues, branches or rows; without `--out` it prints and writes nothing. What is new is that it *reads the project*: documents named with `--doc`, or a short list of guesses, plus the tracked tree and recent history. A named document that is missing is reported rather than skipped silently — the failure this exists to prevent is a confident plan built without the file that states the project's direction. That is exactly what happened: a real repository kept its roadmap in two documents nothing in the harness had ever opened, so seven items were hand-written instead, all real, all delivered, none of them the work that mattered. ## deliverable: an item can produce an answer instead of a diff (#182) Every outcome path assumed a diff. An item like "compare these three approaches" left a clean worktree, hit the clean-tree path, and was recorded `escalated / no_target` — a finished investigation was indistinguishable from an agent that did nothing. Writing the answer to a file instead met the rest of the pipeline: the checks ran against a document, and the reviewer graded it with the diff rubric. An item now declares what it produces. `deliverable: findings` tells the agent to write its answer to `.harness-findings.md` and change nothing else; the answer becomes the item's result and the file is never committed. `code` is the default and behaves exactly as before. **The plan declares this; the agent never chooses it.** Otherwise the first hard test failure becomes an essay about why the test was wrong. A findings item can still refuse — "this question cannot be answered from this repository" is an escalation whatever it was asked to produce — and one that answers nothing lands in the clean-tree path, which is where an agent that did nothing belongs. ## Three defects the tests caught, not the reasoning **Phase headings were read back as work items.** `## P0 Upgrade` matches the item pattern, so every generated plan carried a phantom item whose brief was the phase's *rationale*. `render_plan` gained `phases_as_items`; `inception`'s default is unchanged, because that behaviour is deliberate and documented there and reversing it on one run's evidence from a different context is not mine to do. #184 records what would settle it. **A refusal on a findings item left the answer in the tree**, which then read as an ordinary change and ran the whole code path over it. Both harness notes are now taken unconditionally. **`deliverable:` never parsed.** `_META` whitelists keys, so the handler was unreachable and the value silently defaulted. Closes #181, closes #182. Co-Authored-By: Claude Opus 5 (1M context) --- docs/USAGE.md | 71 ++++++ src/agent_harness/__main__.py | 115 ++++++++++ src/agent_harness/adoption.py | 8 +- src/agent_harness/inception.py | 21 +- src/agent_harness/plan.py | 29 ++- src/agent_harness/session_executor.py | 97 ++++++++- src/agent_harness/survey.py | 296 ++++++++++++++++++++++++++ src/agent_harness/work.py | 18 +- tests/test_plan.py | 32 +++ tests/test_session_executor.py | 127 +++++++++++ tests/test_survey.py | 215 +++++++++++++++++++ 11 files changed, 1014 insertions(+), 15 deletions(-) create mode 100644 src/agent_harness/survey.py create mode 100644 tests/test_survey.py diff --git a/docs/USAGE.md b/docs/USAGE.md index c4dc7eb..9a612d1 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -324,6 +324,77 @@ and sync it, exactly as though you had written it by hand. --- +## 0bb. Or point it at a project and state an objective + +`inception` scopes a **new** project from prose. `survey` does the same for one +that already exists: you say what you want done, and the first run works out +what the items should be instead of being handed them. + +```console +$ agent-harness survey "review and generate a plan to upgrade to Node v22" \ + --work ./service --doc docs/roadmap.md \ + --surveyor claude-sonnet-4-6 --endpoint $HARNESS_ENDPOINT --out PLAN.md +read 3 source(s): docs/roadmap.md, 412 tracked path(s), recent history +9 work item(s), 4 heading(s) skipped as narrative +blocking question: is the native addon in vendor/ still maintained upstream? +wrote PLAN.md +Review it, then: agent-harness adopt PLAN.md --project NAME --work ./service +``` + +**Name your roadmap with `--doc`.** Without it the harness guesses from a short +list (`docs/current-state.md`, `ROADMAP.md`, `README.md`, …) and stops at two. +A named file that is missing is *reported*, not skipped — the failure this +command exists to prevent is a confident plan built without the document that +states the project's direction. + +**The gate is the harness's own parser.** The generated plan is read back by +`parse_plan`, the same function a hand-written plan goes through, so a plan the +queue would read differently from how it looks is caught here rather than three +commands later. If it cannot be read — no items, or duplicate ids, which cannot +each become one issue — nothing is written. `--force` overrides that, and means +executing a plan the harness has told you it does not understand. + +**Nothing external happens.** No queue rows, no issues, no branches. Without +`--out` it prints the plan and writes nothing at all, which is the right +default for output whose entire purpose is to be argued with. + +Blocking questions are reported and do **not** stop the file being written. +They are questions for you, and your answer decides — the same rule +`inception` applies at its approval gate. + +### 0bb.1 Items that produce an answer rather than a change + +Some work has no diff. "Compare these three approaches", "is this feasible", +"which of these is the cause" — the answer *is* the deliverable, and an item +like that used to leave a clean worktree and be recorded as +`escalated / no_target`: indistinguishable from an agent that did nothing. + +An item can now say what it produces: + +```markdown +### T1 — Choose the Tailnet attachment architecture + +Compare a host-managed daemon, a managed sidecar, and a tsnet bridge. Say +which fits this deployment and what rules the others out. + +deliverable: findings +``` + +`deliverable: code` is the default and means a diff, judged exactly as before. +`deliverable: findings` tells the agent to write its answer to +`.harness-findings.md` and change nothing else. The answer becomes the item's +result, the item completes, and the file is never committed. + +**The plan declares this; the agent never chooses it.** Otherwise the first +hard test failure becomes an essay about why the test was wrong. + +A findings item can still refuse — "this question cannot be answered from this +repository" is an escalation whatever the item was asked to produce — and one +that answers nothing at all lands in the same clean-tree path as any other +agent that did nothing, which is where it belongs. + +--- + ## 0c. Or adopt a project that is already half-built The common case is not a blank repository. It is a plan, a repository, some diff --git a/src/agent_harness/__main__.py b/src/agent_harness/__main__.py index 72b24dd..fe59c06 100644 --- a/src/agent_harness/__main__.py +++ b/src/agent_harness/__main__.py @@ -505,6 +505,7 @@ def _run(args: argparse.Namespace) -> int: title=i.title, brief=i.brief(), depends_on=i.depends_on, + deliverable=i.deliverable, project_id=args.project, ) for i in plan.deduplicated() @@ -859,6 +860,71 @@ def ask(prompt: str) -> str: return ModelAssessor(ask) +def _survey(args: argparse.Namespace) -> int: + """Read an existing project and propose a plan for a stated objective. + + Writes nothing unless `--out` is given, and refuses to write a plan the + harness's own parser could not read. The parser is the gate on purpose: a + generated plan that the queue reads differently from how it looks is worse + than no plan, and catching it here costs one function call. + """ + from .executor import _text_of + from .model_client import ModelClient, Route + from .survey import SURVEYOR, survey + + if not (args.work / ".git").exists(): + print(f"{args.work} is not a git repository", file=sys.stderr) + return 2 + if not args.surveyor: + print( + "--surveyor MODEL is required: this command's whole job is one model call", + file=sys.stderr, + ) + return 2 + if not args.endpoint: + print("--endpoint or $HARNESS_ENDPOINT is required", file=sys.stderr) + return 2 + + api_key = os.environ.get("HARNESS_API_KEY", "") + client = ModelClient( + roles={ + SURVEYOR: Route(args.surveyor, args.endpoint, api_key=api_key, preset=args.preset), + }, + transport=_http_transport(api_key), + ) + + def ask(prompt: str) -> str: + return str(_text_of(client.call(SURVEYOR, [{"role": "user", "content": prompt}]).body)) + + report = survey( + args.objective, + args.work, + ask=ask, + docs=list(args.doc), + name=args.name, + now=time.time(), + ) + for line in report.lines(): + print(line) + + if not report.usable and not args.force: + print( + "refusing to write: the harness cannot read the plan it just generated. " + "Re-run, name the roadmap with --doc, or pass --force.", + file=sys.stderr, + ) + return 1 + if args.out is None: + print() + print(report.markdown) + print("(nothing was written; pass --out PATH to keep it)") + return 0 + args.out.write_text(report.markdown) + print(f"wrote {args.out}") + print(f"Review it, then: agent-harness adopt {args.out} --project NAME --work {args.work}") + return 0 + + def _adopt(args: argparse.Namespace) -> int: """Inspect an existing project, then reconcile only with explicit approval. @@ -1029,6 +1095,52 @@ def main(argv: list[str] | None = None) -> int: help="where `export` writes its JSON (default: stdout)", ) + p_survey = sub.add_parser( + "survey", help="read an existing project and propose a plan for an objective" + ) + p_survey.add_argument( + "objective", + help='what you want done, in prose: "review and generate a plan to upgrade to Node v22"', + ) + p_survey.add_argument( + "--work", type=Path, default=Path("."), help="the existing repository to read" + ) + p_survey.add_argument( + "--out", + type=Path, + default=None, + help="write the proposed plan here. Without this it is printed and nothing is " + "written, which is the safe default for a command whose whole output is a " + "proposal to argue with.", + ) + p_survey.add_argument( + "--doc", + action="append", + default=[], + metavar="PATH", + help="a document that states this project's direction, relative to the " + "repository. Repeatable. Naming them beats the guesses: a plan built without " + "the roadmap is confident and wrong, and a named file that is missing is " + "reported rather than skipped silently.", + ) + p_survey.add_argument("--name", default="Plan", help="title for the generated plan") + p_survey.add_argument("--surveyor", default="", help="model for the surveyor role") + p_survey.add_argument( + "--endpoint", + default=os.environ.get("HARNESS_ENDPOINT", ""), + help="model API base url (or $HARNESS_ENDPOINT)", + ) + p_survey.add_argument( + "--preset", default="", help="route preset, as for run: claw-bay, chat-completions, generic" + ) + p_survey.add_argument( + "--force", + action="store_true", + help="write the plan even when the harness's own parser could not read it. " + "The parser is the gate here; overriding it means executing a plan the " + "queue will read differently from how it looks.", + ) + p_adopt = sub.add_parser( "adopt", help="inspect an existing project and propose a reconciliation" ) @@ -1410,6 +1522,9 @@ def main(argv: list[str] | None = None) -> int: if args.command == "adopt": return _adopt(args) + if args.command == "survey": + return _survey(args) + store = EventStore(args.db) if args.command == "ingest": diff --git a/src/agent_harness/adoption.py b/src/agent_harness/adoption.py index b94d028..351719e 100644 --- a/src/agent_harness/adoption.py +++ b/src/agent_harness/adoption.py @@ -52,7 +52,7 @@ from .executor import Checks from .github import MARKER, GitHub from .outcomes import ESCALATE, RETRY -from .plan import ParsedPlan, WorkItem +from .plan import CODE, ParsedPlan, WorkItem from .work import CLAIMED, DONE, PENDING, Project, WorkQueue, WorkRecord, revives #: The lifecycle of one adoption (proposal §5.1). `rejected` and `revise` are @@ -195,6 +195,10 @@ class AdoptionItem: #: The brief the queue is holding, so the report can tell a re-sync that #: changes nothing from one that rewrites the item. queue_brief: str | None = None + #: What the plan says this item produces. Carried through rather than + #: defaulted, or a `deliverable: findings` item would be adopted as one + #: that must produce a diff (#182). + deliverable: str = CODE proposed_state: str = PENDING evidence: list[Evidence] = field(default_factory=list) candidates: list[ExternalCandidate] = field(default_factory=list) @@ -595,6 +599,7 @@ def _inspect_item( depends_on=list(item.depends_on), queue_state=existing.state if existing else None, queue_brief=existing.brief if existing else None, + deliverable=item.deliverable, candidates=list(candidates), ) @@ -868,6 +873,7 @@ def reconcile(self, project_id: str, *, dry_run: bool = False) -> AdoptionReport title=item.title, brief=item.brief, depends_on=item.depends_on, + deliverable=item.deliverable, state=DONE if item.item_id in approved else PENDING, ) for item in report.items diff --git a/src/agent_harness/inception.py b/src/agent_harness/inception.py index 03fb36b..f0b6a43 100644 --- a/src/agent_harness/inception.py +++ b/src/agent_harness/inception.py @@ -193,11 +193,22 @@ def parse_proposal(text: str, revision: int, now: float, feedback: str | None = ) -def render_plan(proposal: Proposal, name: str) -> str: +def render_plan(proposal: Proposal, name: str, *, phases_as_items: bool = True) -> str: """A proposal as a PLAN.md the existing parser can read. Headings use the `### T1 — Title` shape the parser recognises, so the generated plan goes through exactly the same path as a hand-written one. + + **`phases_as_items` decides whether a phase heading is itself work.** A + heading of `## P0 Upgrade` matches the parser's item pattern, because `P0` + is a well-formed id, so by default each phase becomes an item as well as a + container. That is deliberate for `inception` — real hand-written plans do + track phases as issues, and a generated plan should behave like one. + + It is wrong for a plan meant to be executed straight away. The phase item's + brief is the phase's *rationale* — "because we need the runtime current" — + which is not a specification, and an agent that claims it is being asked to + implement a reason. `survey` therefore passes False. """ out: list[str] = [f"# {name}", ""] if proposal.goal: @@ -231,7 +242,13 @@ def render_plan(proposal: Proposal, name: str) -> str: out += ["## Work", ""] for phase in proposal.phases: title = phase.get("title") or phase.get("id") or "Phase" - out += [f"## {phase.get('id', '')} {title}".strip(), ""] + if phases_as_items: + out += [f"## {phase.get('id', '')} {title}".strip(), ""] + else: + # "Phase" first: the word cannot start an id, so the heading stays + # readable and stops matching the item pattern. + marker = f"Phase {phase['id']} — " if phase.get("id") else "Phase — " + out += [f"## {marker}{title}".rstrip(), ""] if phase.get("why"): out += [str(phase["why"]), ""] for item in phase.get("items") or []: diff --git a/src/agent_harness/plan.py b/src/agent_harness/plan.py index e64ac40..cf34dbf 100644 --- a/src/agent_harness/plan.py +++ b/src/agent_harness/plan.py @@ -84,13 +84,24 @@ #: treating it as one would fill the backlog with the document's structure. ID = r"[A-Z][A-Z0-9]{0,7}-?\d{1,4}(?:\.\d{1,3})*" +#: What an item produces. `code` is a diff, judged by the checks and a reviewer +#: reading that diff. `findings` is an answer \u2014 a feasibility verdict, a +#: comparison, a recommendation \u2014 where there may be nothing to commit, and +#: where an empty worktree is the expected shape rather than a failure. +#: +#: Two kinds, not a taxonomy. A third would need a third review rubric and a +#: third definition of done, and neither exists yet. +CODE = "code" +FINDINGS = "findings" +DELIVERABLES = (CODE, FINDINGS) + _SEP = r"[:.)\s\u2010-\u2015-]+" _HEADING = re.compile(rf"^(#{{2,6}})\s+({ID}){_SEP}\s*(.+?)\s*$") _PLAIN_HEADING = re.compile(r"^(#{1,6})\s+(.+?)\s*$") _CHECKBOX = re.compile(rf"^\s*[-*]\s+\[( |x|X)\]\s+(?:({ID}){_SEP}\s*)?(.+?)\s*$") _TABLE_ROW = re.compile(rf"^\|\s*({ID})\s*\|(.+)$") _META = re.compile( - r"^\s*(labels?|milestone|phase|depends[ _-]?on|size|risk|verify)\s*:\s*(.+?)\s*$", + r"^\s*(labels?|milestone|phase|depends[ _-]?on|size|risk|verify|deliverable)\s*:\s*(.+?)\s*$", re.IGNORECASE, ) _TABLE_SEP = re.compile(r"^\|[\s:|-]+\|$") @@ -122,6 +133,14 @@ class WorkItem: verification: list[str] | None = None #: Line number in the source plan, so a reader can find it again. line: int = 0 + #: What this item produces, from `deliverable:`. `code` is the default and + #: means a diff. `findings` means the answer *is* the output — "is X + #: feasible", "compare these three approaches" — and there may be nothing + #: to commit at all. + #: + #: **The item declares this; the agent never chooses it.** Otherwise the + #: first hard test failure becomes an essay about why the test was wrong. + deliverable: str = CODE def brief(self) -> str: """What an agent is told to do. The title alone is rarely enough; @@ -465,6 +484,14 @@ def _apply_metadata(item: WorkItem) -> None: item.depends_on.extend(_split(value)) elif key in ("size", "risk"): item.labels.append(f"{key}:{value.lower()}") + elif key == "deliverable": + kind = value.strip().lower() + if kind not in DELIVERABLES: + raise ValueError( + f"item {item.id} deliverable must be one of {', '.join(DELIVERABLES)}, " + f"not {value.strip()!r}" + ) + item.deliverable = kind elif key == "verify": try: argv = json.loads(value) diff --git a/src/agent_harness/session_executor.py b/src/agent_harness/session_executor.py index 285c8ae..0d565d4 100644 --- a/src/agent_harness/session_executor.py +++ b/src/agent_harness/session_executor.py @@ -66,6 +66,7 @@ Stop, stop_for, ) +from .plan import FINDINGS from .reaper import DEFAULT_MAX_AGE_SECONDS, ReapReport, reap_abandoned_sessions from .session_host import Session, SessionHost from .work import ( @@ -139,6 +140,30 @@ what the item asks, do it and leave the file absent. """ +FINDINGS_PROMPT = """\ +## What this item produces + +**This item's deliverable is an answer, not a change.** Write it to +`{findings_file}` in this directory. Change nothing else — no implementation, +no tests, no refactoring on the way past. + +The answer is the work, so it is judged as work: cite what you actually read, +with paths and line numbers. If the item names alternatives, examine each one +and say what you found, not what you expect. A recommendation the evidence does +not support is worse than "I could not determine this". + +If you conclude the question itself cannot be answered from this repository, +that is a refusal — use `{refusal_file}` and say why. +""" + +#: Where an agent leaves an answer, for an item whose deliverable is findings. +#: +#: Separate from the refusal file on purpose. "Here is the answer you asked +#: for" and "this item cannot be done" are different outcomes — one is +#: `completed`, the other `escalated` — and a single file would make the +#: harness guess which it was holding. +FINDINGS_FILE = ".harness-findings.md" + #: Where a refusing agent leaves its reasoning. #: #: The rule above used to end at "stop and say so plainly", which told the @@ -158,23 +183,41 @@ #: and there is no second copy of it once the worktree is gone. REFUSAL_LIMIT = 4000 +#: How much of an answer reaches the outcome. Larger than a refusal's: an +#: investigation's whole product is its text, and truncating the finding is +#: throwing away the item's deliverable rather than trimming an explanation. +FINDINGS_LIMIT = 16_000 -def _take_refusal(tree: Path) -> str: - """The agent's refusal, removed from the tree as it is read. - Removed rather than ignored so that the file cannot be committed, cannot - show up in a diff a reviewer reads, and cannot make a genuinely empty - attempt look like a change. Returns an empty string if no note was left, - which is the ordinary case. +def _take_findings(tree: Path) -> str: + """The agent's answer, removed from the tree as it is read. + + Removed for the same reason a refusal is: the answer is the item's + result, recorded in the outcome, and leaving it in the worktree would + turn a question into a commit nobody asked for. """ - note = tree / REFUSAL_FILE + return _take(tree / FINDINGS_FILE, FINDINGS_LIMIT) + + +def _take(note: Path, limit: int) -> str: try: text = note.read_text(errors="replace").strip() except OSError: return "" finally: note.unlink(missing_ok=True) - return text[:REFUSAL_LIMIT] if text else "" + return text[:limit] if text else "" + + +def _take_refusal(tree: Path) -> str: + """The agent's refusal, removed from the tree as it is read. + + Removed rather than ignored so that the file cannot be committed, cannot + show up in a diff a reviewer reads, and cannot make a genuinely empty + attempt look like a change. Returns an empty string if no note was left, + which is the ordinary case. + """ + return _take(tree / REFUSAL_FILE, REFUSAL_LIMIT) #: The review rubric lives with the headless executor and is imported, not @@ -548,15 +591,25 @@ def _execute(self, record: WorkRecord) -> Outcome: self._emit(record, "stacked", detail=f"based on {base} ({stacked_on})") try: + findings_item = record.deliverable == FINDINGS prompt_file = tree / ".harness-prompt.md" prompt_file.write_text( PROMPT_TEMPLATE.format( title=record.title, brief=record.brief, - checks_description=self._describe_checks(), + checks_description=( + "Nothing is compiled or tested: this item changes no code." + if findings_item + else self._describe_checks() + ), prior=self._prior_failure(record), refusal_file=REFUSAL_FILE, ) + + ( + FINDINGS_PROMPT.format(findings_file=FINDINGS_FILE, refusal_file=REFUSAL_FILE) + if findings_item + else "" + ) ) session = self.devenv.create_session( @@ -621,7 +674,33 @@ def _execute(self, record: WorkRecord) -> Outcome: # Read and remove before the tree is inspected: a refusal note is # the agent's answer *about* the item, never a change to it, and # leaving it would make a refusing agent look like a working one. + # Both are the agent's answer *about* the item rather than a change + # to it, and both are taken unconditionally — a note left behind is + # a note that reaches a commit, a diff and a reviewer. Taking the + # findings file only when it was going to be used left it in the + # tree whenever a refusal won, which then read as an ordinary + # change and ran the whole code path over it. refusal = _take_refusal(tree) + findings = _take_findings(tree) + # A findings item is finished when it has an answer. Its worktree is + # *expected* to be clean, so it must be decided before the clean-tree + # path below, which would otherwise escalate every successful + # investigation as "the agent made no changes" (#182). + # + # A refusal still wins: "this question cannot be answered from this + # repository" is an escalation whatever the item was asked to + # produce. + # + # Falling through means asked for an answer and given nothing, and + # not told why. That is "an agent that did nothing", which the + # clean-tree path below already handles correctly. + if findings_item and not refusal and findings: + outcome.stages.append("findings") + outcome.reason = findings + self._emit(record, "findings", detail=findings, session_id=session.id) + outcome.state = DONE + outcome.stop = Stop(COMPLETED, detail=findings) + return outcome diff = run_git(tree, "diff", "HEAD") if not diff.strip() and not run_git(tree, "status", "--porcelain").strip(): # Nobody has judged this item and nobody can: the agent is diff --git a/src/agent_harness/survey.py b/src/agent_harness/survey.py new file mode 100644 index 0000000..329d627 --- /dev/null +++ b/src/agent_harness/survey.py @@ -0,0 +1,296 @@ +"""Producing a plan for a project that already exists. + +`inception` scopes a *new* project from a paragraph: describe it, argue with +the proposal, and on approval create the repository and the backlog. This is +the same idea pointed at a repository that is already there — the user states +an objective, and the first run of the harness works out what the items should +be rather than being handed them. + + "review and generate a plan to upgrade to Node v22" + -> read the project + -> propose a PLAN.md + -> validate it with the harness's own parser + -> a human approves, and `plan`/`adopt` execute it + +**The deliverable is a `PLAN.md`, never queue rows.** Writing straight to the +queue would fork the pipeline in two, a generated path and a hand-written one, +diverging forever. A document means the existing parse/sync/queue machinery +runs unchanged, the scope is diffable and reviewable, and a human can edit it +at any point. + +**The gate is the harness's own reader.** A generated plan is validated by +`parse_plan` — the same function that reads a hand-written one — so "the model +produced something the harness cannot consume" is caught here rather than +three commands later. That is a far better check than a model's opinion of its +own output, and it costs nothing. + +**It reads the project rather than guessing.** This module exists because a +run against a real repository produced seven items that were all real, all +delivered, and none of them the work that mattered — the project kept its +roadmap in two documents that nothing in the harness ever opened (#181). + +**Nothing external happens here.** No queue rows, no issues, no branches. The +output is a string and a report about it. +""" + +from __future__ import annotations + +import logging +import subprocess +from collections.abc import Callable +from dataclasses import dataclass, field +from pathlib import Path + +from .inception import Proposal, parse_proposal, render_plan + +log = logging.getLogger(__name__) + +#: The role that surveys an existing project. Separate from `scoper`, which +#: scopes greenfield from prose, and from `planner`, which picks target files +#: inside one item: all three want different models, and naming a role rather +#: than a model keeps that a configuration change. +SURVEYOR = "surveyor" + +#: Documents a project is likely to keep its direction in, tried in order when +#: the caller names none. Deliberately short: a wrong guess here costs context +#: budget and dilutes the evidence, and the caller can always be explicit. +ROADMAP_CANDIDATES = ( + "docs/current-state.md", + "docs/roadmap.md", + "docs/status.md", + "ROADMAP.md", + "STATUS.md", + "PLAN.md", + "README.md", +) + +#: How much of any one document reaches the prompt. A roadmap is the most +#: valuable evidence there is here, so this is generous — but one enormous file +#: must not crowd out every other source. +DOC_LIMIT = 40_000 + +SURVEY_PROMPT = """\ +You are producing a work plan for a project that already exists. You are not +writing code and not starting work. + +## The objective + +{objective} + +## The project + +{evidence} + +## What to produce + +Return JSON only, matching this shape exactly: + +{{ + "goal": "one paragraph restating the objective in your words, in terms of + this specific project", + "assumptions": ["things you are taking as given"], + "non_goals": ["what this explicitly does not cover"], + "risks": ["what could make this fail"], + "phases": [ + {{"id": "P0", "title": "...", "why": "...", + "items": [{{"id": "T1", "title": "...", "brief": "what to do and how it + will be judged", "depends_on": []}}]}} + ], + "open_questions": [ + {{"id": "Q1", "question": "...", "severity": "blocking|deferrable", + "why_it_matters": "what changes depending on the answer"}} + ] +}} + +## Rules + +- **Ground every item in this repository.** Name the file, module, command or + document you are working from. An item you cannot point at is a guess, and a + guess is worse than an open question. +- **Order the work.** An unordered list of forty items is barely better than + none. Use `depends_on`, and put the phases in the order they should happen. +- **Say what you could not determine.** If the project's own direction is + unclear from what you were shown, that is an open question, not something to + fill in. An invented constraint is indistinguishable from a decision the + human made. +- **Item briefs are specifications an agent works from alone**, with no access + to this conversation. State what is *out of scope* as explicitly as what is + in it, and make the verb match the criteria — a brief that says "mirror the + existing behaviour" licenses the opposite of criteria that say "change it". +- Ids must be unique across the whole plan. + +## How to judge severity + +`blocking` means the answer changes what gets built, so choosing wrong means +work is done and thrown away. `deferrable` means it is worth knowing but a +reasonable default holds. + +Be sparing with `blocking`. If everything is blocking, nothing is, and the +human answers carelessly to get past the gate. +""" + + +@dataclass +class Evidence: + """What the surveyor was shown, kept so a proposal can be argued with. + + Retained rather than discarded because the first question about any + generated plan is "what did it actually read?" — and on the run that + motivated this module the honest answer was "not the roadmap". + """ + + sources: list[str] = field(default_factory=list) + text: str = "" + + def render(self) -> str: + return self.text or "(nothing could be read from this repository)" + + +@dataclass +class SurveyReport: + """A generated plan, and what the harness's own parser made of it.""" + + markdown: str + proposal: Proposal + evidence: Evidence + item_count: int = 0 + skipped: int = 0 + duplicate_ids: list[str] = field(default_factory=list) + dependency_problems: list[str] = field(default_factory=list) + blocking_questions: list[str] = field(default_factory=list) + + @property + def usable(self) -> bool: + """Whether this plan can be executed as it stands. + + Unreadable or empty is fatal — there is nothing to run. Duplicate ids + are fatal too, because each id becomes one issue and one queue row. + A blocking question is *not* fatal here: it is a question for the + human, and it is their answer that decides, which is the same rule + `inception` applies at its approval gate. + """ + return self.item_count > 0 and not self.duplicate_ids + + def lines(self) -> list[str]: + out = [ + f"read {len(self.evidence.sources)} source(s): " + + (", ".join(self.evidence.sources) or "none"), + f"{self.item_count} work item(s), {self.skipped} heading(s) skipped as narrative", + ] + if self.duplicate_ids: + out.append( + "duplicate ids, which cannot become issues: " + ", ".join(self.duplicate_ids) + ) + for problem in self.dependency_problems: + out.append(f"dependency: {problem}") + for question in self.blocking_questions: + out.append(f"blocking question: {question}") + return out + + +def _read(repo: Path, relative: str) -> str | None: + path = repo / relative + try: + if not path.is_file(): + return None + return path.read_text(errors="replace")[:DOC_LIMIT] + except OSError: + return None + + +def _git(repo: Path, *args: str) -> str: + try: + result = subprocess.run( # noqa: S603 + ["git", "-C", str(repo), *args], # noqa: S607 + capture_output=True, + text=True, + timeout=30, + ) + except (OSError, subprocess.SubprocessError): # pragma: no cover - defensive + return "" + return result.stdout.strip() if result.returncode == 0 else "" + + +def gather(repo: Path, docs: list[str] | None = None) -> Evidence: + """Read what the project says about itself. + + Named documents win over guessed ones, and a named document that does not + exist is reported rather than skipped silently — asking for a roadmap and + getting a plan built without it is the failure this whole module is for. + """ + sources: list[str] = [] + parts: list[str] = [] + + wanted = list(docs) if docs else list(ROADMAP_CANDIDATES) + found = 0 + for relative in wanted: + text = _read(repo, relative) + if text is None: + if docs: + parts.append(f"### {relative}\n\n(named by the operator, and not present)\n") + sources.append(f"{relative} (MISSING)") + continue + found += 1 + sources.append(relative) + parts.append(f"### {relative}\n\n{text}\n") + # Guessing stops at the first hit; being explicit does not. One + # README is evidence, seven candidate files is noise. + if not docs and found >= 2: + break + + tree = _git(repo, "ls-files") + if tree: + paths = tree.splitlines() + sources.append(f"{len(paths)} tracked path(s)") + parts.append("### Tracked paths\n\n" + "\n".join(paths[:600]) + "\n") + + recent = _git(repo, "log", "--oneline", "-40") + if recent: + sources.append("recent history") + parts.append(f"### Recent commits\n\n{recent}\n") + + return Evidence(sources=sources, text="\n".join(parts)) + + +def survey( + objective: str, + repo: Path, + *, + ask: Callable[[str], str], + docs: list[str] | None = None, + name: str = "Plan", + now: float = 0.0, +) -> SurveyReport: + """Propose a plan for `objective` against `repo`, and check it can be read. + + Takes `ask` rather than a client so the surveyor's transport, routing and + retry ladder stay the caller's business — the same shape `ModelAssessor` + uses, and what makes this testable without a network. + """ + from .plan import parse_plan + + if not objective.strip(): + raise ValueError("a survey needs an objective; there is nothing to plan towards") + + evidence = gather(repo, docs) + prompt = SURVEY_PROMPT.format(objective=objective.strip(), evidence=evidence.render()) + proposal = parse_proposal(ask(prompt), 1, now) + + # Phase headings are containers here, not work. Their brief would be the + # phase's rationale, and an agent that claims one is being asked to + # implement a reason. `inception` keeps the other default; see there. + markdown = render_plan(proposal, name, phases_as_items=False) + # The harness's own reader is the gate. A generated plan that this cannot + # consume is caught here rather than by `plan` three commands later, and + # the parser reports what it could not read rather than dropping it. + parsed = parse_plan(markdown) + return SurveyReport( + markdown=markdown, + proposal=proposal, + evidence=evidence, + item_count=len(parsed.items), + skipped=len(parsed.skipped), + duplicate_ids=sorted(parsed.duplicate_ids()), + dependency_problems=parsed.dependency_report().lines(), + blocking_questions=[q.question for q in proposal.blocking_open()], + ) diff --git a/src/agent_harness/work.py b/src/agent_harness/work.py index 5a0b28a..f63dda5 100644 --- a/src/agent_harness/work.py +++ b/src/agent_harness/work.py @@ -153,6 +153,11 @@ last_error TEXT, branch TEXT, pr_url TEXT, + -- What this item produces: 'code' (a diff) or 'findings' (an answer, + -- possibly with nothing to commit). Declared by the plan, never by the + -- agent. Defaults to 'code', which is what every row written before + -- this column existed meant. + deliverable TEXT NOT NULL DEFAULT 'code', -- The graph revision this claim was admitted at. Admission and the check -- before the expensive gate have to be talking about the same graph; this -- is how the second one can tell that the first one saw a different one. @@ -429,6 +434,10 @@ class WorkRecord: attempts: int = 0 last_error: str | None = None branch: str | None = None + #: What this item produces: `code` (a diff) or `findings` (an answer). + #: The plan declares it; the agent never chooses it, or the first hard + #: test failure becomes an essay about why the test was wrong (#182). + deliverable: str = "code" pr_url: str | None = None updated_at: float = 0.0 project_id: str = DEFAULT_PROJECT @@ -600,6 +609,9 @@ def _migrate(self) -> None: "first_started_at": "REAL NOT NULL DEFAULT 0", # Stage J, additive: an existing row reads as "not held". "held_until": "REAL NOT NULL DEFAULT 0", + # #182, additive: every existing item produces a diff, which is + # what it always did. + "deliverable": "TEXT NOT NULL DEFAULT 'code'", }, } @@ -810,7 +822,7 @@ def add( if existing is None: conn.execute( "INSERT INTO work (project_id, item_id, issue, title, brief, depends_on, " - "state, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", + "state, updated_at, deliverable) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", ( project_id, record.item_id, @@ -820,19 +832,21 @@ def add( json.dumps(record.depends_on), record.state, self.now(), + record.deliverable, ), ) added += 1 else: conn.execute( "UPDATE work SET title = ?, brief = ?, depends_on = ?, issue = ?, " - "updated_at = ? WHERE project_id = ? AND item_id = ?", + "updated_at = ?, deliverable = ? WHERE project_id = ? AND item_id = ?", ( record.title, record.brief, json.dumps(record.depends_on), record.issue, self.now(), + record.deliverable, project_id, record.item_id, ), diff --git a/tests/test_plan.py b/tests/test_plan.py index c74b640..4396158 100644 --- a/tests/test_plan.py +++ b/tests/test_plan.py @@ -6,6 +6,8 @@ from __future__ import annotations +import pytest + from agent_harness.plan import parse_plan PLAN = """\ @@ -235,3 +237,33 @@ def test_a_dotted_dependency_resolves_to_the_sub_item() -> None: ) assert plan.items[1].depends_on == ["P0.1"] assert plan.unresolved_dependencies() == {} + + +# ---------------------------------------- what an item says it produces + + +def test_an_item_declares_what_it_produces() -> None: + """#182. The plan says it; the agent never chooses it, or the first hard + test failure becomes an essay about why the test was wrong.""" + from agent_harness.plan import CODE, FINDINGS + + plan = parse_plan( + "### T1 — Compare three approaches\n\n" + "Which of tsnet, a sidecar or the host daemon fits.\n\n" + "deliverable: findings\n\n" + "### T2 — Implement the chosen one\n\n" + "Ordinary work.\n" + ) + + items = {i.id: i for i in plan.items} + assert items["T1"].deliverable == FINDINGS + assert items["T2"].deliverable == CODE, "the default is a diff, as it always was" + # Bookkeeping is removed from the brief: an agent should read the + # specification, not the metadata. + assert "deliverable:" not in items["T1"].brief() + + +def test_an_unrecognised_deliverable_is_refused() -> None: + """Two kinds, not a taxonomy. A third needs a third review rubric.""" + with pytest.raises(ValueError, match="deliverable must be one of"): + parse_plan("### T1 — A thing\n\ndeliverable: interpretive dance\n") diff --git a/tests/test_session_executor.py b/tests/test_session_executor.py index 1680ea3..0def50a 100644 --- a/tests/test_session_executor.py +++ b/tests/test_session_executor.py @@ -20,12 +20,15 @@ from agent_harness.model_client import ModelClient, Response, RetryExhausted, Route from agent_harness.outcomes import ( BLOCKED, + COMPLETED, ESCALATED, ITEM_IMPOSSIBLE, NEEDS_A_PERSON, NO_TARGET, ) +from agent_harness.plan import FINDINGS from agent_harness.session_executor import ( + FINDINGS_FILE, REFUSAL_FILE, REFUSAL_LIMIT, AgentSpec, @@ -869,3 +872,127 @@ def test_a_refusal_lands_in_blocked_not_failed(repo: Path, tmp_path: Path) -> No item = queue.get("W1") assert item is not None assert item.state == BLOCKED + + +# ------------------------------- an item whose deliverable is an answer + + +def answer(text: str) -> Callable[[Path], None]: + """An agent given a question, which writes the answer and changes nothing.""" + + def agent(tree: Path) -> None: + (tree / FINDINGS_FILE).write_text(text) + + return agent + + +def add_question(queue: WorkQueue, item_id: str = "W1") -> None: + queue.add( + [ + WorkRecord( + item_id=item_id, + title="Is multiply feasible", + brief="Say whether calc.py can support exact decimal arithmetic.", + deliverable=FINDINGS, + ) + ] + ) + + +def test_a_findings_item_completes_on_its_answer(repo: Path, tmp_path: Path) -> None: + """The whole of #182: an investigation is finished when it has an answer. + + Before this, a findings item left a clean tree, hit the clean-tree path, + and was recorded `escalated / no_target` — a completed investigation was + indistinguishable from an agent that did nothing. + """ + found = "calc.py uses floats throughout (calc.py:1). Decimal would need a new module." + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=answer(found))) + add_question(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.state == DONE, outcome.reason + assert outcome.stop is not None + assert outcome.stop.disposition == COMPLETED + assert found in outcome.reason + + +def test_the_answer_is_not_committed(repo: Path, tmp_path: Path) -> None: + """The answer is the item's result, not a change to the repository.""" + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=answer("no, and here is why"))) + add_question(queue) + + executor.run_once() + + assert git(repo, "log", "--oneline", "main..harness/w1").strip() == "" + assert not (tmp_path / "trees" / "W1" / FINDINGS_FILE).exists() + + +def test_a_findings_item_is_told_what_to_produce(repo: Path, tmp_path: Path) -> None: + seen: list[str] = [] + + def read_prompt(tree: Path) -> None: + seen.append((tree / ".harness-prompt.md").read_text()) + (tree / FINDINGS_FILE).write_text("an answer") + + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=read_prompt)) + add_question(queue) + executor.run_once() + + assert FINDINGS_FILE in seen[0] + assert "deliverable is an answer, not a change" in seen[0] + # Its checks cannot apply, and saying so beats describing gates that will + # never run against a change it is told not to make. + assert "changes no code" in seen[0] + + +def test_a_code_item_is_told_none_of_that(repo: Path, tmp_path: Path) -> None: + """The default is unchanged: an ordinary item never hears about findings.""" + seen: list[str] = [] + + def read_prompt(tree: Path) -> None: + seen.append((tree / ".harness-prompt.md").read_text()) + add_multiply(tree) + + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=read_prompt)) + add_item(queue) + executor.run_once() + + assert FINDINGS_FILE not in seen[0] + + +def test_a_refusal_still_wins_over_an_answer(repo: Path, tmp_path: Path) -> None: + """ "This question cannot be answered here" is an escalation, whatever the + item was asked to produce.""" + + def refuse_the_question(tree: Path) -> None: + (tree / FINDINGS_FILE).write_text("a half-hearted guess") + (tree / REFUSAL_FILE).write_text("the module this asks about does not exist") + + executor, queue = build(repo, tmp_path, FakeDevEnv(agent=refuse_the_question)) + add_question(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.stop is not None + assert outcome.stop.disposition == ESCALATED + assert outcome.stop.reason_kind == ITEM_IMPOSSIBLE + + +def test_a_findings_item_that_answers_nothing_still_needs_a_person( + repo: Path, tmp_path: Path +) -> None: + """Asked for an answer, given nothing, and not told why. That is the + clean-tree case exactly, and it belongs there.""" + executor, queue = build(repo, tmp_path, FakeDevEnv()) + add_question(queue) + + outcome = executor.run_once() + + assert outcome is not None + assert outcome.stop is not None + assert outcome.stop.disposition == ESCALATED + assert outcome.state == BLOCKED diff --git a/tests/test_survey.py b/tests/test_survey.py new file mode 100644 index 0000000..0850a3b --- /dev/null +++ b/tests/test_survey.py @@ -0,0 +1,215 @@ +"""Generating a plan for a project that already exists. + +The model is faked; the repository, the git calls and the plan parser are all +real. That split is the point: what matters here is not what a model says, it +is whether the harness can *read back* what it produced, and that answer must +come from the same parser a hand-written plan goes through. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path + +import pytest + +from agent_harness.survey import DOC_LIMIT, gather, survey + + +def git(path: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(path), *args], capture_output=True, text=True, check=True + ).stdout + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + path = tmp_path / "project" + (path / "docs").mkdir(parents=True) + git(path.parent, "init", "-q", "-b", "main", str(path)) + git(path, "config", "user.email", "t@t") + git(path, "config", "user.name", "t") + (path / "README.md").write_text("# Project\n\nA thing.\n") + (path / "docs" / "current-state.md").write_text( + "# Current state\n\n## Active forward roadmap\n\nUpgrade the runtime to Node 22.\n" + ) + (path / "src.js").write_text("console.log(1)\n") + git(path, "add", "-A") + git(path, "commit", "-q", "-m", "initial") + return path + + +def proposal_json(items: list[dict[str, object]], **extra: object) -> str: + payload: dict[str, object] = { + "goal": "Upgrade the runtime.", + "assumptions": [], + "non_goals": [], + "risks": [], + "phases": [{"id": "P0", "title": "Upgrade", "why": "because", "items": items}], + "open_questions": [], + } + payload.update(extra) + return json.dumps(payload) + + +def item(item_id: str, title: str = "Do the thing", **extra: object) -> dict[str, object]: + out: dict[str, object] = { + "id": item_id, + "title": title, + "brief": "Change the engines field in package.json to >=22, and keep CI green.", + "depends_on": [], + } + out.update(extra) + return out + + +def answering(text: str): # type: ignore[no-untyped-def] + prompts: list[str] = [] + + def ask(prompt: str) -> str: + prompts.append(prompt) + return text + + ask.prompts = prompts # type: ignore[attr-defined] + return ask + + +# ------------------------------------------------- what the surveyor is shown + + +def test_the_project_s_own_roadmap_reaches_the_prompt(repo: Path) -> None: + """The failure this module exists for: the roadmap was never opened.""" + ask = answering(proposal_json([item("T1")])) + + survey("upgrade to Node 22", repo, ask=ask, docs=["docs/current-state.md"]) + + assert "Active forward roadmap" in ask.prompts[0] + assert "Upgrade the runtime to Node 22" in ask.prompts[0] + + +def test_a_named_document_that_is_missing_is_reported_not_skipped(repo: Path) -> None: + """Asking for the roadmap and silently planning without it is the bug.""" + evidence = gather(repo, ["docs/no-such-file.md"]) + + assert any("MISSING" in source for source in evidence.sources) + assert "not present" in evidence.text + + +def test_without_named_documents_it_guesses_and_stops_at_two(repo: Path) -> None: + """One README is evidence. Seven candidate files is noise.""" + evidence = gather(repo, None) + + docs = [s for s in evidence.sources if s.endswith(".md")] + assert docs == ["docs/current-state.md", "README.md"] + + +def test_the_tree_and_recent_history_are_shown(repo: Path) -> None: + evidence = gather(repo, ["README.md"]) + + assert "src.js" in evidence.text + assert "initial" in evidence.text + assert any("tracked path" in s for s in evidence.sources) + + +def test_an_enormous_document_cannot_crowd_out_everything_else(repo: Path) -> None: + (repo / "docs" / "current-state.md").write_text("x" * (DOC_LIMIT * 3)) + + evidence = gather(repo, ["docs/current-state.md"]) + + assert len(evidence.text) < DOC_LIMIT * 2 + + +def test_an_empty_objective_is_refused(repo: Path) -> None: + with pytest.raises(ValueError, match="objective"): + survey(" ", repo, ask=answering(proposal_json([item("T1")]))) + + +# ------------------------------------- the harness's own parser is the gate + + +def test_a_generated_plan_is_read_back_by_the_real_parser(repo: Path) -> None: + report = survey( + "upgrade to Node 22", + repo, + ask=answering(proposal_json([item("T1"), item("T2", "And another")])), + ) + + assert report.item_count == 2 + assert report.usable + assert "### T1 — Do the thing" in report.markdown + + +def test_a_plan_the_harness_cannot_read_is_not_usable(repo: Path) -> None: + """A model that returns valid JSON with no items produces an empty plan. + + Nothing downstream would fail on this — `plan` would sync zero issues and + `run` would say "nothing to do" — so it has to be caught where it happens. + """ + report = survey("upgrade to Node 22", repo, ask=answering(proposal_json([]))) + + assert report.item_count == 0 + assert not report.usable + + +def test_duplicate_ids_make_a_plan_unusable(repo: Path) -> None: + """Each id becomes one issue and one queue row, so two T1s is not a plan.""" + report = survey( + "upgrade to Node 22", + repo, + ask=answering(proposal_json([item("T1"), item("T1", "A different thing")])), + ) + + assert report.duplicate_ids == ["T1"] + assert not report.usable + + +def test_a_blocking_question_is_reported_and_does_not_make_it_unusable(repo: Path) -> None: + """It is a question for the human. Their answer decides, not this code.""" + report = survey( + "upgrade to Node 22", + repo, + ask=answering( + proposal_json( + [item("T1")], + open_questions=[ + { + "id": "Q1", + "question": "Is the native addon still maintained?", + "severity": "blocking", + "why_it_matters": "it decides whether this is possible at all", + } + ], + ) + ), + ) + + assert report.blocking_questions == ["Is the native addon still maintained?"] + assert report.usable + assert "Q1" in report.markdown + + +def test_the_report_says_what_was_read(repo: Path) -> None: + """The first question about any generated plan is what it actually read.""" + report = survey( + "upgrade to Node 22", repo, ask=answering(proposal_json([item("T1")])), docs=["README.md"] + ) + + assert "README.md" in report.lines()[0] + + +def test_a_phase_heading_is_not_read_back_as_a_work_item(repo: Path) -> None: + """`## P0 Upgrade` matches the parser's item pattern — `P0` is a valid id. + + Every generated plan therefore carried one phantom item per phase, whose + brief was the phase's rationale. Found by counting items in a two-item + plan and getting three; it applies to `inception`'s output equally. + """ + report = survey( + "upgrade to Node 22", + repo, + ask=answering(proposal_json([item("T1"), item("T2", "And another")])), + ) + + assert report.item_count == 2 + assert [i for i in report.markdown.splitlines() if i.startswith("## Phase")]