From b6dda2bf8fdc250fb1d9b3d6545fc5394aa29071 Mon Sep 17 00:00:00 2001 From: "linh.doan" Date: Mon, 14 Sep 2026 16:42:12 +0700 Subject: [PATCH] feat(prd-intake): make the PRD an input, and one command bring the factory up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The driver had no route from what the operator writes into what it builds. docs/PRD.md is a state document by policy and a dirty one *blocks* every iteration, the deterministic lane was empty so the LLM could only find itself to work on (#355: weeks of loop plumbing), and starting the driver meant assembling SELFBUILD_* exports, a `devagent` on PATH, and `make loop-start` by hand. Three changes, one value: write what you want built, type one word, get tested PRs. internal/prdintake (#370): an OPEN checkbox in docs/PRD.md is an instruction. Each `- [ ] item` becomes a queue row carrying its heading as context, its indented sub-bullets as acceptance criteria, and its section body as the task-PRD sidecar, so the worker reads the spec rather than one bullet; the row id is a content hash, so re-running intake over an unchanged PRD queues nothing and the shipped PR ticks the box it built. `- [x]`, blockquotes, struck lines, plain bullets and fenced examples stay exactly what policy says they are — never work. The driver ingests at iteration head, after the PRD-currency gate (a draft mid-edit is never read as intent) and before the pick, so the iteration that reads your edit is the iteration that builds it (`SELFBUILD_PRD_INTAKE=0` opts out); phase 2a already outranks the tracker, so operator intent now outranks LLM self-selection. `devagent prd-intake --dry-run|--json|--max N` runs the same step by hand. An empty lane is now a named state instead of silent progress: the fallback logs and breadcrumbs `queue-empty` with the refill instructions. `devagent up` / `devagent down` (#371): the prerequisite gate (a missing worker CLI aborts instead of burning tokens), the state/queue dirs, PRD intake, a lane census (pending rows / open PRD items / open selfbuild issues — the number that predicts whether the next ten hours ship product or plumbing), a refusal to start behind a dirty docs/PRD.md (the driver would skip every iteration and say nothing), the daemon, then the detached driver launched from its own executable with SELFBUILD_DEVAGENT_BIN pinned to match — the PATH trap that made a Go-only checkout silently shell out to a stale CLI. It ends in a HEALTH receipt, not a spawn receipt: `up` is the driver's parent, so a driver that halts at its own gate exits 0 and stays a pid that answers any liveness probe (measured: 15 seconds of "healthy" after `max iterations reached`), so `up` reaps its own child and waits until the loop lock AND a phase-naming heartbeat exist, or fails the run with the halt line from the driver's log. `--scout` adds the researcher as a second recorded child; `down` signals recorded pids only (never `pkill -f`, #354); `make up|down` now delegate to it. internal/scout (#372): `runScoutOnce`/`runScoutLoop` are ported at last, so `devagent scout --once/--interval` does its real cycle (lock -> config -> depth check BEFORE the paid dispatch -> prompt -> workers.GetWorker().Spawn -> extract -> parse -> enqueue + task-PRD sidecar -> heartbeat) instead of exiting 3 while the scout LaunchAgent ticks every 30 minutes and the queue writer stays silently dead. `devagent create --scout/--tracker` also stopped baking the deleted `dist/src/cli.js` into plist argv, where cobra would choke on it. Verification: gofmt/go vet/golangci-lint clean; full `go test ./...` green (24 new tests: parser contract incl. fences/blockquotes/struck/nesting, goal shape at the 120-word dispatch cap, ingest idempotency + reword + per-pass cap, same-iteration claim, per-iteration idempotency, off-switch and failure paths, dry-run writes nothing, up's step plan/idempotency/gate refusal/ empty-lane hint/dirty-PRD refusal/both health-receipt directions, down's recorded-pid stop); live smokes end to end — a scratch PRD item queued -> claimed -> published as goals/loop-1.md, `up` printing `driver running (pid 31815) — holds the loop lock, iteration 2, phase queue-empty` and then `✗ driver pid 27840 is not running: exited: max iterations reached`, `down` leaving no surviving process, and `scout --once --dry-run` previewing a cycle with no AI call and no writes. docs/PRD.md grows FR-SIMPLE-07/08 + §12 rows + the status note and footer (and the gap-audit blockquote staged in the tree), docs/SELF-BUILD-LOOP.md gains the one-command section, the step table and the intake policy exception, README gains the driver path, PRD.html regenerated with pandoc, and the 2026-09-14 cold-start research pass that shaped the health receipt is committed so its citations resolve. --- Makefile | 29 +- README.md | 46 + docs/PRD.html | 135 ++- docs/PRD.md | 62 ++ docs/SELF-BUILD-LOOP.md | 67 ++ ...6-09-14-easy-local-setup-selfbuild-loop.md | 341 +++++++ internal/cli/actions_prdintake.go | 79 ++ internal/cli/actions_scout.go | 63 +- internal/cli/actions_updown.go | 120 +++ internal/cli/root.go | 8 + internal/commands/updown.go | 898 ++++++++++++++++++ internal/commands/updown_other.go | 25 + internal/commands/updown_test.go | 790 +++++++++++++++ internal/commands/updown_unix.go | 31 + internal/loopdriver/config.go | 32 + internal/loopdriver/intake.go | 72 ++ internal/loopdriver/intake_test.go | 166 ++++ internal/loopdriver/run.go | 17 +- internal/pipeline/create.go | 9 +- internal/prdintake/prdintake.go | 399 ++++++++ internal/prdintake/prdintake_test.go | 255 +++++ internal/scout/extract.go | 11 +- internal/scout/run.go | 350 +++++++ internal/scout/run_test.go | 315 ++++++ internal/scout/scout.go | 6 +- 25 files changed, 4292 insertions(+), 34 deletions(-) create mode 100644 docs/research/2026-09-14-easy-local-setup-selfbuild-loop.md create mode 100644 internal/cli/actions_prdintake.go create mode 100644 internal/cli/actions_updown.go create mode 100644 internal/commands/updown.go create mode 100644 internal/commands/updown_other.go create mode 100644 internal/commands/updown_test.go create mode 100644 internal/commands/updown_unix.go create mode 100644 internal/loopdriver/intake.go create mode 100644 internal/loopdriver/intake_test.go create mode 100644 internal/prdintake/prdintake.go create mode 100644 internal/prdintake/prdintake_test.go create mode 100644 internal/scout/run.go create mode 100644 internal/scout/run_test.go diff --git a/Makefile b/Makefile index 17aa2dd1..be6a06f9 100644 --- a/Makefile +++ b/Makefile @@ -5,8 +5,10 @@ # make agents-on # re-enable + load them again # make loop-status # is the loop running? (+ supervision mode) tail of its latest iteration log # make agents-uninstall # bootout + disable + delete installed plists (repo copies kept) -# make loop-start # start the Go selfbuild loop (./devagent-go loop) in the background -# make loop-stop # stop the background selfbuild loop +# make up # seed the lane from docs/PRD.md + start the driver (proven alive) +# make down # stop what `make up` recorded +# make loop-start # alias of `make up` +# make loop-stop # alias of `make down` # make loop-status # is the loop running? tail of its latest iteration log # make loop-log # tail -f the running loop's driver output # make daemon-start # start the FR-CTRL daemon (./devagent-go daemon) in the background @@ -39,26 +41,25 @@ WATCHDOG_LABELS := \ ALL_LABELS := $(DEVAGENT_LABELS) $(WATCHDOG_LABELS) -.PHONY: loop-start loop-stop loop-status loop-log daemon-start daemon-stop +.PHONY: up down loop-start loop-stop loop-status loop-log daemon-start daemon-stop # --- Selfbuild loop (background) -------------------------------------------- # The loop runs via hub-managed nohup; workers default to visible herdr panes # (attach with `devagent attach `), headless via DEVAGENT_VISIBILITY=headless. LOOP_LOG_DIR := .selfbuild/logs -loop-start: - @if pgrep -f "devagent-go loop" >/dev/null 2>&1; then \ - echo "selfbuild loop already running (pid $$(pgrep -f 'devagent-go loop' | head -1))"; exit 0; \ - fi +# Thin wrappers over the binary's own lifecycle commands (issue #371): the +# muscle memory survives, but there is now ONE start path. `up` seeds the work +# lane from docs/PRD.md and proves the driver is alive (loop lock + heartbeat) +# instead of printing a pid; `down` signals the pids it recorded, so the old +# `pkill -f "devagent-go loop"` is retired — a pattern kill also hits drivers +# started from another checkout (issue #354). +up loop-start: @if [ ! -x ./devagent-go ]; then echo "no ./devagent-go — run: make build" >&2; exit 1; fi - @mkdir -p "$(LOOP_LOG_DIR)" - @nohup ./devagent-go loop >> "$(LOOP_LOG_DIR)/driver.log" 2>&1 & \ - echo "selfbuild loop started (pid $$!) — log: $(LOOP_LOG_DIR)/driver.log; TUI: devagent tui" + @./devagent-go up --no-daemon -loop-stop: - @if pgrep -f "devagent-go loop" >/dev/null 2>&1; then \ - pkill -f "devagent-go loop" && echo "selfbuild loop stopped"; \ - else echo "selfbuild loop not running"; fi +down loop-stop: + @if [ -x ./devagent-go ]; then ./devagent-go down; else pkill -f "devagent-go loop" && echo "selfbuild loop stopped"; fi loop-status: @if pgrep -f "devagent-go loop" >/dev/null 2>&1; then \ diff --git a/README.md b/README.md index 55614751..0cd29067 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,52 @@ Tracked in the [roadmap](docs/PRD.md#17-roadmap) and the [Grok Bot & control app - **Bot-style UX floor** — named persistent agent identities, teach-once routines, visible bot-to-bot handoff (§20.1, Q45) - **Durable knowledge-graph context** — whether the LeanKG digest persists across runs or stays per-run (Q28) +## Run the automated driver (one command) + +```bash +make build # -> ./devagent-go + +devagent up # check prerequisites → queue your PRD items → start the driver +devagent up --scout # ...and run the 24/7 researcher beside it (FR-SCOUT-01) +devagent up --dry-run # print that plan without touching anything +devagent status # what is running, what it is doing, what happens next +devagent down # stop exactly what `up` started +``` + +`devagent up` is the whole setup: it runs the `devagent init` prerequisite +gate (a missing worker CLI aborts the start instead of burning tokens on work +that can never be dispatched), creates the loop state and queue directories, +ingests your `docs/PRD.md` into the work lane, prints the lane census (pending +rows, open PRD items, open `selfbuild` issues — the number that says whether +the next ten hours ship product or plumbing), brings up the localhost daemon +the TUI reads, and starts the self-build driver detached — launched from the +same binary you just ran, so nothing has to be installed on `PATH`. + +Then it proves the start instead of reporting a pid: a green line means the +driver holds the loop lock and has published a heartbeat naming its iteration +and phase (`driver running (pid 31815) — holds the loop lock, iteration 2, +phase queue-empty`); a driver that halts at its own gate fails `up` with the +halt line from its log. `--wait 0` skips that proof for scripted starts. + +**Tell it what to build in the PRD.** An open checkbox anywhere in +`docs/PRD.md` is an instruction; the driver picks items top-of-file first, +implements each against its own pipeline, and ships a PR that updates the PRD +and ticks that checkbox: + +```markdown +- [ ] Add a `--watch` mode to `devagent status` + - one line per iteration, exits on SIGINT +``` + +```bash +devagent prd-intake --dry-run # preview what your PRD currently queues +devagent prd-intake # queue it — the next iteration claims it +``` + +Everything the driver does per iteration is in +[docs/SELF-BUILD-LOOP.md](docs/SELF-BUILD-LOOP.md); `devagent loop` remains +the foreground form of the same driver. + ## Factory (24/7 scout + Orca workers) ```bash diff --git a/docs/PRD.html b/docs/PRD.html index ce0ec3dd..a71b0335 100644 --- a/docs/PRD.html +++ b/docs/PRD.html @@ -1471,6 +1471,18 @@

Commands

Show effective configuration (workers, budgets, credentials presence) + +devagent up / devagent down +One command to check prerequisites, seed the work lane from +docs/PRD.md, and start/stop the self-build driver + daemon +(FR-SIMPLE-07) + + +devagent prd-intake +Queue the operator's open - [ ] items in +docs/PRD.md as loop work (FR-SIMPLE-08; deterministic, +idempotent, --dry-run/--json) +

Flags (run)

@@ -2435,6 +2447,31 @@

Phase 5 — (FR-TUI-P-01..12, PR #268). The network sandbox allowlist (#180, P2) is the remaining Phase 4 leftover. Shipped from this phase: Grok/xAI worker adapter (internal/workers/grok.go),

+

Go-port gap audit (2026-09-13): a code-level audit +of the Go surface found the following specced-but-stubbed seams, each +now tracked as an issue (issue-first per the tracker note above) — +serve webhook-triggered dispatch still prints the FR-GO-07 +#194 deviation stub and never calls the pipeline (#356, +internal/cli/actions_serve.go:220); +init --smoke reports hardcoded +fixtureOK/gatesOK := true, so the FR-SIMPLE-01 +"verified smoke" verifies nothing (#357, +internal/commands/init.go:131); the daemon/executor +worker-reaper seams are nil by default, so stale-worker reaping silently +skips on paths that don't inject a dispatcher (#358, FR-GO-05); +spawn.visibility in devagent.json resolves env-only — the +SpawnVisibilityConfig global is read but never assigned +(#359); the Grok cost-ledger seam OnCost (FR-GROK-03) is +invoked but never set, so per-run cost never reaches the ledger (#363); +herdr-sweep --orphan-brokers remains Node-only per the +HERDR.md status note (#360); FR-CTX-01..05 remain unimplemented (#368, +see §20.1); and docs/PRODUCTION-READINESS.md still describes the retired +Node tree, so no Go-era readiness verdict exists (#362). Lower-priority +convergence work, also issue-first: lessons-digest dedupe + eval guard +(#361), Windows parity (#364), parity-shim convergence + vestigial +exit-3 machinery (#365), ClaudeCode adapter hardening (#366), +internal/cli + cmd/devagent test coverage (#367), War Room as a +first-class Go command (#369).

  • Easy handoff / control plane (FR-HAND, #145) — @@ -4549,11 +4586,74 @@

    21.2 Requirements

    release C + +FR-SIMPLE-07 +One-command factory lifecycle: devagent up runs the +prerequisite gate, creates the state/queue directories, ingests +docs/PRD.md intent into the work lane, prints the lane +census, starts the control-plane daemon and the self-build driver +(detached, from its own executable, idempotent against a live loop +lock), and proves the start — it holds the lock and +published a heartbeat — instead of reporting a pid; +devagent down stops exactly what up +recorded +M + + +FR-SIMPLE-08 +Intent lane: an open - [ ] checkbox in +docs/PRD.md is an instruction the driver implements, so +stating what to build uses the document the operator already writes in — +no second tool, no tracker syntax, no env wiring +(internal/prdintake, devagent prd-intake) +M +

    Boundary: simplicity is the default presentation, not a restriction — everything stays scriptable over FR-CTRL and flags; automation is unaffected.

    +
    +

    Status (2026-09-14): FR-SIMPLE-07 and FR-SIMPLE-08 +shipped together — internal/commands/updown.go +(devagent up / devagent down, #371) and +internal/prdintake (#370), wired +into the driver at internal/loopdriver/intake.go so every +iteration ingests the PRD before it picks. This is the fix for #355's "long +runtime, little value": with an empty lane the driver could only find +itself, and the empty-lane fallback now logs and breadcrumbs as +queue-empty with the refill instructions. #371 +also retires the remaining setup traps it exposed: the driver no longer +needs a devagent on PATH (up pins +SELFBUILD_DEVAGENT_BIN to the executable that started it), +and devagent create's scout/tracker LaunchAgents no longer +bake the deleted dist/src/cli.js into argv where cobra +would choke on it. The companion scout-cycle port is #372, which +up --scout now starts as a second recorded child so the +researcher fills the lane between iterations and +devagent down stops both.

    +

    Green up means a running factory (the review +finding that shaped FR-SIMPLE-07). Because up is +the driver's parent, a driver that halts at its own gate — iteration cap +already reached, starvation halt, a lock refusal — stays a pid that +answers a liveness probe, and it exits 0, so nothing else reports it +either; the first smoke run printed a healthy pid for fifteen seconds +after max iterations reached. up now reaps its +own child and treats that reap as the verdict, waiting (bounded by +--wait, default 15s) until the driver holds +​.selfbuild/loop.lock.d AND its heartbeat.json +names an iteration and a phase — then reporting exactly that +(driver running (pid N) — holds the loop lock, iteration 2, phase queue-empty) +or failing the run with the halt line pulled from +.selfbuild/logs/. The lane step reports the +same census upstream: pending queue rows, open PRD checkboxes, open +selfbuild issues, with the #355 remedy as its hint when all +three are zero.

    +

    22. Addendum: Full Go Migration (FR-GO)

    @@ -4945,8 +5045,39 @@

    23.1 Requirements

    chaos schedule) so "the driver works perfectly" is a checkable claim, not a hope.

    -

    *Last updated: 2026-09-13 (caveat on #349's revision stamp: -linked-worktree builds mis-stamp — *Last updated: 2026-09-14 (the PRD became an input, and +up became a health receipt: FR-SIMPLE-07/08, #370 + #371) — +devagent up now covers the whole setup-to-running path +(prerequisite gate through the init checks → state/queue +dirs → docs/PRD.md intake → a lane census of +pending rows / open PRD checkboxes / open selfbuild issues +→ daemon → detached driver launched from its own executable with +SELFBUILD_DEVAGENT_BIN pinned to match) and +proves the start rather than reporting a pid: +up is the driver's parent, so a driver that halts at its +own gate exits 0 and stays a zombie whose pid answers any liveness probe +(measured 2026-09-14: max iterations reached on stderr +while up printed a healthy pid for fifteen more seconds), +so it now reaps its own child and waits (bounded by --wait, +default 15s) until the loop lock AND a phase-naming heartbeat exist — +driver running (pid N) — holds the loop lock, iteration 2, phase queue-empty +— or fails the run with the halt line read out of +.selfbuild/logs/. Work intake flipped the other way too: an +open - [ ] checkbox in docs/PRD.md is now an +instruction (internal/prdintake queues it with its heading +as context, its sub-bullets as criteria and its section body as the +task-PRD sidecar; the driver ingests at iteration head after the +PRD-currency gate, SELFBUILD_PRD_INTAKE=0 opts out) and the +shipped PR ticks the checkbox it built, so a built item cannot re-enter +the lane. Queue-first already outranked the tracker, so operator intent +now outranks LLM self-selection — the value half of #355, whose other +half (an empty lane logged and breadcrumb'd as queue-empty +with the refill instructions instead of reading as progress) landed in +the same change. devagent create --scout/--tracker also +stopped baking the deleted dist/src/cli.js into LaunchAgent +argv, and the scout cycle itself is live in Go at last (#372). *Last +updated: 2026-09-13 (caveat on #349's revision stamp: linked-worktree +builds mis-stamp — #352) — while verifying #349's landing we found Go's -buildvcs resolves vcs.revision from the shared common gitdir in a linked diff --git a/docs/PRD.md b/docs/PRD.md index c121673e..214fe572 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -470,6 +470,8 @@ devagent run --ticket LINEAR-204 --worker claude-code # or opencode | both | `devagent status [--run ]` | Show recent runs and stage states | | `devagent log --run ` | Print structured run log | | `devagent config` | Show effective configuration (workers, budgets, credentials presence) | +| `devagent up` / `devagent down` | One command to check prerequisites, seed the work lane from `docs/PRD.md`, and start/stop the self-build driver + daemon (FR-SIMPLE-07) | +| `devagent prd-intake` | Queue the operator's open `- [ ]` items in `docs/PRD.md` as loop work (FR-SIMPLE-08; deterministic, idempotent, `--dry-run`/`--json`) | ### Flags (`run`) @@ -1090,6 +1092,29 @@ Direction addendum 2026-09-03 (section 20): DevAgent becomes the local-first, BY > scaffolded — PR #267), #146 TUI polish (FR-TUI-P-01..12, PR #268). The > network sandbox allowlist (#180, P2) is the remaining Phase 4 leftover. > Shipped from this phase: Grok/xAI worker adapter (`internal/workers/grok.go`), +> +> **Go-port gap audit (2026-09-13):** a code-level audit of the Go surface +> found the following specced-but-stubbed seams, each now tracked as an issue +> (issue-first per the tracker note above) — `serve` webhook-triggered +> dispatch still prints the FR-GO-07 #194 deviation stub and never calls the +> pipeline (#356, `internal/cli/actions_serve.go:220`); `init --smoke` +> reports hardcoded `fixtureOK`/`gatesOK := true`, so the FR-SIMPLE-01 +> "verified smoke" verifies nothing (#357, `internal/commands/init.go:131`); +> the daemon/executor worker-reaper seams are nil by default, so +> stale-worker reaping silently skips on paths that don't inject a +> dispatcher (#358, FR-GO-05); `spawn.visibility` in devagent.json resolves +> env-only — the `SpawnVisibilityConfig` global is read but never assigned +> (#359); the Grok cost-ledger seam `OnCost` (FR-GROK-03) is invoked but +> never set, so per-run cost never reaches the ledger (#363); +> `herdr-sweep --orphan-brokers` remains Node-only per the HERDR.md status +> note (#360); FR-CTX-01..05 remain unimplemented (#368, see §20.1); and +> docs/PRODUCTION-READINESS.md still describes the retired Node tree, so no +> Go-era readiness verdict exists (#362). Lower-priority convergence work, +> also issue-first: lessons-digest dedupe + eval guard (#361), Windows +> parity (#364), parity-shim convergence + vestigial exit-3 machinery +> (#365), ClaudeCode adapter hardening (#366), internal/cli + +> cmd/devagent test coverage (#367), War Room as a first-class Go command +> (#369). - **Easy handoff / control plane (FR-HAND, #145)** — Shipped 2026-09-08: cold path is exactly `devagent init` → `devagent tui` (optional `start` alias); TUI dispatch sheet (`n`, FR-HAND-02) + approve sheet (`g`, FR-HAND-07); `POST /dispatch` threads `autoPr` into the spawned `task --auto-pr` argv gated on `GITHUB_TOKEN` (FR-HAND-03); init adds worker-detect chips (FR-HAND-04), herdr default-on advisory (FR-HAND-05), Orca repo registration without worktree provisioning (FR-HAND-06). Look/feel bar: #146. - **Daemon control API** — ~~localhost REST + SSE control surface on the existing `serve` pattern with per-boot token auth and Origin/Host validation (FR-CTRL-01..05).~~ **Shipped 2026-09-08 (#179, PR #243):** internal/daemon serving `/status` `/agents` `/events` SSE `/history` + `POST /dispatch` `/approve`, per-boot token + loopback bind, wired as `devagent daemon` and live on :7788. - **Cross-platform desktop control app** — ~~Tauri 2 tray + dashboard on macOS, Linux, and Windows: dispatch agents/roles/tools, live agent log tails, approval inbox, notifications, pipeline visualization (FR-UI-01..09).~~ **Shipped 2026-09-09 (#181, PR #267):** Tauri 2 thin client at `app/` (Rust core + TS webview): FR-UI-01/02/03/04/07/08 functional (tray state, dispatch sheet, SSE dashboard, approval inbox + notifications, stage timeline), FR-UI-05 wired (autostart/single-instance), FR-UI-06 signing and FR-UI-09 3-OS CI scaffolded by design (no fake signing); see `app/README.md`. @@ -1580,10 +1605,46 @@ new runtime subsystem and does not change the pipeline contract (§8, §10–11) | FR-SIMPLE-04 | Progressive disclosure: at any moment each surface shows the current phase and the one next action (including the `devagent attach ` hint from FR-VIS-02); everything else is one keystroke/click away but never required to reach the first PR | S | | FR-SIMPLE-05 | First-run "where do I look" screen: after init, one view answers what the factory is doing right now, what happens next, and where to look — composed from the existing FR-CTRL status/events API (no second event system, no new tooling to learn) | S | | FR-SIMPLE-06 | Simplicity regression review: each release walks the four principles over the onboarding path and default surfaces; a change that adds a required step or a required concept is fixed or flagged before release | C | +| FR-SIMPLE-07 | One-command factory lifecycle: `devagent up` runs the prerequisite gate, creates the state/queue directories, ingests `docs/PRD.md` intent into the work lane, prints the lane census, starts the control-plane daemon and the self-build driver (detached, from its own executable, idempotent against a live loop lock), and **proves** the start — it holds the lock and published a heartbeat — instead of reporting a pid; `devagent down` stops exactly what `up` recorded | M | +| FR-SIMPLE-08 | Intent lane: an open `- [ ]` checkbox in `docs/PRD.md` is an instruction the driver implements, so stating what to build uses the document the operator already writes in — no second tool, no tracker syntax, no env wiring (`internal/prdintake`, `devagent prd-intake`) | M | **Boundary:** simplicity is the default presentation, not a restriction — everything stays scriptable over FR-CTRL and flags; automation is unaffected. +> **Status (2026-09-14):** FR-SIMPLE-07 and FR-SIMPLE-08 shipped together — +> `internal/commands/updown.go` (`devagent up` / `devagent down`, +> [#371](https://github.com/FreePeak/devagent/issues/371)) and +> `internal/prdintake` ([#370](https://github.com/FreePeak/devagent/issues/370)), +> wired into the driver at `internal/loopdriver/intake.go` so every iteration +> ingests the PRD before it picks. This is the fix for +> [#355](https://github.com/FreePeak/devagent/issues/355)'s "long runtime, +> little value": with an empty lane the driver could only find itself, and the +> empty-lane fallback now logs and breadcrumbs as `queue-empty` with the refill +> instructions. `#371` also retires the remaining setup traps it exposed: the +> driver no longer needs a `devagent` on `PATH` (`up` pins +> `SELFBUILD_DEVAGENT_BIN` to the executable that started it), and +> `devagent create`'s scout/tracker LaunchAgents no longer bake the deleted +> `dist/src/cli.js` into argv where cobra would choke on it. The companion +> scout-cycle port is +> [#372](https://github.com/FreePeak/devagent/issues/372), which `up --scout` +> now starts as a second recorded child so the researcher fills the lane +> between iterations and `devagent down` stops both. +> +> **Green `up` means a running factory (the review finding that shaped +> FR-SIMPLE-07).** Because `up` is the driver's parent, a driver that halts at +> its own gate — iteration cap already reached, starvation halt, a lock +> refusal — stays a pid that answers a liveness probe, and it exits 0, so +> nothing else reports it either; the first smoke run printed a healthy pid for +> fifteen seconds after `max iterations reached`. `up` now reaps its own child +> and treats that reap as the verdict, waiting (bounded by `--wait`, default +> 15s) until the driver holds `​.selfbuild/loop.lock.d` AND its +> `heartbeat.json` names an iteration and a phase — then reporting exactly that +> (`driver running (pid N) — holds the loop lock, iteration 2, phase +> queue-empty`) or failing the run with the halt line pulled from +> `.selfbuild/logs/`. The `lane` step reports the same census upstream: +> pending queue rows, open PRD checkboxes, open `selfbuild` issues, with the +> #355 remedy as its hint when all three are zero. + ## 22. Addendum: Full Go Migration (FR-GO) > Added 2026-09-07 (operator decision): DevAgent's core — CLI, daemon, @@ -1695,6 +1756,7 @@ existing driver with validation surfaces (test, command, telemetry, chaos schedule) so "the driver works perfectly" is a checkable claim, not a hope. --- +*Last updated: 2026-09-14 (the PRD became an input, and `up` became a health receipt: FR-SIMPLE-07/08, #370 + #371) — `devagent up` now covers the whole setup-to-running path (prerequisite gate through the `init` checks → state/queue dirs → `docs/PRD.md` intake → a `lane` census of pending rows / open PRD checkboxes / open `selfbuild` issues → daemon → detached driver launched from its own executable with `SELFBUILD_DEVAGENT_BIN` pinned to match) and **proves** the start rather than reporting a pid: `up` is the driver's parent, so a driver that halts at its own gate exits 0 and stays a zombie whose pid answers any liveness probe (measured 2026-09-14: `max iterations reached` on stderr while `up` printed a healthy pid for fifteen more seconds), so it now reaps its own child and waits (bounded by `--wait`, default 15s) until the loop lock AND a phase-naming heartbeat exist — `driver running (pid N) — holds the loop lock, iteration 2, phase queue-empty` — or fails the run with the halt line read out of `.selfbuild/logs/`. Work intake flipped the other way too: an open `- [ ]` checkbox in `docs/PRD.md` is now an instruction (`internal/prdintake` queues it with its heading as context, its sub-bullets as criteria and its section body as the task-PRD sidecar; the driver ingests at iteration head after the PRD-currency gate, `SELFBUILD_PRD_INTAKE=0` opts out) and the shipped PR ticks the checkbox it built, so a built item cannot re-enter the lane. Queue-first already outranked the tracker, so operator intent now outranks LLM self-selection — the value half of #355, whose other half (an empty lane logged and breadcrumb'd as `queue-empty` with the refill instructions instead of reading as progress) landed in the same change. `devagent create --scout/--tracker` also stopped baking the deleted `dist/src/cli.js` into LaunchAgent argv, and the scout cycle itself is live in Go at last (#372). *Last updated: 2026-09-13 (caveat on #349's revision stamp: linked-worktree builds mis-stamp — [#352](https://github.com/FreePeak/devagent/issues/352)) — while verifying #349's landing we found Go's `-buildvcs` resolves `vcs.revision` from the shared common gitdir in a linked worktree, so a `devagent` binary built inside a per-task worktree stamps the primary checkout's `refs/heads/main` tip rather than its own HEAD (`vcs.modified=true`). Proven A/B at the same commit `8ca0688`: a plain clone stamps `8ca0688`, a linked worktree stamps `50b03e8`. Scope: this is a latent defect confined to *worktree-built* binaries — the shipped driver is built by `scripts/self-update.sh` from the primary non-worktree checkout (`go build -trimpath ./cmd/devagent` after `git pull --ff-only`), so it stamps correctly and `RunLoop`'s stale-binary guard behaves as intended in the live loop; only a manually-built worktree binary writes a wrong `release-created` revision and false-trips the (advisory-only) WARN. #349's "a stale binary is loud" holds for the normal-checkout build path; the worktree edge is tracked in #352. *Last updated: 2026-09-13 (release-created rows carry `tag`/`sha` again: PR #349's Revision stamp landed as a silent data-loss regression) — #349 added a `Revision` field to the Go `ReleaseRecord` but dropped `Tag`/`SHA` from the `record release` literal while still printing `(tag @ sha)` to stdout, so every Go-written release row persisted `"tag":""` `"sha":""` (the Q24 fields the record exists to carry) and no test covered the write site, so it shipped green. Restored both fields; `internal/cli/actions_release_test.go` now pins the CLI row shape (proven red on the regressed literal, green after); regenerated `docs/PRD.html` so the styled mirror matches `PRD.md`. *Last updated: 2026-09-13 (herdr orphan-pane reaping: both probes were dead, repaired as one change) — `devagent herdr-sweep --orphans` had never reaped a live orphan: `pane process-info` went out without `--session`, so herdr answered for its own default session and "no such pane" was every devagent pane's liveness, which left the FR-VIS-07 in-flight guard, the FR-VIS-02 roster upgrade (#317) and the orphan class — gated on a live foreground worker — inert at once; the owner probe was `herdr.*pane run .*`, a process that does not exist while a pane runs (`pane run` types the script into the pane's shell and exits; verified live parent chain `omp -> -zsh -> herdr --session server -> launchd`), so `PaneRunOwnerOrphaned` always took its "no owner CLI -> orphaned" exit and the driver spare behind it was unreachable. Each half alone is a regression — a working owner probe over an inert liveness probe closes live workers — so the fix lands atomically: `PaneForegroundWorker(cli, session, paneID)` scopes the probe (roster and sweep share it), the owner is now the surviving dispatcher (argv0-anchored `devagent`/`devagent-go` `task`/`pane-run`, optionally behind the driver's `timeout` wrapper, so a prompt quoting devagent cannot impersonate its own collector), non-worktree panes need positive per-pane dispatch evidence (the capture contract on the worker's fd 1/2), and the orphan class moved ahead of the roster spare. Reaping also fails closed: `psPidsMatching` and `pidAncestryCommands` now report whether the probe answered at all — pgrep's exit-1 "no match" is an answer (no collector, reap), an absent binary / the 5s cap / a rejected pattern / an unfinished ancestry walk is not (spare) — because a reaper that read "could not inspect" as "no collector" would close every live pane on such a host. Test seams: `DEVAGENT_SWEEP_OWNER_PIDS_JSON` is keyed by the pattern asked for and a miss now counts as an unanswered probe (never a clean no-match), `DEVAGENT_SWEEP_PANE_CAPTURE_JSON` supplies pane-keyed evidence — and because the 2026-09-13 bug survived a green stub suite, both shapes are pinned from outside: `TestProcFdsCaptureSeesRealCaptureContract` runs the real `lsof` probe against a live child holding `/devagent-herdr-/out` on fd 1/2 (skipped where lsof is absent), `TestPaneRunOwnerPatternMatchesRealDispatchShapes` checks `paneRunOwnerPattern` against the command lines `internal/loopdriver` really builds and against the transient `herdr pane run` shape it used to search for, and `TestOrphanClassGoesInertWhenProbesCannotAnswer` pins the spare-on-no-answer direction at the matrix level. `TestPsPidsMatchingAnswersOnlyWhenPgrepAnswers` pins the same split against real `pgrep` exit codes: 1 means no collector (reap), 2 means the probe could not run (spare). Docs corrected: FR-VIS-02/07/10, §18 Q23, `docs/HERDR.md` sweep safety. diff --git a/docs/SELF-BUILD-LOOP.md b/docs/SELF-BUILD-LOOP.md index 32843e26..804a7c32 100644 --- a/docs/SELF-BUILD-LOOP.md +++ b/docs/SELF-BUILD-LOOP.md @@ -109,6 +109,49 @@ the Node tree (FR-GO-16, #205). ## Running +### One command (the intended path, issue #371) + +```sh +make build # ./devagent-go — the Go single binary +./devagent-go up # check → seed the lane from docs/PRD.md → start the driver +./devagent-go down # stop exactly the processes `up` recorded +``` + +`up` is idempotent and reports one line per step: + +| Step | What it proves | +|---|---| +| `checks` | the `devagent init` prerequisite gate (a missing worker CLI aborts the start with the install line, instead of burning tokens on iterations that can never dispatch) | +| `dirs` | `.selfbuild/{research,goals,logs,curation,run}` + the queue/prds dirs exist, so every path the report names is real | +| `intake` | how many of the operator's `docs/PRD.md` items became queue rows (see "Tracker + PRD policy") | +| `prd` | `docs/PRD.md` is committed — the driver's own currency gate skips **every** iteration while the state doc is dirty, so starting over a draft buys a busy factory that ships nothing, and intake would have queued the draft as intent (`up` refuses until it is committed) | +| `lane` | the one number that predicts the next ten hours: pending queue rows, open PRD checkboxes, open `selfbuild` issues. An empty lane says so, with the fix — this is issue #355's "long runtime, little value" made visible before the start rather than after | +| `daemon` | the localhost control plane is listening (port answer, not a process-name guess; `--no-daemon` skips it) | +| `scout` | with `--scout`: the 24/7 researcher (`devagent scout --interval N`, cadence from `scout.intervalMinutes`) as a second detached child with its own pid file and log — the lane's other feeder (FR-SCOUT-01, issue #372) | +| `loop` | **a health receipt, not a spawn receipt**: `up` watches the driver it launched until it holds the loop lock AND writes a heartbeat naming an iteration and phase, and fails the run with the cause when it doesn't | + +The driver is launched from **its own executable** with +`SELFBUILD_DEVAGENT_BIN` pinned to match, which is what makes a checkout that +never installed a `devagent` on PATH work end to end. A live loop lock +outranks our pid file: a second `up` reports the running driver rather than +starting a competing one, and `devagent down` signals recorded pids only — +never a pattern kill (issue #354). `--dry-run` prints the plan and changes +nothing (no network probe either); `--foreground` runs the driver in this +terminal; `--wait 0` skips the health window for scripted starts; and +`--scout` adds the researcher as a second child so the lane fills itself +between iterations (`devagent down` stops both). + +Why the health window exists: `up` is the driver's **parent**, so a driver +that halts at its own gate — iteration cap reached, starvation halt, a lock +refusal — stays a reap-able zombie whose pid still answers a liveness probe, +and it exits 0, so no supervisor says anything either. The measured failure +(2026-09-14 smoke): `max iterations reached` on stderr while `up` printed a +healthy pid for fifteen more seconds. `up` therefore reaps its own child and +treats "the OS said it exited" as the verdict, reading the halt line out of +`.selfbuild/logs/` to name the cause. + +### The driver directly + Single-process infinite runner (the Go driver, `internal/loopdriver` — production since FR-GO-16, #205; the bash driver `scripts/selfbuild-loop.sh` was deleted in the same change): @@ -129,6 +172,8 @@ Environment knobs (all optional): | `SELFBUILD_PUSH_MODE` | `pr` | `pr` (branch + PR via auto-pr) or `main` (direct commit) | | `SELFBUILD_TEST_CMD` | `go test ./...` (default since the Node-tree retirement, #300) | Post-merge-back repo-level test gate (seam #230) | | `SELFBUILD_ISSUE_LABEL` | `selfbuild` | Issue label defining the loop's tracker queue | +| `SELFBUILD_PRD_INTAKE` | `1` | `0` stops the driver ingesting `docs/PRD.md` at iteration head (issue #370) | +| `SELFBUILD_PRD_INTAKE_MAX` | `5` | Cap on new queue rows per intake pass (document order wins) | | `SELFBUILD_ISSUE_MAX` | `50` | Max issues fetched per pick (deterministic sort: priority rank, then issue number) | | `SELFBUILD_GH_REPO` | derived from `git remote get-url origin` | Target repo for the tracker pick (`gh issue`) | | `SELFBUILD_DEVAGENT_BIN` | `devagent` | CLI binary the driver shells out to (pane-run, task, preflight, sync-docs, scan-text, ledger, herdr-sweep, page-degrade-breach) | @@ -196,6 +241,28 @@ default and the bash driver is gone. rank, then oldest issue number), builds it, and closes it with evidence. The LLM selection path exists only as the empty-tracker fallback; anything it picks still ships against the repo directly. +- **The one PRD exception: an open checkbox is an instruction (2026-09-14, + issue #370).** A tracker-only lane with no tracker left the driver inventing + work for itself — #355, where every iteration repaired loop plumbing. So + `internal/prdintake` reads `docs/PRD.md` at the head of each iteration and + queues every OPEN `- [ ]` item: its enclosing heading is the section + context, its indented sub-bullets become the acceptance criteria, and the + section body rides along as the queue task-PRD sidecar, so the worker gets + the spec rather than one bullet. Everything else in the PRD stays exactly + what policy says it is — `- [x]` is shipped state, a blockquote is a state + note, a struck line is history, a fenced example is documentation, a plain + bullet is prose — and the currency gate above still means an *uncommitted* + PRD edit is never read as intent. +- **Intake is idempotent by content hash.** A row's queue id is `PRD-<8 hex>` + of the normalized item text, so re-running over an unchanged PRD queues + nothing, and the row's own `TickCriterion` makes the shipped PR flip the + source checkbox to `- [x]` so a built item cannot re-enter the lane. By + hand: `devagent prd-intake [--dry-run] [--json] [--max N]`; automatically: + every iteration, or `devagent up` before it starts the driver. Knobs: + `SELFBUILD_PRD_INTAKE=0`, `SELFBUILD_PRD_INTAKE_MAX`. +- **Queue-first outranks the tracker, deliberately.** Phase 2a claims the + queue before `pickIssue` is consulted, so intent a human wrote down beats + both the issue lane and LLM self-selection. - **The phase-1 pick rides along (issue #301).** The tracker decides *which* issue, research decides *what to do with it* — so on an issue-first pick the driver reads this iteration's `.selfbuild/research/loop-N.md` (the `## Pick` diff --git a/docs/research/2026-09-14-easy-local-setup-selfbuild-loop.md b/docs/research/2026-09-14-easy-local-setup-selfbuild-loop.md new file mode 100644 index 00000000..c9a51a5b --- /dev/null +++ b/docs/research/2026-09-14-easy-local-setup-selfbuild-loop.md @@ -0,0 +1,341 @@ +# Making devagent easy to set up, easy to use, and easy to run unattended (local selfbuild loop) + +*Research pass 2026-09-14. Method: three read-only code/doc scouts over +`internal/**`, `docs/**`, `Makefile`, `scripts/`, `launchagents/`; a quantified +read of the production ledger (321 rows, 2026-08-23 → 2026-09-13) and 250 +iteration logs; and live cold-start experiments run against throwaway clones in +`/tmp` with a **local** git mirror as `origin` (no dispatch, no writes to the +real repo or GitHub). Every claim below carries either a `path:line` in the +current tree or the command that produced it.* + +## 1. What "easy" has to mean here + +devagent is a local-first factory: one Go binary that drives worker CLIs +(`omp`/`claude`/`opencode`/`grok`/`codex`) through a deterministic loop — +state sync → provider gate → doc gates → pick work → research → PO → implement +→ PR → auto-merge — and writes an append-only ledger of every iteration. "Easy" +is therefore not a nicer README; it is four properties: + +1. **Cold start is one command** and ends in a *live* driver, not a process + receipt. +2. **Zero env exports.** Every knob the operator needs is in `devagent.json`. +3. **Every stop is explained.** If the factory is not shipping, one command + says why and what to do next. +4. **The factory is quarantined.** Starting it in one checkout cannot change + another checkout's state (issue #354). + +Today 1–4 all fail, and the failure is cheap to demonstrate. + +## 2. Measured cold start (the experiment that should own the roadmap) + +Throwaway clone of `origin/main` (`68c36cf`) → local mirror as `origin` → +`make build` (1 s) → the documented sequence (`docs/SELF-BUILD-LOOP.md` §"Run +the loop"): + +| Step | Result | +|---|---| +| `make build` | ✓ 1 s, warm Go cache; 16 MB binary | +| `./devagent-go doctor` | **✗ exit 1** — `herdr` (no `devagent` session) and `daemon` (`:7788` refused) fail; both are "not set up yet", not "broken" | +| `./devagent-go loop --dry-run --max-iterations 2` | `max iterations reached`, **exit 0**, 0.5 s, zero work | +| `./devagent-go loop` (cap raised) | `[state] pulled 321 ledger entries` → `[starvation] 5 consecutive non-productive iterations — halting loop`, **exit 0**, <1 s | +| delete `.selfbuild/ledger.jsonl`, run again | `[state] pulled 321 ledger entries` → same starvation halt. The tail is **re-fetched from the shared `selfbuild/state` branch**, so the local delete cannot help | +| `SELFBUILD_HOME=/tmp/isolated ./devagent-go loop` | state still pulled into `/tmp/dvx/.selfbuild` — the loopdriver derives its state dir from `cfg.Repo` (`run.go:90`, `state.go:51`), so `SELFBUILD_HOME` **does not isolate a second driver** (it isolates the daemon and the runner only) | +| `./devagent-go up --no-daemon` (the in-flight #371 command, run twice: 07:36:30Z and 07:58:14Z) | **`ok=true`** — `checks` 6 passed, `dirs` ready, `intake: 0 open PRD item(s) … queue depth 0`, `loop: driver started (pid 11415 / 74548)`. Both drivers were dead at the next probe (t+5 s, t+10 s); the iteration log says `[starvation] 5 consecutive non-productive iterations — halting loop`. Because the driver dies and releases the lock, the *next* `up` starts a *new* one and reports green again — the idempotence check is correct and useless at the same time | +| second `up` | starts another pid, reports success again (the lock is free, because the last driver is dead) | + +Reproduce any of these; nothing above costs more than a minute. + +### 2.1 Why the halt is unavoidable + +The starvation verdict is a tail-walk over the ledger, with degraded rows +exempt and only `ok|pr-open|merged|pushed` breaking the streak +(`internal/orchestrator/selfbuild-gate.go:14,32,34,100-126`), evaluated +*before* any work (`internal/loopdriver/run.go:156-160`), and the ledger is a +cross-machine union pulled at driver start (`run.go:113` → +`state.go:213-222`, `mergeLedgerLines` `state.go:131-165`). + +The shared tail right now is **15 consecutive non-productive rows** (all +`invalid`, from 2026-09-12) measured by replaying that exact walk over +`.selfbuild/ledger.jsonl`. Limit is 5 (`config.go:130`). So the state branch +itself is the thing that refuses to start — not this machine, not the config. +There is no acknowledge/resume seam anywhere: no `resume` event in the ledger +schema (`internal/ledger/ledger.go`), no flag, no reset command. Exit is 0, so +`hub restart=on-failure` and the opt-in launchd template +(`KeepAlive {SuccessfulExit=false}`) deliberately do **not** bring it back. + +### 2.2 The `--max-iterations` trap in the same table + +`MaxIterations` is compared against the **absolute loop number**, not a count: +`n := nextLoopNumber(...)`, `if cfg.MaxIterations > 0 && n >= cfg.MaxIterations` +(`run.go:134-142`). On a repo whose next number is 362, `--max-iterations 2` +(and `20`, and `100`) means "run zero iterations", silently. The flag help says +"cap the iteration count (0 = unbounded)" (`internal/cli/actions_loop.go:37`). + +## 3. Why the operator can't see any of this + +**The stop has no face.** The halt line goes to +`.selfbuild/logs/loop-N.log` (`run.go:157`), which is *gitignored* and read by +nobody. `driver.log` — the thing `make loop-log` tails — never sees it (it +receives only the state-sync banners and the `tail -5` of fall-through +iterations, `run.go:175`). The TUI/dashboard read +`.selfbuild/heartbeat.json` (`internal/daemon/endpoints.go:51`), which the +driver last wrote *while working* and never updates on a terminal halt: the +production heartbeat right now still says +`{"iteration":361,"phase":"task","pid":51174}` with a dead pid, 18 h stale. +So the observed behavior of an unattended factory is "stopped, looks busy, +says nothing". + +**The hygiene report is a false all-clear.** `devagent herdr-sweep --dry-run` +printed `[devagent] no stale panes` and exited **0** while the `devagent` herdr +session had no server at all (`server_not_running`). On a CLI error the pane +list simply comes back empty, so the sweep reports the same "nothing stale" +line and exits 0 (`internal/herdr/commands.go:39-42`, +`internal/herdr/sweep.go:155-183`), and `loopdriver/herdrSweep` echoes exactly +the last three lines regardless of rc (`internal/loopdriver/dispatch.go:214-219`). +That string appears **359 times across the 242 retained production iteration +logs**, and because the failure path is indistinguishable from the clean path, +none of those 359 lines proves anything: the sweep ran, or the sweep could not +run. + +**Status mixes projects.** `devagent status` on a clean clone printed the +correct "● not started" card and then ~25 event rows belonging to *other* +repos, because `runStatusHuman` tails `$HOME/.devagent/runs/*.jsonl` +(`internal/cli/actions_status.go:125-133`), a process-global directory with +2 712 files and no repo column. Same global-scope class for +`~/.devagent/locks/*` (`doctor.go:535-601` reported other tasks' stale locks +on a pristine checkout). + +## 4. Two config systems, one binary + +`devagent.json` is the product's config surface (`internal/config`, written by +`init`/`create`); the selfbuild driver is **env-only** +(`loopdriver.ConfigFromEnv`, `internal/loopdriver/config.go:274-331` — no +config-file read anywhere in the package). The consequences are concrete: + +| Setting | File path | Env path | Effect today | +|---|---|---|---| +| worker | `devagent.json:worker` | `SELFBUILD_WORKER` (default `omp`) | the TUI and the loop can run different workers | +| model | `devagent.json:model` | `SELFBUILD_MODEL` (default `""`) | **the gates disagree**: `devagent preflight` resolves the model from `config.Load(repo)` (`internal/cli/actions_gates.go:180-190`), while dispatch pins `--model` only when `SELFBUILD_MODEL` is set (`internal/loopdriver/dispatch.go:475-476`). The probe green-lights one model, the workers run another | +| herdr session | `devagent.json:herdr.session` | `DEVAGENT_HERDR_SESSION` (`internal/herdr/herdr.go:99-109`), and `ResolveSession("")` also exists (`internal/config/config.go:594-602`) | two resolvers with the same precedence, no shared entry point | +| daemon | no config key at all — the TUI default is the hardcoded `http://127.0.0.1:7788` (`internal/tui/tui.go:45-46`) | `--port` flag (`internal/daemon/daemon.go:61-63`, nil → 7788), `DEVAGENT_DAEMON_TOKEN` (`:457`), `DEVAGENT_HOME` (`:481`) | port is a flag, token is an env, URL is a compiled-in default; the driver reads none of them | + +The 27 distinct `SELFBUILD_*` knobs the driver reads (`internal/loopdriver/config.go`, +measured by enumeration) are fully documented (`docs/SELF-BUILD-LOOP.md` +§"Configuration", :144-163) and still require a shell. And the surface leaks: +`os.Environ()` is forwarded whole to every dispatch +(`internal/loopdriver/dispatch.go:34-38`, which only adds +`DEVAGENT_VISIBILITY`), so a stale `GITHUB_TOKEN` in the driver's shell lands +inside every worker pane — the `env -u GITHUB_TOKEN` workaround every operator +has memorised for `gh` is a symptom of this. + +The single highest-leverage cleanup is the identity one: the driver's default +`DevagentBin` is the bare name `devagent` (`config.go:234-235`, pinned by +`config_test.go:9,25`), so `preflight`, `pane-run`, `task`, `sync-docs`, +`herdr-sweep`, `ledger --clusters`, `scan-text` and `page-degrade-breach` are +all shells out to whatever `devagent` is on `PATH`. In my `/tmp` experiment the +fresh clone's driver called **the installed binary of a different checkout** +(`command -v devagent` → `~/.local/bin/devagent` → the main worktree). The +in-flight `up` fixes this for the path it starts +(`SELFBUILD_DEVAGENT_BIN=`, `internal/commands/updown.go:~281`) +but `make loop-start`, a bare `devagent loop`, and a hand-made `hub` job all +keep the trap. + +Two residue items in the same category. (a) `devagent create` *is* fixing up +right now: the retired Node argv (`devagentBin := +filepath.Join(opts.RepoPath,"dist","src","cli.js")`, `internal/pipeline/create.go:164` +at HEAD, used at :171/:178, plus `scripts/build-loop.sh` at :183/:195) is +replaced by the self-executable in today's working tree — but the builder still +forces `KeepAlive=true` (`create.go:148`) for every agent it writes, which is +the restart policy the repo retired in #321 for a halting driver. `create +--dry-run` itself is honest (it needs `--repo`; measured plan-only output). +(b) `devagent loop` does not accept `--repo` at all (`Error: unknown flag: +--repo`, measured) while the handler uses `os.Getwd()` +(`internal/cli/actions_loop.go:29`), so pointing the factory at another repo +means `cd`-ing into it and losing the `make`-target muscle memory; and +`daemon --repo` *is* honored (`actions_serve.go:237-239`), so the two +long-running commands disagree about their own flags. + +## 5. What is already good (build on this, don't rebuild it) + +- **`up` is the right shape and it already exists.** `devagent up` / `down` / + `prd-intake` are registered (`internal/cli/root.go:474-476`, + `internal/cli/actions_updown.go`, `internal/commands/updown.go`) with a + 469-line hermetic suite (`updown_test.go`): checks → dirs → intake → daemon → + driver, own-session detach (`updown_unix.go`), pid-recorded stop, the + loop-lock holder outranking its own pid file, `--dry-run`/`--json`/ + `--foreground`/`--no-daemon`, and plain-language hints. It also pins + `SELFBUILD_DEVAGENT_BIN` to `os.Executable()` (`updown.go:292`), which is the + identity fix §4 asks for. +- **The gates already know how to explain themselves.** `preflight` writes a + structured `operator-degraded` row, exempts the omp startup wedge from opening + the shared circuit, and pages only after 3 consecutive breaches + (`internal/cli/actions_gates.go:149-232`, + `internal/resilience/preflight-gate.go:154-175`); `status --providers` counts + the degraded window (`actions_status.go:417-460`); `sync-docs` refuses with + the blocking file named and exit 2 (`internal/git/doc_sync.go:186-241`). The + plumbing for "why did it stop" exists — it just never reaches the loop's + terminal paths (§3). +- **Sweep hygiene is session-scoped already** (`herdr-sweep --session`, + `internal/herdr/sweep.go:301-310`, with the 2026-09-13 exemption ordering at + `:105-120`) — but it has no *repo* scope, and `--orphans` from the loop is + invoked without either (`dispatch.go:214-219`). +- `Makefile` `loop-start/stop/status/log` and `daemon-start/stop` exist with + binary-presence and already-running guards (Makefile:49-73, :76-…): the right + shape, minus the env that makes the driver trustworthy, and with a + `pkill -f` stop (:60, :149) that can reach another checkout's driver. + +## 6. The failure taxonomy (what unattended running actually dies of) + +Production data, 21 days, 321 ledger rows, 250 iteration logs. Statuses: +`provider-degraded 65 · ok 62 · operator-degraded 55 · failed 49 · pr-open 21 · +invalid 21 · failed-tests 14 · no-pr 11 · operator-diverged 10 · skipped 9 · +merged 3 · push-failed 1`. Throughput was real — 97 merged `devagent/TASK-*` +branches in the window — at a median iteration of **28.3 min** (p90 141 min). + +| # | Class | Evidence | Fix shape | +|---|---|---|---| +| F1 | Inherited starvation blocks every start | §2; 15-row non-productive shared tail | scope the streak to the driver's own rows + a `resume` marker | +| F2 | Operator pause reads as a fault | 55 × `operator-degraded: doc-sync deferred: PRD locally modified`, ~1 spin/73 s (49 rows on 09-12 alone); gate at `run.go:232-238`; still live today | the refusal already names the blocking file and exits 2 (`internal/git/doc_sync.go:186-241`, mapped to `operator-degraded` at `run.go:675-679`) — surface that text in `up`/`status` instead of only in a gitignored log; there is no `sync-docs --dry-run` (flags: `--branch/--json/--repo`) | +| F3 | Provider/model mismatch | 65 × `provider-degraded`, 50 with `preflight: provider probe failed`; 71 `operator-degraded` events whose detail is a raw session line (the probe's answer-shape parsing, not an outage); §4 model split | one model source of truth + the probe result echoed per iteration | +| F4 | Empty deterministic lane | 35 × `no open selfbuild issue found — falling back to LLM selection`; queue = 54 done / 1 failed / **0 pending** today | (#355) + make the empty lane a distinct ledger detail, which #355 already asks for | +| F5 | Intake has no input | `up --dry-run` on this repo reports `0 open PRD item(s)`; `git show HEAD:docs/PRD.md \| grep -c '^- \[ \]'` → **0** | convert the PRD's actionable backlog lines to `- [ ]` (content work, no code) | +| F6 | Dispatch timeouts, then blind fallback | 25 × `pane-run dispatch failed (rc=124) — falling back to direct dispatch` (`internal/loopdriver/run.go:697-705` — a non-zero `pane-run` rc logs one line and re-runs the same prompt *directly*, doubling the wall budget); 46 × circuit breaker; 49 × `task failed` | make the fallback a reported, budgeted event (ledger detail), not a log line in a gitignored file | +| F7 | Pane/orphan leak | 31 × `closed … reason=agent-idle`. The liveness probe **is** session-scoped now (`sweep.go:301-310` passes `--session`; the 2026-09-13 comment at `:105-120` records the fix and the new exemption ordering), so the #317 blind spot is closed — but nothing scopes a *repo*: `herdr-sweep --help` offers only `--dry-run/--orphans/--session` (measured) and the loop runs bare `herdr-sweep --orphans` (`dispatch.go:214-219`), so one checkout's sweep still acts on another checkout's panes (#354) | add a repo/ownership scope to `herdr-sweep` and pass it from the loop | +| F8 | Cross-checkout contamination | #354; and `SELFBUILD_HOME` doesn't isolate the driver (§2) | quarantine: refuse a non-primary checkout, `--repo`-scope the sweep, per-row provenance | +| F9 | Global `~/.devagent` mixing | §3 | key `runs/` + `locks/` by repo hash; keep a global index for `status` with an explicit `--all-repos` | +| F10 | Node-era plist/argv | §4(a): argv fixed in today's tree, `KeepAlive=true` still forced (`create.go:148`) | self-executable argv (done), `KeepAlive {SuccessfulExit=false}` for the loop, no restart-resurrect of an intentional halt | + +## 7. The design (in the order that buys the most trust per line) + +**D1 — Make the stop explain itself, and make resuming safe. *(~60 lines)*** +Add a terminal-halt row to the ledger: `{"event":"loop-halt","loop":N,"reason": +"starved|circuit-breaker|max-iterations|lock-held","detail":…}`, written by +`RunLoop` at each `haltExit` site, plus a matching `haltReason` field in +`heartbeat.json` so `devagent status` / the TUI header / `GET /status` show +*"loop 361 halted: starved after 15 non-productive iterations (newest: +invalid)"* instead of a stale "task" phase. Pair it with a stamp in the +`loop-result` rows — `origin:` + `repo:` — so `EvaluateStarvation` can +count only this checkout's streak (F1) and #354's provenance complaint dies with +it. `devagent up --resume` appends a `loop-resume` row that breaks the streak +explicitly; nothing about the gate becomes silent again. + +**D2 — One model/worker truth: read the file, keep env as override. *(~120 lines)*** +`loopdriver.ConfigFromRepo(repo)` = `config.Load(repo)` merged *under* +`SELFBUILD_*`, with `DevagentBin` defaulting to `os.Executable()` (env still +wins) and `preflight`/dispatch taking the same resolved pair. Echo the resolved +pair in the iteration log so a mismatch is visible in the artifact that +survives (`[gate] worker=omp model=onegw/free bin=`). This is what +turns "export 14 variables from a table in a doc" into "edit `devagent.json`". + +**D3 — Finish `up` as a health receipt, not a spawn receipt. *(~80 lines)*** +After starting the driver, `up` waits a bounded window and asserts the three +things that actually prove life: lock holder pid alive, `heartbeat.json` +advanced (mtime or iteration), and no `loop-halt` row. Report `ok=false` with +the halt reason when they don't. Then close the two gaps in the gate: +`up`'s prerequisite step (`RunInit`) never checks herdr or the daemon — so it +green-lit a factory whose visible-worker path was dead — and nothing in the +product creates the herdr session it assumes (the only mention is a doctor +*hint*, `doctor.go:463`). Either `up` provisions it (`herdr session create +--session `, or falls back to `visibility=headless` and says so), or +`up` must fail with that hint. Note the control plane offers no help here: the +daemon has **no** loop start/stop route at all (its surface is +`/status,/events,/agents,/dispatch,/approve,/history,/healthz`, +`internal/daemon/daemon.go:301-352`), so `up`'s own spawn path is the only +lifecycle code and must be the one that is right. + +**D4 — Quarantine by default. *(~40 lines, mostly the #354 ask)*** +Refuse to drive from a linked worktree (`git rev-parse --git-common-dir` vs +`--git-dir`), give `herdr-sweep` a repo scope — it has none today, only +`--dry-run/--orphans/--session` (measured `--help`) — and pass it from the loop, +make `--dry-run` skip `record()` +and the state push (it currently appends an `ok`-status `(dry-run)` row, +`run.go:395-397`, which then satisfies the Q27 already-shipped guard and can +close an issue with nothing landed — #354), and stop forwarding a stale +`GITHUB_TOKEN` into panes. + +**D5 — Make the lane fillable, and say when it's empty. *(content + ~20 lines)*** +F4/F5 are the "long runtime, little value" half: intake is a no-op because this +repo's PRD contains zero checkboxes, and the queue holds zero pending items. +Convert the PRD's actionable items to `- [ ]` in the same change that ships +intake, keep #355's distinct `queue-empty` ledger detail, and have `up` print +`lane: 0 issues labeled selfbuild, 0 pending queue rows, 0 open PRD checkboxes` +— the one number that predicts whether the next 10 hours produce code. + +**D6 — Stop the false all-clears. *(~30 lines)*** +`herdr-sweep` must return the CLI error (`server_not_running` ≠ clean), the +loop's sweep must pass `--session`/`--repo`, and `status`'s recent-runs must be +repo-scoped. Each is a one-line behavior change with a test that a *real* herdr +error path can satisfy — the current fakes strip `--session` from argv, which +is why no suite can see F7/D4 today. + +**D7 — Delete the second runbook. *(docs)*** +`make loop-start` and `devagent up` must not coexist as the entry point: keep +the make targets as thin wrappers (`devagent up` / `devagent down`) so the +muscle memory survives, retire the `pkill -f` stop (Makefile:60, :149) in favor +of the pid-recorded stop, fix the plist argv (F10), and replace the +`SELFBUILD_*` table in §"Configuration" with "`up` reads `devagent.json`; +`SELFBUILD_*` overrides it" plus a short "knob reference" sub-list. + +## 8. Definition of done for "easy to run locally" + +A single scriptable check, runnable on any machine, that fails today and must +pass after D1–D3: + +```bash +git clone https://github.com/FreePeak/devagent /tmp/dvx && cd /tmp/dvx +make build && ./devagent-go up --dry-run --json # 1: no env, exit 0, reports lane depth +./devagent-go up --json # 2: within 90 s the driver holds the lock, + # heartbeat advanced, no loop-halt row +./devagent-go status # 3: names this repo's state, not ~/.devagent's + # aggregate; a halted loop says WHY +./devagent-go down # 4: no loop/daemon pid survives, no pane leaks +``` + +Criterion 2 is the one to hold every design to: **green `up` must mean a +running factory.** Everything in §6 is either a way to break that promise or a +way to hide that it broke. + +## 9. What this analysis does *not* propose + +No new supervisor (launchd/hub/nohup already cover it; the loop intentionally +exits 0 on purpose and `devagent supervision` already reports the mode truth +correctly, `Makefile:66-73`), no loop control inside the daemon (it has no +start/stop route today and adding one duplicates `up`), no new config format, +no queue service, no TUI work beyond reading the halt reason, and no attempt to +make `scripts/*.sh` ergonomic — those five scripts are a separate convergence +decision already tracked as #365. + +## 10. State of play at the end of this pass (2026-09-14 ~14:40 local) + +The work analysed here is landing *while* this note is written. In the working +tree at the time of writing (all uncommitted, one `omp` session driving it): +`internal/commands/updown.go` + `updown_unix.go` + `updown_other.go` + +`updown_test.go` (469 lines), `internal/cli/actions_updown.go`, +`actions_prdintake.go`, `internal/prdintake/` + tests, +`internal/loopdriver/intake.go` + tests, `create.go`'s plist argv fix, and the +`README.md` / `docs/SELF-BUILD-LOOP.md` one-command runbook. So **D3 (spawn +half), F5's mechanism, F10's argv, and the second-runbook doc problem are in +flight**; what this pass adds to that plan is the measured list the WIP does +not yet cover: **F1 (inherited starvation halt), D1 (halt reason + resume seam), +D3's verify-after-start and herdr provisioning, F3's model two-truths, F9 +(global `~/.devagent`), and the false `[devagent] no stale panes` in §3.** + +Reproduce the headline result in one minute: clone, `make build`, run +`devagent up --no-daemon` in the clone, then `cat .selfbuild/logs/loop-*.log`. +Nothing from this pass was left running: no loop, no daemon, no worker, no +sandbox process; port 7788 is closed; the real `selfbuild/state` branch on +GitHub is untouched (tip `85add7a`, verified after the experiments, which wrote +only to a `/tmp` mirror). +## Related + +- Issues: #371 (`up`/`down` — in flight in the working tree as of this pass), + #370 (PRD checkbox intake), #355 (empty lane), #354 (quarantine — D4 is its + fix), #350/#352 (pane liveness, revision stamping), #365 (shell residue), + #362 (stale readiness doc), #286 (dry-run side effects). +- Docs: `docs/SELF-BUILD-LOOP.md` (protocol + knob table), `README.md` §"Run + the self-build loop manually", `docs/HERDR.md`, `docs/TUI.md`, + `docs/GROK.md` §2.3/§3/§5 (the earlier F1–F7 install-friction list this pass + supersedes for the loop path), PRD §21 FR-SIMPLE and §20.1. +- Data: `.selfbuild/ledger.jsonl` (321 rows), `.selfbuild/logs/loop-*.log` + (250 files), `.devagent/runs/orchestration/events.jsonl` (6 203 rows, 120 + `operator-degraded` probes classified in §6). \ No newline at end of file diff --git a/internal/cli/actions_prdintake.go b/internal/cli/actions_prdintake.go new file mode 100644 index 00000000..c10638f9 --- /dev/null +++ b/internal/cli/actions_prdintake.go @@ -0,0 +1,79 @@ +package cli + +// actions_prdintake.go wires `devagent prd-intake` (issue #370): the one +// sanctioned route from docs/PRD.md into the loop's deterministic lane. +// +// The PRD stays a state document — prose, blockquotes and completed +// checkboxes are never work — but an OPEN checkbox (`- [ ] …`) is an +// explicit operator request, and until now nothing read it: an empty lane +// made the driver fall back to LLM self-selection and ship loop plumbing +// (issue #355). Intake is deterministic (no LLM, no network) and idempotent +// (a row's queue id is a hash of its text), so it also runs unattended at +// the head of every loop iteration. + +import ( + "encoding/json" + "fmt" + "os" + + "github.com/FreePeak/devagent/internal/prdintake" + "github.com/spf13/cobra" +) + +func newPrdIntakeCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "prd-intake", + Short: "Queue the operator's open `- [ ]` items in docs/PRD.md as loop work (issue #370): deterministic, idempotent, no LLM", + RunE: func(cmd *cobra.Command, args []string) error { + repo, _ := cmd.Flags().GetString("repo") + if repo == "" { + repo, _ = os.Getwd() + } + dryRun, _ := cmd.Flags().GetBool("dry-run") + asJSON, _ := cmd.Flags().GetBool("json") + maxItems, _ := cmd.Flags().GetInt("max") + + report, err := prdintake.Ingest(prdintake.Options{ + RepoPath: repo, + MaxItems: maxItems, + DryRun: dryRun, + }) + if err != nil { + fmt.Fprintf(os.Stderr, "prd-intake: %v\n", err) + setExitCode(1) + return nil + } + if asJSON { + blob, jerr := json.MarshalIndent(report, "", " ") + if jerr != nil { + return jerr + } + fmt.Println(string(blob)) + return nil + } + verb := "queued" + if dryRun { + verb = "would queue" + } + fmt.Printf("PRD %s: %d open item(s), %s %d, already known %d\n", + report.PRDPath, report.Open, verb, len(report.Queued), len(report.Known)) + for _, it := range report.Queued { + fmt.Printf(" + %s %s\n", it.ID, it.Title) + } + for _, it := range report.Known { + fmt.Printf(" = %s %s\n", it.ID, it.Title) + } + if report.Open == 0 { + fmt.Println("Nothing to build: write `- [ ] ` anywhere in docs/PRD.md and commit it — the next iteration picks it up.") + } else { + fmt.Printf("Queue depth: %d. Next: `devagent up` (or `devagent loop`) builds the top of the queue.\n", report.QueueDepth) + } + return nil + }, + } + cmd.Flags().String("repo", "", "repository whose docs/PRD.md is ingested (default cwd)") + cmd.Flags().Bool("dry-run", false, "report what would be queued without writing queue rows") + cmd.Flags().Bool("json", false, "machine-readable report") + cmd.Flags().Int("max", 0, fmt.Sprintf("cap new rows per pass (0 = default %d, negative = unbounded)", prdintake.DefaultMaxItems)) + return cmd +} diff --git a/internal/cli/actions_scout.go b/internal/cli/actions_scout.go index 5ca63fe8..37124755 100644 --- a/internal/cli/actions_scout.go +++ b/internal/cli/actions_scout.go @@ -1,9 +1,12 @@ package cli import ( + "context" "encoding/json" "fmt" "os" + "os/signal" + "syscall" "time" "github.com/FreePeak/devagent/internal/scout" @@ -11,11 +14,14 @@ import ( "github.com/spf13/cobra" ) -// scoutCommand mirrors `devagent scout` for the read-only surface the scout -// package ports (--replay: replay captured worker-output fixtures through -// extractScoutPayload and diff against golden.json). Live dispatch -// (runScoutOnce / runScoutLoop) depends on the queue + worker runtime — -// FR-GO-04 #194 — so every other mode keeps the exit-3 not-ported contract. +// scoutCommand mirrors `devagent scout`: --replay replays the captured +// worker-output fixtures through extractScoutPayload and diffs against +// golden.json; every other mode runs the live FR-SCOUT-01 cycle +// (scout.RunOnce / scout.RunLoop). The exit-3 not-ported stub retired with +// FR-GO-04 #194 — the LaunchAgent (`com.devagent.scout`, installed by +// `devagent create --scout`) launches `devagent scout --repo … --interval …` +// 24/7, and until this wiring landed those launches died silently on exit 3 +// while the queue writer starved (2026-09-14 audit). func scoutCommand() *cobra.Command { return &cobra.Command{ Use: "scout", @@ -23,7 +29,7 @@ func scoutCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { replay, _ := cmd.Flags().GetBool("replay") if !replay { - return ¬PortedError{msg: "devagent scout: not yet ported to Go (FR-GO #192) — only --replay is wired; live dispatch needs the queue + worker runtime (FR-GO-04 #194)"} + return runScoutLive(cmd) } results, err := scout.ReplayScoutFixtures("") if err != nil { @@ -52,6 +58,51 @@ func scoutCommand() *cobra.Command { } } +// runScoutLive wires the frozen scout flag surface (--repo --worker +// --interval --timeout --once --dry-run) onto the cycle. Units match +// scripts/install-scout-launchagent.sh: --interval and --timeout are +// MINUTES. A bare run (neither --once nor --interval) performs ONE cycle, +// not a foreground daemon: an operator typing `devagent scout` must not be +// surprised into a Ctrl+C-or-SIGHUP session, while the LaunchAgent always +// passes --interval explicitly. --dry-run dispatches nothing (no AI call, +// docs/SCOUT.md) and prints the prompt length + what it would enqueue. +func runScoutLive(cmd *cobra.Command) error { + repo, _ := cmd.Flags().GetString("repo") + if repo == "" { + repo, _ = os.Getwd() // same cwd fallback as scout-status + } + worker, _ := cmd.Flags().GetString("worker") + interval, _ := cmd.Flags().GetInt("interval") + timeout, _ := cmd.Flags().GetInt("timeout") + once, _ := cmd.Flags().GetBool("once") + dryRun, _ := cmd.Flags().GetBool("dry-run") + + opts := scout.RunOptions{RepoPath: repo, Worker: worker, DryRun: dryRun} + if timeout > 0 { + opts.Timeout = time.Duration(timeout) * time.Minute + } + if once || interval <= 0 { + res, err := scout.RunOnce(opts) + if err != nil { + fmt.Fprintf(os.Stderr, "scout: %v\n", err) + setExitCode(1) + return nil + } + fmt.Printf("[scout] %s %s\n", res.Status, res.Detail) + if !res.OK { + setExitCode(1) + return nil + } + setExitCode(0) + return nil + } + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + scout.RunLoop(repo, time.Duration(interval)*time.Minute, ctx.Done(), opts) + setExitCode(0) + return nil +} + // jsJSONString mirrors JSON.stringify of a string|null: quoted JSON string // or the literal null. func jsJSONString(s *string) string { diff --git a/internal/cli/actions_updown.go b/internal/cli/actions_updown.go new file mode 100644 index 00000000..b1f71c4e --- /dev/null +++ b/internal/cli/actions_updown.go @@ -0,0 +1,120 @@ +package cli + +// actions_updown.go wires `devagent up` / `devagent down` (issue #371): the +// lifecycle surface for the automated workflow driver. The logic — the +// prerequisite gate, the docs/PRD.md lane seeding, the detached start, the +// pid-recorded stop — lives in internal/commands; this file only turns flags +// into options and the report into output. + +import ( + "encoding/json" + "fmt" + "os" + "time" + + "github.com/FreePeak/devagent/internal/commands" + "github.com/spf13/cobra" +) + +func newUpCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "up", + Short: "Bring the factory up: check prerequisites, queue the operator's docs/PRD.md items, start the self-build driver (issue #371)", + RunE: func(cmd *cobra.Command, args []string) error { + repo, _ := cmd.Flags().GetString("repo") + if repo == "" { + repo, _ = os.Getwd() + } + foreground, _ := cmd.Flags().GetBool("foreground") + skipChecks, _ := cmd.Flags().GetBool("skip-checks") + dryRun, _ := cmd.Flags().GetBool("dry-run") + noDaemon, _ := cmd.Flags().GetBool("no-daemon") + maxIntake, _ := cmd.Flags().GetInt("max-intake") + waitSecs, _ := cmd.Flags().GetInt("wait") + scoutLane, _ := cmd.Flags().GetBool("scout") + scoutInterval, _ := cmd.Flags().GetInt("scout-interval") + asJSON, _ := cmd.Flags().GetBool("json") + + daemon := !noDaemon + res, err := commands.RunUp(commands.UpOptions{ + RepoPath: repo, + Daemon: &daemon, + Foreground: foreground, + SkipChecks: skipChecks, + DryRun: dryRun, + MaxIntakeItems: maxIntake, + // --wait 0 = skip the health proof entirely (a negative + // window is RunUp's "not asked to watch"). + HealthWindow: time.Duration(waitSecs) * time.Second, + Scout: scoutLane, + ScoutIntervalMinutes: scoutInterval, + Stdout: os.Stdout, + Stderr: os.Stderr, + }) + if err != nil { + return err + } + if asJSON { + blob, jerr := json.MarshalIndent(res, "", " ") + if jerr != nil { + return jerr + } + fmt.Println(string(blob)) + } else { + commands.RenderUpReport(res, func(s string) { fmt.Println(s) }) + } + if !res.OK { + setExitCode(1) + } + return nil + }, + } + cmd.Flags().String("repo", "", "repository to drive (default cwd)") + cmd.Flags().Bool("foreground", false, "run the driver in this terminal instead of detaching it") + cmd.Flags().Bool("skip-checks", false, "start even when a required prerequisite check fails") + cmd.Flags().Bool("no-daemon", false, "do not bring up the control-plane daemon") + cmd.Flags().Int("max-intake", 0, "cap how many docs/PRD.md items are queued by this run (0 = default)") + cmd.Flags().Bool("scout", false, "also run the 24/7 researcher (devagent scout --interval) as a second child (FR-SCOUT-01)") + cmd.Flags().Int("scout-interval", 0, "minutes between scout cycles (0 = config scout.intervalMinutes, else 30)") + cmd.Flags().Int("wait", 15, "seconds to prove the driver is alive (loop lock + heartbeat) before up reports success; 0 = skip the proof") + cmd.Flags().Bool("dry-run", false, "print the plan; write and start nothing") + cmd.Flags().Bool("json", false, "machine-readable report") + return cmd +} + +func newDownCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "down", + Short: "Stop the self-build driver and daemon `devagent up` started (pid-recorded; never a pattern kill)", + RunE: func(cmd *cobra.Command, args []string) error { + repo, _ := cmd.Flags().GetString("repo") + if repo == "" { + repo, _ = os.Getwd() + } + dryRun, _ := cmd.Flags().GetBool("dry-run") + asJSON, _ := cmd.Flags().GetBool("json") + + res, err := commands.RunDown(commands.UpOptions{RepoPath: repo, DryRun: dryRun}) + if err != nil { + return err + } + if asJSON { + blob, jerr := json.MarshalIndent(res, "", " ") + if jerr != nil { + return jerr + } + fmt.Println(string(blob)) + } else { + commands.RenderDownReport(res, func(s string) { fmt.Println(s) }) + } + if !res.OK { + setExitCode(1) + } + return nil + }, + } + cmd.Flags().String("repo", "", "repository whose driver and daemon should stop (default cwd)") + cmd.Flags().Bool("dry-run", false, "report what would be signalled") + cmd.Flags().Bool("json", false, "machine-readable report") + return cmd +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 63acb77a..453d429e 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -467,6 +467,14 @@ func wiredCommands() map[string]*cobra.Command { // Curator follow-through (issue #202 / PRD Q15 wiring). "prd-audit": prdAuditCommand(), + // Lifecycle family (issue #371): `up` brings the driver + daemon up + // after checking prerequisites and seeding the lane from docs/PRD.md; + // `down` stops what `up` recorded. `prd-intake` (#370) is the same + // intake step run by hand. + "up": newUpCmd(), + "down": newDownCmd(), + "prd-intake": newPrdIntakeCmd(), + // Driver-validation family (§23): doctor (FR-VAL-02, issue #290), // supervision (issue #321 — the loop's resolved supervisor policy, // also surfaced by doctor and `make loop-status`). diff --git a/internal/commands/updown.go b/internal/commands/updown.go new file mode 100644 index 00000000..288d87a2 --- /dev/null +++ b/internal/commands/updown.go @@ -0,0 +1,898 @@ +package commands + +// updown.go is `devagent up` / `devagent down` (issue #371): one command to +// bring the automated workflow driver up, one to take it down. +// +// Before this, starting the factory meant assembling tribal knowledge: the +// SELFBUILD_* exports are read from the environment only +// (internal/loopdriver/config.go:245), DevagentBin defaults to a bare +// `devagent` a Go-only checkout never installed on PATH, `make loop-start` +// needs `./devagent-go` built first and knows nothing about the work lane, +// and an empty lane makes the driver ship loop plumbing instead of product +// (issue #355). `up` collapses all of it — check, seed, start, report — and +// `down` stops exactly what was recorded, by pid, never a blind pattern +// kill. + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "time" + + "github.com/FreePeak/devagent/internal/config" + + "github.com/FreePeak/devagent/internal/prdintake" + "github.com/FreePeak/devagent/internal/queue" +) + +// DefaultDaemonAddr is where the FR-CTRL control-plane daemon listens, the +// TUI's default endpoint too (internal/tui DefaultDaemonURL). +const DefaultDaemonAddr = "127.0.0.1:7788" + +// defaultHealthWindow is how long `up` watches the driver it just started +// before calling the start good (issue #371). Fifteen seconds clears the +// driver's own startup sequence (state pull, lock acquisition, first +// heartbeat write) on a warm machine without turning `up` into a supervisor. +const defaultHealthWindow = 15 * time.Second + +// driverHeartbeatPath is the loop's own liveness record (internal/loopdriver +// writeHeartbeat). +func driverHeartbeatPath(repo string) string { + return filepath.Join(repo, ".selfbuild", "heartbeat.json") +} + +// UpOptions configures one `up` run. Every function field is a +// hermetic-test seam; nil runs the real thing. +type UpOptions struct { + // RepoPath is the checkout to drive; "" = cwd. + RepoPath string + // Daemon also brings up the control-plane daemon (the TUI's data + // source). Unset means yes; only --no-daemon clears it. + Daemon *bool + // Foreground runs the driver in this terminal (stdio inherited, `up` + // waits for it) instead of detaching it. + Foreground bool + // Scout also runs the 24/7 research lane (`devagent scout --interval N`, + // FR-SCOUT-01) as a second detached child, which is the other half of + // keeping the lane filled: PRD intake reads what the operator wrote, the + // scout reads what the repo still needs. Off by default because each + // cycle spends worker tokens; `config.scout.enabled` is what the + // LaunchAgent route uses, so `up --scout` is the foreground-of-the-factory + // equivalent. + Scout bool + // ScoutIntervalMinutes overrides `config.scout.intervalMinutes` for the + // child `up` starts (0 = config, else 30). + ScoutIntervalMinutes int + // SkipChecks starts the driver even when a required prerequisite failed. + SkipChecks bool + // DryRun reports the plan and changes nothing: no config write, no + // intake, no spawn. + DryRun bool + // HealthWindow is how long `up` waits for proof that the driver it + // started is actually running (issue #371: a green `up` must mean a + // running factory, not a spawn receipt — a driver that dies holding the + // lock for two seconds is otherwise reported healthy). 0 = the default + // 15s; negative disables the wait. + HealthWindow time.Duration + // Sleep is the poll seam (nil = time.Sleep). + Sleep func(time.Duration) + MaxIntakeItems int + + // Checks runs the prerequisite gate (nil = RunInit, whose devagent.json + // merge is idempotent — existing choices always win). + Checks func(InitOptions) (InitResult, error) + // Intake ingests docs/PRD.md into the queue (nil = prdintake.Ingest). + Intake func(prdintake.Options) (prdintake.Report, error) + // Spawn launches one factory child. + Spawn func(Child) (ChildHandle, error) + // Alive probes pid liveness (nil = the signal-0 probe). + Alive func(pid int) bool + // PortOpen reports whether an address already accepts connections + // (nil = a bounded TCP dial). + PortOpen func(addr string) bool + // Terminate stops one recorded pid (nil = the process-group SIGTERM). + // Tests must inject it: `down` otherwise signals a live system process. + Terminate func(pid int) error + // SelfExe is the executable children launch from (nil = os.Executable), + // so `./devagent-go up` supervises `./devagent-go loop`. + SelfExe func() string + // CountIssues reads the tracker lane's depth (nil = `gh issue list + // --label selfbuild`; skipped entirely on --dry-run, which must not hit + // the network). + CountIssues func(dir string) (int, error) + // Dirty reports whether a path has uncommitted changes in the checkout + // (nil = `git diff --quiet`). A dirty state doc is the one condition + // where a healthy driver deliberately does nothing. + Dirty func(dir, path string) (bool, error) + Stdout io.Writer + Stderr io.Writer +} + +// Child is one process `up` launches. +type Child struct { + // Name keys the pid file and the report row ("loop", "daemon"). + Name string + // Bin is the executable path. + Bin string + Args []string + Dir string + LogPath string + // Env entries are merged over the parent environment (KEY=VALUE). + Env []string + Detach bool + Out io.Writer // foreground: the child's stdio goes here + ErrOut io.Writer + PidFile string +} + +// ChildHandle is a launched factory child: its pid, plus a channel closed +// when the OS reaps it. Exited is nil for a caller that cannot observe the +// child (a test seam, or a driver another route already started), in which +// case `up` falls back to a liveness probe. +// +// The channel exists because a signal-0 probe cannot see the case that +// matters: `up` is the driver's parent, so a driver that halts at its own +// gate stays a readable zombie until someone waits it, and a poll that only +// asks "is the pid alive" reports a dead factory as healthy for the whole +// window (measured on the 2026-09-14 smoke: "max iterations reached" on +// stderr, `up` still watching). +type ChildHandle struct { + Pid int + Exited <-chan struct{} +} + +// UpStep is one line of the `up` report. +type UpStep struct { + Name string `json:"name"` + OK bool `json:"ok"` + Skipped bool `json:"skipped,omitempty"` + Detail string `json:"detail"` + Hint string `json:"hint,omitempty"` +} + +// UpResult is the `up` report (`--json` emits it verbatim for the TUI). +type UpResult struct { + OK bool `json:"ok"` + RepoPath string `json:"repoPath"` + Steps []UpStep `json:"steps"` + PIDs map[string]int `json:"pids,omitempty"` + Logs map[string]string `json:"logs,omitempty"` + Intake *prdintake.Report `json:"intake,omitempty"` + NextAction string `json:"nextAction,omitempty"` +} + +// DownResult is the `down` report. +type DownResult struct { + OK bool `json:"ok"` + RepoPath string `json:"repoPath"` + Stopped []string `json:"stopped"` + Notes []string `json:"notes,omitempty"` +} + +// RunUp checks the prerequisites, seeds the work lane from docs/PRD.md, +// starts the driver, and reports what is running plus where to look. It is +// idempotent: a live driver is reported, never doubled. +func RunUp(opts UpOptions) (UpResult, error) { + repo := opts.RepoPath + if repo == "" { + repo, _ = os.Getwd() + } + out := opts.Stdout + if out == nil { + out = os.Stdout + } + errOut := opts.Stderr + if errOut == nil { + errOut = os.Stderr + } + alive := opts.Alive + if alive == nil { + alive = processAlive + } + withDaemon := opts.Daemon == nil || *opts.Daemon + + result := UpResult{OK: true, RepoPath: repo, PIDs: map[string]int{}, Logs: map[string]string{}} + step := func(name string, ok bool, detail, hint string) { + result.Steps = append(result.Steps, UpStep{Name: name, OK: ok, Detail: detail, Hint: hint}) + } + skip := func(name, detail string) { + result.Steps = append(result.Steps, UpStep{Name: name, OK: true, Skipped: true, Detail: detail}) + } + fail := func(name, detail, hint string) { + step(name, false, detail, hint) + result.OK = false + } + + // 1. Prerequisite gate. RunInit is the same check list `devagent init` + // prints (git, worker CLI, provider, credentials, docker), and its + // config merge is idempotent, so `up` needs no separate setup step. + if opts.DryRun { + skip("checks", "dry run: prerequisites not probed, devagent.json untouched") + } else { + checks := opts.Checks + if checks == nil { + checks = RunInit + } + r, err := checks(InitOptions{RepoPath: repo}) + if err != nil { + return UpResult{}, err + } + if !r.OK && !opts.SkipChecks { + for _, c := range r.Checks { + if c.OK || !c.Required { + continue + } + fail("checks", c.Name+": "+c.Detail, FailureAdvice(c.Name)) + } + result.NextAction = "fix the required check(s) above, then re-run `devagent up`" + return result, nil + } + if !r.OK { + step("checks", true, "required prerequisites failed; started anyway on --skip-checks", "") + } else { + step("checks", true, fmt.Sprintf("%d prerequisite check(s) passed; config %s", len(r.Checks), r.ConfigPath), "") + } + } + + // 2. State dirs. The driver mkdirs its own; doing it here means every + // path `up` prints exists before anything points at it. + if opts.DryRun { + skip("dirs", "dry run: nothing created") + } else { + var broken []string + for _, dir := range []string{".selfbuild/research", ".selfbuild/goals", ".selfbuild/logs", ".selfbuild/curation", ".selfbuild/run"} { + if err := os.MkdirAll(filepath.Join(repo, dir), 0o755); err != nil { + broken = append(broken, dir+": "+err.Error()) + } + } + queue.EnsureQueueDirs(repo) + if len(broken) > 0 { + fail("dirs", strings.Join(broken, "; "), "make the repo directory writable") + return result, nil + } + step("dirs", true, ".selfbuild state and .devagent queue directories ready", "") + } + + // 2b. A dirty state doc is the one case where a *healthy* driver does + // nothing on purpose: its currency gate skips every iteration while + // docs/PRD.md has uncommitted edits, stamping `operator-degraded` rows + // (internal/loopdriver run.go PRD gate). Checked before intake for the + // same reason: the worktree text is a draft, and a draft is not intent. + // Without this line the operator starts the factory, watches it stay busy, + // and gets no explanation. + if opts.DryRun { + skip("prd", "dry run: PRD worktree state not probed") + } else { + dirty := opts.Dirty + if dirty == nil { + dirty = gitDirty + } + if on, derr := dirty(repo, "docs/PRD.md"); derr != nil { + step("prd", true, "PRD worktree state unknown: "+derr.Error(), "") + } else if on { + fail("prd", "docs/PRD.md has uncommitted edits — the driver will skip every iteration until they land", + "commit it (`git commit -m \"docs(prd): …\" docs/PRD.md`) — intake reads the committed text, never a draft") + return result, nil + } else { + step("prd", true, "docs/PRD.md is clean and committed", "") + } + } + + // 3. Seed the lane from the committed PRD. This is the difference between a driver that ships + // product and one that ships plumbing (issue #355): the operator's open + // PRD checkboxes become queue rows before the next iteration picks. + intake := opts.Intake + if intake == nil { + intake = prdintake.Ingest + } + rep, err := intake(prdintake.Options{RepoPath: repo, MaxItems: opts.MaxIntakeItems, DryRun: opts.DryRun}) + switch { + case err != nil && os.IsNotExist(err): + skip("intake", "no docs/PRD.md to ingest") + case err != nil: + // An unreadable PRD is a lane we could not fill, not a reason to + // leave the factory down: report it and keep going. + step("intake", true, "PRD intake skipped: "+err.Error(), "fix docs/PRD.md, then `devagent prd-intake`") + default: + result.Intake = &rep + switch { + case opts.DryRun: + skip("intake", fmt.Sprintf("dry run: %d open PRD item(s), %d would queue", rep.Open, len(rep.Queued))) + case len(rep.Queued) > 0: + step("intake", true, fmt.Sprintf("queued %d operator item(s) from docs/PRD.md; queue depth %d", len(rep.Queued), rep.QueueDepth), "") + default: + step("intake", true, fmt.Sprintf("%d open PRD item(s) already queued; queue depth %d", rep.Open, rep.QueueDepth), "") + } + } + + // 3b. The lane census: the one number that predicts whether the next + // hours ship product or plumbing (issue #355 — an empty tracker and an + // empty queue leave the driver free to select itself as its own work, + // which is exactly what it has been doing). Reported before the start so + // an operator can abort instead of watching. + pending := len(queue.ListTasks(repo, queue.StatusPending)) + openItems := 0 + if result.Intake != nil { + openItems = result.Intake.Open + } + census := fmt.Sprintf("queue %d pending (%d open PRD item(s))", pending, openItems) + countIssues := opts.CountIssues + if countIssues == nil && !opts.DryRun { + countIssues = countSelfbuildIssues + } + if countIssues != nil { + if n, ierr := countIssues(repo); ierr != nil { + census += "; tracker not probed: " + ierr.Error() + } else { + census += fmt.Sprintf("; tracker %d open selfbuild issue(s)", n) + } + } + switch { + case pending == 0 && openItems == 0: + step("lane", true, census, "the driver has nothing to build and will invent work: write `- [ ] ` in docs/PRD.md (issue #355)") + default: + step("lane", true, census, "") + } + + // 4. The control-plane daemon (the TUI attaches to it). Its port is the + // liveness answer — not a process-name guess. + if withDaemon { + portOpen := opts.PortOpen + if portOpen == nil { + portOpen = tcpOpen + } + switch { + case portOpen(DefaultDaemonAddr): + skip("daemon", "already listening on "+DefaultDaemonAddr) + case opts.DryRun: + skip("daemon", "dry run: daemon not started") + default: + h, serr := launch(opts, Child{ + Name: "daemon", + Bin: selfExe(opts), + Args: []string{"daemon", "--repo", repo}, + Dir: repo, + LogPath: filepath.Join(repo, ".selfbuild", "logs", "daemon.log"), + Detach: !opts.Foreground, + Out: errOut, + ErrOut: errOut, + PidFile: pidFile(repo, "daemon"), + }) + if serr != nil { + fail("daemon", serr.Error(), "or start it by hand: devagent daemon --repo "+repo) + return result, nil + } + result.PIDs["daemon"] = h.Pid + result.Logs["daemon"] = filepath.Join(repo, ".selfbuild", "logs", "daemon.log") + step("daemon", true, fmt.Sprintf("listening on %s (pid %d)", DefaultDaemonAddr, h.Pid), "") + } + } + + self := selfExe(opts) + + // 4b. The scout lane (--scout): the researcher that proposes work between + // iterations. Same detach + pid record as the driver, so `down` stops it, + // and the same interval default the LaunchAgent installer uses. + if opts.Scout { + interval := opts.ScoutIntervalMinutes + if interval <= 0 { + interval = scoutInterval(repo) + } + switch { + case opts.DryRun: + skip("scout", fmt.Sprintf("dry run: scout not started (every %dm)", interval)) + default: + h, serr := launch(opts, Child{ + Name: "scout", + Bin: self, + Args: []string{"scout", "--repo", repo, "--interval", strconv.Itoa(interval)}, + Dir: repo, + LogPath: filepath.Join(repo, ".selfbuild", "logs", "scout.log"), + Detach: true, + PidFile: pidFile(repo, "scout"), + }) + if serr != nil { + fail("scout", serr.Error(), "or run it by hand: devagent scout --interval "+strconv.Itoa(interval)) + return result, nil + } + result.PIDs["scout"] = h.Pid + result.Logs["scout"] = filepath.Join(repo, ".selfbuild", "logs", "scout.log") + step("scout", true, fmt.Sprintf("researcher started (pid %d), one cycle every %dm", h.Pid, interval), "") + } + } + + // 5. The driver. A live holder of the loop lock outranks our own pid + // file: `make loop-start` and a hand-run `devagent loop` are both + // legitimate starts, and a second concurrent driver is the ledger-race + // class that has killed this loop before. + if lp := loopHolderPid(repo); lp != 0 && alive(lp) { + result.PIDs["loop"] = lp + skip("loop", fmt.Sprintf("already running (pid %d) — leave it alone; `devagent down` stops it", lp)) + } else if opts.DryRun { + skip("loop", "dry run: driver not started") + } else { + h, serr := launch(opts, Child{ + Name: "loop", + Bin: self, + Args: []string{"loop"}, + Dir: repo, + // The driver shells out for task/preflight/sync-docs/pane-run; + // pinning that at this exact executable is what makes + // `./devagent-go up` self-contained (its historical default + // was the bare name `devagent`, absent on a Go-only checkout). + Env: []string{"SELFBUILD_DEVAGENT_BIN=" + self}, + LogPath: filepath.Join(repo, ".selfbuild", "logs", "driver.log"), + Detach: !opts.Foreground, + Out: out, + ErrOut: errOut, + PidFile: pidFile(repo, "loop"), + }) + if serr != nil { + fail("loop", serr.Error(), "or run it in the foreground: devagent loop") + return result, nil + } + result.PIDs["loop"] = h.Pid + result.Logs["loop"] = filepath.Join(repo, ".selfbuild", "logs", "driver.log") + switch { + case opts.Foreground: + step("loop", true, fmt.Sprintf("foreground driver exited (pid %d)", h.Pid), "") + default: + proof, why := verifyDriver(opts, repo, h, alive) + if why != "" { + fail("loop", fmt.Sprintf("driver pid %d is not running: %s", h.Pid, why), + "the reason is in "+result.Logs["loop"]+"; `devagent supervision` shows who would restart it") + return result, nil + } + step("loop", true, fmt.Sprintf("driver running (pid %d) — %s", h.Pid, proof), "") + } + } + + switch { + case opts.DryRun: + result.NextAction = "nothing was changed — re-run without --dry-run to start the factory" + case opts.Foreground: + result.NextAction = "driver finished; `devagent up` starts it detached" + case result.OK: + result.NextAction = "watch: `devagent status` or `devagent tui`; logs: .selfbuild/logs/driver.log; stop: `devagent down`" + } + return result, nil +} + +// RunDown stops the factory processes `up` recorded (and a driver that took +// the loop lock by any other route). It signals recorded pids only — a +// pattern kill can reach a driver started from another checkout, which is +// the class issue #354 was filed about. +func RunDown(opts UpOptions) (DownResult, error) { + repo := opts.RepoPath + if repo == "" { + repo, _ = os.Getwd() + } + alive := opts.Alive + if alive == nil { + alive = processAlive + } + result := DownResult{OK: true, RepoPath: repo} + + // The lock record wins when present: it is what the driver itself wrote. + loopPid := loopHolderPid(repo) + if loopPid == 0 { + loopPid = readPidFile(pidFile(repo, "loop")) + } + targets := []struct { + name string + pid int + }{{"loop", loopPid}, {"daemon", readPidFile(pidFile(repo, "daemon"))}} + if p := readPidFile(pidFile(repo, "scout")); p != 0 { + targets = append(targets, struct { + name string + pid int + }{"scout", p}) + } + for _, t := range targets { + switch { + case t.pid == 0: + result.Notes = append(result.Notes, t.name+": not running (no pid recorded)") + case !alive(t.pid): + result.Notes = append(result.Notes, fmt.Sprintf("%s: pid %d is already gone", t.name, t.pid)) + _ = os.Remove(pidFile(repo, t.name)) + case opts.DryRun: + result.Notes = append(result.Notes, fmt.Sprintf("%s: dry run, pid %d left running", t.name, t.pid)) + default: + if err := stop(opts, t.pid); err != nil { + result.OK = false + result.Notes = append(result.Notes, fmt.Sprintf("%s: pid %d could not be stopped: %v", t.name, t.pid, err)) + continue + } + result.Stopped = append(result.Stopped, fmt.Sprintf("%s (pid %d)", t.name, t.pid)) + _ = os.Remove(pidFile(repo, t.name)) + } + } + return result, nil +} + +// launch runs one child through the injected seam or the real spawn path. +func launch(opts UpOptions, c Child) (ChildHandle, error) { + if opts.Spawn != nil { + return opts.Spawn(c) + } + return spawnChild(c) +} + +// stop signals one recorded pid through the injected seam when there is one. +// `down` must never pattern-match a process name: the class issue #354 is +// about a driver from another checkout sharing this repo's state, and a +// wildcard kill cannot tell the two apart. +func stop(opts UpOptions, pid int) error { + if opts.Terminate != nil { + return opts.Terminate(pid) + } + return terminateTree(pid) +} + +// spawnChild launches one factory child. Detached children get their own +// session (detachAttrs) and append into LogPath; a background reaper waits +// them so `up` learns the moment the driver exits, and `up` itself still +// returns immediately (the goroutine dies with the process, after which the +// driver is reparented and keeps running — the whole point of detaching). +// Foreground children inherit the given writers and `up` waits for them, so +// its own exit code is the driver's verdict. +func spawnChild(c Child) (ChildHandle, error) { + if c.Bin == "" || len(c.Args) == 0 { + return ChildHandle{}, fmt.Errorf("child %q needs an executable and argv", c.Name) + } + logW, errW := c.Out, c.ErrOut + if c.Detach { + f, oerr := os.OpenFile(c.LogPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if oerr != nil { + return ChildHandle{}, fmt.Errorf("open %s: %w", c.LogPath, oerr) + } + logW, errW = f, f + defer func() { _ = f.Close() }() + } + + cmd := exec.Command(c.Bin, c.Args...) + cmd.Dir = c.Dir + cmd.Env = append(os.Environ(), c.Env...) + cmd.Stdout, cmd.Stderr = logW, errW + if c.Detach { + detachAttrs(cmd) + } else { + cmd.Stdin = os.Stdin + } + if serr := cmd.Start(); serr != nil { + return ChildHandle{}, fmt.Errorf("start %s: %w", c.Name, serr) + } + h := ChildHandle{Pid: cmd.Process.Pid} + if c.PidFile != "" { + writePidFile(c.PidFile, h.Pid) + } + if !c.Detach { + if werr := cmd.Wait(); werr != nil { + return h, fmt.Errorf("%s exited: %w", c.Name, werr) + } + return h, nil + } + exited := make(chan struct{}) + h.Exited = exited + go func() { + _ = cmd.Wait() + close(exited) + }() + return h, nil +} + +// selfExe resolves the executable children are launched from. +func selfExe(opts UpOptions) string { + if opts.SelfExe != nil { + return opts.SelfExe() + } + if p, err := os.Executable(); err == nil { + return p + } + return "devagent" +} + +// pidFile is where `up` records a child pid for `down` to find. +func pidFile(repo, name string) string { + return filepath.Join(repo, ".selfbuild", "run", name+".pid") +} + +func writePidFile(path string, pid int) { + _ = os.MkdirAll(filepath.Dir(path), 0o755) + _ = os.WriteFile(path, []byte(strconv.Itoa(pid)+"\n"), 0o644) +} + +func readPidFile(path string) int { + raw, err := os.ReadFile(path) + if err != nil { + return 0 + } + n, cerr := strconv.Atoi(strings.TrimSpace(string(raw))) + if cerr != nil || n <= 0 { + return 0 + } + return n +} + +// loopHolderPid reads the driver's own mkdir-lock record — the authoritative +// answer to "is a driver already running in this repo", whoever started it. +func loopHolderPid(repo string) int { + return readPidFile(filepath.Join(repo, ".selfbuild", "loop.lock.d", "pid")) +} + +// tcpOpen answers "is something listening there" with a bounded dial. +func tcpOpen(addr string) bool { + conn, err := net.DialTimeout("tcp", addr, 300*time.Millisecond) + if err != nil { + return false + } + _ = conn.Close() + return true +} + +// RenderUpReport prints the `up` report as the plain-language checklist §21 +// asks for: one line per step, ✓/–/✗, one advice line per miss, never raw +// logs. +func RenderUpReport(r UpResult, render func(string)) { + render("devagent up — " + r.RepoPath) + for _, s := range r.Steps { + mark := "✓" + switch { + case !s.OK: + mark = "✗" + case s.Skipped: + mark = "–" + } + render(fmt.Sprintf(" %s %-7s %s", mark, s.Name, s.Detail)) + if s.Hint != "" { + render(" → " + s.Hint) + } + } + if r.Intake != nil && len(r.Intake.Queued) > 0 { + render(" built from docs/PRD.md:") + for _, it := range r.Intake.Queued { + render(fmt.Sprintf(" %s %s", it.ID, it.Title)) + } + } + for _, name := range []string{"loop", "daemon"} { + if p := r.Logs[name]; p != "" { + render(fmt.Sprintf(" log %-6s %s", name, p)) + } + } + if r.NextAction != "" { + render(" next: " + r.NextAction) + } +} + +// RenderDownReport prints `down` in the same idiom. +func RenderDownReport(r DownResult, render func(string)) { + render("devagent down — " + r.RepoPath) + for _, s := range r.Stopped { + render(" ✓ stopped " + s) + } + for _, n := range r.Notes { + render(" – " + n) + } +} + +// verifyDriver proves the detached driver is actually running before `up` +// calls the start good. +// +// Without this, `up` is a spawn receipt: it reports a pid the OS handed out +// and nothing about whether the factory is alive. A driver that halts at its +// own gate — the starvation halt, an iteration cap already reached, a state +// branch it cannot pull — exits with code 0, so no supervisor would ever +// restart it and nothing else says so either (the 2026-09-14 research pass, +// docs/research/2026-09-14-easy-local-setup-selfbuild-loop.md: "green `up` +// must mean a running factory"). Proof is the two artifacts the driver itself +// writes: it holds the loop lock, and its heartbeat names a phase. +// +// The window is bounded and the verdict is early either way: a live driver is +// reported as soon as its own heartbeat lands (usually well under a second), +// a dead one is reported with the reason, and an inconclusive one says what +// it looked for. +func verifyDriver(opts UpOptions, repo string, h ChildHandle, alive func(pid int) bool) (string, string) { + pid := h.Pid + window := opts.HealthWindow + switch { + case window < 0: + return "health proof skipped (--wait 0)", "" + case window == 0: + window = defaultHealthWindow + } + sleep := opts.Sleep + if sleep == nil { + sleep = time.Sleep + } + deadline := time.Now().Add(window) + for { + select { + case <-h.Exited: + return "", stoppedReason(repo, pid) + default: + } + if !alive(pid) { + return "", stoppedReason(repo, pid) + } + if hb, ok := readDriverHeartbeat(repo); ok && loopHolderPid(repo) == pid && hb.Pid == pid { + return fmt.Sprintf("holds the loop lock, iteration %d, phase %s", hb.Iteration, hb.Phase), "" + } + if !time.Now().Before(deadline) { + return "", fmt.Sprintf("pid %d is alive but wrote no loop lock or heartbeat within %s", pid, window) + } + sleep(100 * time.Millisecond) + } +} + +// driverHeartbeat is the subset of `.selfbuild/heartbeat.json` that proves +// life (the loop's own writeHeartbeat record). +type driverHeartbeat struct { + Iteration int `json:"iteration"` + Phase string `json:"phase"` + Pid int `json:"pid"` + UpdatedAt string `json:"updatedAt"` +} + +func readDriverHeartbeat(repo string) (driverHeartbeat, bool) { + raw, err := os.ReadFile(driverHeartbeatPath(repo)) + if err != nil { + return driverHeartbeat{}, false + } + var hb driverHeartbeat + if json.Unmarshal(raw, &hb) != nil || hb.Pid <= 0 { + return driverHeartbeat{}, false + } + return hb, true +} + +// stoppedReason names why a driver that started has already exited, so the +// operator reads a cause instead of a dead pid. The newest per-iteration log +// carries the driver's own verdict (starvation halt, breaker trip, lock +// refusal); driver.log carries only the startup banners, because the driver +// tails the iteration log into it at the END of an iteration. +func stoppedReason(repo string, pid int) string { + if tail := lastNonEmpty(logTail(repo)); tail != "" { + return "exited: " + tail + } + // A driver that halted at the loop head never opened an iteration log: + // the starvation halt, the iteration cap, and a lock refusal are all + // announced on the driver's own stdout before phase work starts. + if tail := lastNonEmpty(readDriverLog(repo)); tail != "" { + return "exited: " + tail + } + if row := lastLedgerRow(repo); row != "" { + return "exited; last ledger row: " + row + } + return fmt.Sprintf("exited before writing any state (pid %d)", pid) +} + +// readDriverLog returns driver.log (the nohup/hub stdout of the driver). +func readDriverLog(repo string) string { + raw, err := os.ReadFile(filepath.Join(repo, ".selfbuild", "logs", "driver.log")) + if err != nil { + return "" + } + return string(raw) +} + +// logTail returns the trailing lines of the newest .selfbuild/logs/loop-N.log. +func logTail(repo string) string { + dir := filepath.Join(repo, ".selfbuild", "logs") + entries, err := os.ReadDir(dir) + if err != nil { + return "" + } + newest := "" + highest := -1 + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasPrefix(name, "loop-") || !strings.HasSuffix(name, ".log") { + continue + } + n, cerr := strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(name, "loop-"), ".log")) + if cerr == nil && n > highest { + highest, newest = n, filepath.Join(dir, name) + } + } + if newest == "" { + return "" + } + raw, rerr := os.ReadFile(newest) + if rerr != nil { + return "" + } + return string(raw) +} + +func lastNonEmpty(text string) string { + lines := strings.Split(strings.TrimSpace(text), "\n") + for i := len(lines) - 1; i >= 0; i-- { + if l := strings.TrimSpace(lines[i]); l != "" { + return l + } + } + return "" +} + +func lastLedgerRow(repo string) string { + raw, err := os.ReadFile(filepath.Join(repo, ".selfbuild", "ledger.jsonl")) + if err != nil { + return "" + } + return lastNonEmpty(string(raw)) +} + +// countSelfbuildIssues reads the depth of the deterministic tracker lane: +// open issues carrying the loop's label. gh resolves the repository from the +// checkout's own origin, so no owner/repo slug is threaded through here, and +// the read is bounded — `up` must not hang on a dead network to report a lane. +func countSelfbuildIssues(dir string) (int, error) { + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "gh", "issue", "list", + "--state", "open", "--label", "selfbuild", "--limit", "200", "--json", "number") + cmd.Dir = dir + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + if err := cmd.Run(); err != nil { + if ctx.Err() != nil { + return 0, fmt.Errorf("gh issue list timed out: %w", ctx.Err()) + } + msg := strings.TrimSpace(stderr.String()) + if msg == "" { + msg = err.Error() + } + return 0, errors.New(msg) + } + var rows []struct { + Number int `json:"number"` + } + if err := json.Unmarshal(stdout.Bytes(), &rows); err != nil { + return 0, fmt.Errorf("gh issue list output unreadable: %w", err) + } + return len(rows), nil +} + +// gitDirty answers "does this path have uncommitted changes" with git's own +// exit code (`git diff --quiet -- `), the same probe the driver's PRD +// currency gate uses — so `up` and the driver can never disagree about it. +func gitDirty(dir, path string) (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "git", "diff", "--quiet", "--", path) + cmd.Dir = dir + if err := cmd.Run(); err != nil { + var ee *exec.ExitError + if errors.As(err, &ee) { + // exit 1 is git's "there IS a difference"; anything else is a + // git failure (not a repo, bad path) and must not read as dirty. + return ee.ExitCode() == 1, nil + } + if ctx.Err() != nil { + return false, fmt.Errorf("git diff timed out: %w", ctx.Err()) + } + return false, err + } + return false, nil +} + +// scoutInterval reads the researcher's cadence from devagent.json +// (`scout.intervalMinutes`), falling back to the 30-minute default +// scripts/install-scout-launchagent.sh uses so both routes tick alike. +func scoutInterval(repo string) int { + if cfg, err := config.Load(repo); err == nil && cfg.Scout != nil && cfg.Scout.IntervalMinutes != nil { + if n := int(*cfg.Scout.IntervalMinutes); n > 0 { + return n + } + } + return 30 +} diff --git a/internal/commands/updown_other.go b/internal/commands/updown_other.go new file mode 100644 index 00000000..133f4153 --- /dev/null +++ b/internal/commands/updown_other.go @@ -0,0 +1,25 @@ +//go:build !unix + +package commands + +import ( + "fmt" + "os" + "os/exec" +) + +// detachAttrs has no setsid equivalent off unix; the child inherits the +// parent's group and is reaped by pid alone (loopdriver's proc_other.go +// carries the same documented degradation). +func detachAttrs(cmd *exec.Cmd) {} + +// terminateTree stops the recorded pid. Off unix there is no process-group +// probe here, so a worker the driver spawned may outlive it; the sweep and +// the loop lock remain the backstops. +func terminateTree(pid int) error { + p, err := os.FindProcess(pid) + if err != nil { + return fmt.Errorf("find %d: %w", pid, err) + } + return p.Kill() +} diff --git a/internal/commands/updown_test.go b/internal/commands/updown_test.go new file mode 100644 index 00000000..d6b8fde9 --- /dev/null +++ b/internal/commands/updown_test.go @@ -0,0 +1,790 @@ +package commands + +import ( + "errors" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + "github.com/FreePeak/devagent/internal/prdintake" + "github.com/FreePeak/devagent/internal/queue" +) + +// upProbe records the side effects `up` would have had, so a test asserts the +// plan rather than the processes it started. +type upProbe struct { + launched []Child + pids []int +} + +func upFixture(t *testing.T) (string, UpOptions, *upProbe) { + t.Helper() + repo := t.TempDir() + probe := &upProbe{} + return repo, UpOptions{ + RepoPath: repo, + Checks: func(InitOptions) (InitResult, error) { + return InitResult{ + OK: true, + ConfigPath: filepath.Join(repo, "devagent.json"), + Checks: []PrereqCheck{{Name: "git", OK: true, Required: true}}, + }, nil + }, + Intake: func(o prdintake.Options) (prdintake.Report, error) { + return prdintake.Report{ + PRDPath: prdintake.DefaultPRDPath(o.RepoPath), + Open: 1, + Queued: []prdintake.Item{{ID: "PRD-deadbeef", Title: "ship the thing", Line: 9}}, + QueueDepth: 1, + }, nil + }, + Spawn: func(c Child) (ChildHandle, error) { + pid := 4242 + len(probe.launched) + 1 + if c.Name == "loop" { + // A real driver proves itself by taking the loop lock and + // writing its heartbeat; the fake has to, or `up`'s health + // receipt (verifyDriver) correctly refuses the start. + proveDriverAlive(t, repo, pid) + } + probe.launched = append(probe.launched, c) + probe.pids = append(probe.pids, pid) + // The real spawn records a pid file; `down` reads those, so the + // fake has to as well or the two halves never line up. + if c.PidFile != "" { + writePidFile(c.PidFile, pid) + } + return ChildHandle{Pid: pid}, nil + }, + Alive: func(pid int) bool { + for _, p := range probe.pids { + if p == pid { + return true + } + } + return false + }, + PortOpen: func(string) bool { return false }, + SelfExe: func() string { return "/tmp/devagent-go" }, + HealthWindow: 2 * time.Second, + Sleep: func(time.Duration) {}, + }, probe +} + +func (p *upProbe) child(t *testing.T, name string) Child { + t.Helper() + for _, c := range p.launched { + if c.Name == name { + return c + } + } + t.Fatalf("no %q child launched (launched %v)", name, names(p.launched)) + return Child{} +} + +// TestUpStartsDriverSeededAndDaemonized: the point of `up` is that the +// operator types one word. Checks pass, the PRD lane is seeded, and the loop +// child launches from THIS executable with its own SELFBUILD_DEVAGENT_BIN, +// detached, logging where the report says. +func TestUpStartsDriverSeededAndDaemonized(t *testing.T) { + repo, opts, probe := upFixture(t) + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if !res.OK { + t.Fatalf("up failed: %+v", res.Steps) + } + if got := names(probe.launched); strings.Join(got, ",") != "daemon,loop" { + t.Errorf("launched %v, want the daemon then the driver", got) + } + loop := probe.child(t, "loop") + if loop.Bin != "/tmp/devagent-go" || strings.Join(loop.Args, " ") != "loop" { + t.Errorf("loop child = %s %v, want the running executable running loop", loop.Bin, loop.Args) + } + if !loop.Detach { + t.Errorf("the loop child must detach so `up` returns") + } + if loop.Dir != repo { + t.Errorf("loop child dir = %q, want the repo", loop.Dir) + } + // The historical failure this replaces: the driver shells out to a bare + // `devagent` that a Go-only checkout never installed on PATH. + if len(loop.Env) != 1 || loop.Env[0] != "SELFBUILD_DEVAGENT_BIN=/tmp/devagent-go" { + t.Errorf("loop env = %v, want the driver pinned to this executable", loop.Env) + } + if loop.LogPath != filepath.Join(repo, ".selfbuild", "logs", "driver.log") { + t.Errorf("loop log = %q", loop.LogPath) + } + if loop.PidFile == "" { + t.Errorf("the driver must record a pid so `down` can stop it") + } + if res.Intake == nil || len(res.Intake.Queued) != 1 { + t.Errorf("intake report missing: %+v", res.Intake) + } + if res.NextAction == "" { + t.Errorf("a successful up must name the next action") + } +} + +// TestUpIsIdempotentAgainstALiveDriver: a second `up` must not start a second +// driver — two concurrent drivers sharing one ledger and one state branch is +// the race class that has killed this loop. The lock holder wins even with no +// pid file of ours on disk. +func TestUpIsIdempotentAgainstALiveDriver(t *testing.T) { + repo, opts, probe := upFixture(t) + lockDir := filepath.Join(repo, ".selfbuild", "loop.lock.d") + if err := os.MkdirAll(lockDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(lockDir, "pid"), []byte("777\n"), 0o644); err != nil { + t.Fatal(err) + } + opts.Alive = func(pid int) bool { return pid == 777 } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + for _, c := range probe.launched { + if c.Name == "loop" { + t.Fatalf("a live driver was double-started: %v", names(probe.launched)) + } + } + if res.PIDs["loop"] != 777 { + t.Errorf("report loop pid = %d, want the live holder 777", res.PIDs["loop"]) + } + var step *UpStep + for i, s := range res.Steps { + if s.Name == "loop" { + step = &res.Steps[i] + } + } + if step == nil { + t.Fatalf("no loop step reported: %+v", res.Steps) + } + if !step.Skipped || !strings.Contains(step.Detail, "already running") { + t.Errorf("loop step = %+v, want a skipped `already running`", step) + } +} + +// TestUpRefusesBrokenPrerequisites: starting a driver that cannot dispatch a +// worker only burns tokens. `up` stops and says what to install; +// --skip-checks is the only way past it. +func TestUpRefusesBrokenPrerequisites(t *testing.T) { + _, opts, probe := upFixture(t) + opts.Checks = func(InitOptions) (InitResult, error) { + return InitResult{ + OK: false, + Checks: []PrereqCheck{ + {Name: "git", OK: true, Required: true}, + {Name: "worker", OK: false, Required: true, Detail: "worker CLI omp not found"}, + }, + }, nil + } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if res.OK || len(probe.launched) != 0 { + t.Fatalf("driver started despite a failed required check: %+v", res.Steps) + } + var hint string + for _, s := range res.Steps { + if s.Name == "checks" { + hint = s.Hint + } + } + if !strings.Contains(hint, "install the worker CLI") { + t.Errorf("hint = %q, want the concrete fix line", hint) + } + + opts.SkipChecks = true + res, err = RunUp(opts) + if err != nil { + t.Fatal(err) + } + if len(probe.launched) == 0 { + t.Errorf("--skip-checks must still start the driver: %+v", res.Steps) + } +} + +// TestUpWithoutDaemon: --no-daemon leaves the control plane alone; the driver +// is the deliverable. +func TestUpWithoutDaemon(t *testing.T) { + _, opts, probe := upFixture(t) + noDaemon := false + opts.Daemon = &noDaemon + + if _, err := RunUp(opts); err != nil { + t.Fatal(err) + } + if got := names(probe.launched); strings.Join(got, ",") != "loop" { + t.Errorf("launched %v, want only the driver", got) + } +} + +// TestUpDryRunChangesNothing: --dry-run is the command an operator runs to +// read the plan; it must spawn nothing, mkdir nothing, queue nothing. +func TestUpDryRunChangesNothing(t *testing.T) { + repo, opts, probe := upFixture(t) + opts.DryRun = true + var sawDryRun bool + opts.Intake = func(o prdintake.Options) (prdintake.Report, error) { + sawDryRun = o.DryRun + return prdintake.Report{PRDPath: prdintake.DefaultPRDPath(o.RepoPath)}, nil + } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if len(probe.launched) != 0 { + t.Errorf("dry run launched %v", names(probe.launched)) + } + if _, err := os.Stat(filepath.Join(repo, ".selfbuild", "logs")); !os.IsNotExist(err) { + t.Errorf("dry run created state dirs") + } + if !sawDryRun { + t.Errorf("intake must run in dry-run mode") + } + if !strings.Contains(res.NextAction, "without --dry-run") { + t.Errorf("next action = %q", res.NextAction) + } +} + +// TestUpSurvivesAnUnreadablePRD: a missing docs/PRD.md is an unfilled lane, +// not a reason to leave the factory down — the driver still starts, and the +// report says why the lane is empty. +func TestUpSurvivesAnUnreadablePRD(t *testing.T) { + _, opts, probe := upFixture(t) + opts.Intake = func(prdintake.Options) (prdintake.Report, error) { + return prdintake.Report{}, os.ErrNotExist + } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if !res.OK || len(probe.launched) == 0 { + t.Fatalf("driver refused to start over a missing PRD: %+v", res.Steps) + } + for _, s := range res.Steps { + if s.Name == "intake" && !s.Skipped { + t.Errorf("intake step = %+v, want it reported as skipped", s) + } + } +} + +// TestUpPropagatesASpawnFailure: an executable that cannot start must fail +// the run with the fallback command named, never print a success card. And a +// daemon that cannot start aborts before the driver is launched — half a +// factory is worse than none, because the TUI then reports a driver with no +// control plane to talk to. +func TestUpPropagatesASpawnFailure(t *testing.T) { + _, opts, _ := upFixture(t) + opts.Spawn = func(c Child) (ChildHandle, error) { + if c.Name == "loop" { + return ChildHandle{}, errors.New("start loop: permission denied") + } + return ChildHandle{Pid: 1234}, nil + } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if res.OK { + t.Errorf("a failed spawn must fail the run: %+v", res.Steps) + } + var hint string + for _, s := range res.Steps { + if s.Name == "loop" { + hint = s.Hint + } + } + if !strings.Contains(hint, "devagent loop") { + t.Errorf("loop hint = %q, want the foreground fallback", hint) + } + + _, opts2, _ := upFixture(t) + opts2.Spawn = func(Child) (ChildHandle, error) { return ChildHandle{}, errors.New("no such file") } + res2, err := RunUp(opts2) + if err != nil { + t.Fatal(err) + } + for _, s := range res2.Steps { + if s.Name == "loop" { + t.Errorf("the driver started after the daemon failed: %+v", res2.Steps) + } + } +} + +// TestUpQueueSeedReachesDisk pins the one thing the seams cannot prove: the +// real intake path writes a claimable queue row, which is what the loop's +// phase-2a claim reads. +func TestUpQueueSeedReachesDisk(t *testing.T) { + repo, opts, probe := upFixture(t) + if err := os.MkdirAll(filepath.Join(repo, "docs"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(prdintake.DefaultPRDPath(repo), []byte("## Scope\n\n- [ ] Ship the intake lane\n"), 0o644); err != nil { + t.Fatal(err) + } + opts.Intake = nil // the real prdintake.Ingest + opts.PortOpen = func(string) bool { return true } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if !res.OK { + t.Fatalf("up failed: %+v", res.Steps) + } + rows := queue.ListTasks(repo, queue.StatusPending) + if len(rows) != 1 || rows[0].Source == nil || *rows[0].Source != "prd" { + t.Fatalf("queue rows = %+v, want one prd-sourced pending row", rows) + } + if got := names(probe.launched); strings.Join(got, ",") != "loop" { + t.Errorf("launched %v, want only the driver (the daemon port was open)", got) + } +} + +func writePid(t *testing.T, dir, name, pid string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(pid+"\n"), 0o644); err != nil { + t.Fatal(err) + } +} + +// TestDownStopsRecordedPidsOnly: `down` signals the pids it holds a record of +// and nothing else — no pattern match that could reach a driver started from +// another checkout (issue #354). +func TestDownStopsRecordedPidsOnly(t *testing.T) { + repo := t.TempDir() + runDir := filepath.Join(repo, ".selfbuild", "run") + writePid(t, runDir, "loop.pid", "500") + writePid(t, runDir, "daemon.pid", "501") + + var killed []string + res, err := RunDown(UpOptions{ + RepoPath: repo, + Alive: func(pid int) bool { return pid == 500 }, + Terminate: func(pid int) error { killed = append(killed, strconv.Itoa(pid)); return nil }, + }) + if err != nil { + t.Fatal(err) + } + if !res.OK { + t.Fatalf("down reported failure: %+v", res) + } + if strings.Join(killed, ",") != "500" { + t.Errorf("killed %v, want only the live loop pid", killed) + } + if len(res.Stopped) != 1 || !strings.Contains(res.Stopped[0], "loop (pid 500)") { + t.Errorf("stopped = %v, want the live loop only", res.Stopped) + } + if _, err := os.Stat(filepath.Join(runDir, "loop.pid")); !os.IsNotExist(err) { + t.Errorf("a stopped driver's pid file must be removed") + } + if !strings.Contains(strings.Join(res.Notes, "|"), "daemon: pid 501 is already gone") { + t.Errorf("notes = %v, want the dead daemon reported as gone", res.Notes) + } +} + +// TestDownPrefersTheLockHolder: a driver started by `make loop-start` or a +// bare `devagent loop` has no pid file of ours but does hold the lock — that +// record is the authoritative pid. +func TestDownPrefersTheLockHolder(t *testing.T) { + repo := t.TempDir() + writePid(t, filepath.Join(repo, ".selfbuild", "loop.lock.d"), "pid", "600") + writePid(t, filepath.Join(repo, ".selfbuild", "run"), "loop.pid", "601") + + var killed []string + res, err := RunDown(UpOptions{ + RepoPath: repo, + Alive: func(pid int) bool { return pid == 600 }, + Terminate: func(pid int) error { killed = append(killed, strconv.Itoa(pid)); return nil }, + }) + if err != nil { + t.Fatal(err) + } + if strings.Join(killed, ",") != "600" { + t.Errorf("killed %v, want the lock holder, not the stale pid file", killed) + } + if len(res.Stopped) != 1 || !strings.Contains(res.Stopped[0], "pid 600") { + t.Errorf("stopped = %v", res.Stopped) + } +} + +// TestDownDryRunSignalsNothing: the operator can ask what would stop. +func TestDownDryRunSignalsNothing(t *testing.T) { + repo := t.TempDir() + writePid(t, filepath.Join(repo, ".selfbuild", "run"), "loop.pid", "700") + + var killed []string + res, err := RunDown(UpOptions{ + RepoPath: repo, + DryRun: true, + Alive: func(int) bool { return true }, + Terminate: func(pid int) error { killed = append(killed, strconv.Itoa(pid)); return nil }, + }) + if err != nil { + t.Fatal(err) + } + if len(killed) != 0 || len(res.Stopped) != 0 { + t.Errorf("dry run signalled %v", killed) + } + if !strings.Contains(strings.Join(res.Notes, "|"), "dry run") { + t.Errorf("notes = %v, want the dry-run note", res.Notes) + } +} + +// TestDownReportsNothingRunning: a repo with no driver must not look broken. +func TestDownReportsNothingRunning(t *testing.T) { + res, err := RunDown(UpOptions{RepoPath: t.TempDir(), Alive: func(int) bool { return false }}) + if err != nil { + t.Fatal(err) + } + if !res.OK || len(res.Stopped) != 0 { + t.Fatalf("res = %+v", res) + } + if len(res.Notes) != 2 { + t.Errorf("notes = %v, want one per candidate", res.Notes) + } +} + +// TestDownReportsAFailedStop: a pid that refuses to die is an operator +// problem, and `down` must say so with a nonzero verdict rather than pretend. +func TestDownReportsAFailedStop(t *testing.T) { + repo := t.TempDir() + runDir := filepath.Join(repo, ".selfbuild", "run") + writePid(t, runDir, "loop.pid", "800") + + res, err := RunDown(UpOptions{ + RepoPath: repo, + Alive: func(int) bool { return true }, + Terminate: func(int) error { return errors.New("operation not permitted") }, + }) + if err != nil { + t.Fatal(err) + } + if res.OK { + t.Errorf("a refused signal must fail the run: %+v", res) + } + if _, err := os.Stat(filepath.Join(runDir, "loop.pid")); err != nil { + t.Errorf("an unstopped driver keeps its pid record: %v", err) + } +} + +// TestUpNamesAnEmptyLane: #355 is the "long runtime, little value" bug — an +// empty lane is not an emergency, it is the reason the next ten hours ship +// loop plumbing. `up` must say so, with the fix, before starting. +func TestUpNamesAnEmptyLane(t *testing.T) { + _, opts, _ := upFixture(t) + opts.Intake = func(o prdintake.Options) (prdintake.Report, error) { + return prdintake.Report{PRDPath: prdintake.DefaultPRDPath(o.RepoPath)}, nil + } + opts.CountIssues = func(string) (int, error) { return 0, nil } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + var hint, detail string + for _, s := range res.Steps { + if s.Name == "lane" { + detail, hint = s.Detail, s.Hint + } + } + if !strings.Contains(detail, "tracker 0 open selfbuild issue") { + t.Errorf("lane detail = %q, want the tracker count", detail) + } + if !strings.Contains(hint, "docs/PRD.md") || !strings.Contains(hint, "will invent work") { + t.Errorf("lane hint = %q, want the empty-lane remedy", hint) + } +} + +// TestUpExplainsAHeadOfIterationHalt: the halt paths that matter most (an +// iteration cap already reached, a starvation halt, a lock refusal) happen +// BEFORE any iteration log opens, so the reason only exists in driver.log — +// and `up` must read it there rather than report an unexplained dead pid. +func TestUpExplainsAHeadOfIterationHalt(t *testing.T) { + repo, opts, probe := upFixture(t) + opts.Spawn = func(c Child) (ChildHandle, error) { + probe.launched = append(probe.launched, c) + // The real spawn path reaps its child; so does the fake, or the + // health receipt would be watching a pid nobody waited. + exited := make(chan struct{}) + close(exited) + return ChildHandle{Pid: 5556, Exited: exited}, nil + } + opts.Alive = func(int) bool { return false } + writeRepoFileForUp(t, repo, "logs/driver.log", "[state] pulled 0 ledger entries\nmax iterations reached\n") + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if res.OK { + t.Fatalf("a driver that halted at the loop head was reported healthy: %+v", res.Steps) + } + var detail string + for _, s := range res.Steps { + if s.Name == "loop" { + detail = s.Detail + } + } + if !strings.Contains(detail, "max iterations reached") { + t.Errorf("loop detail = %q, want the halt line from driver.log", detail) + } +} + +// TestUpRefusesToStartBehindADirtyStateDoc: the driver's own currency gate +// skips every iteration while docs/PRD.md is uncommitted, so a factory started +// over a draft looks busy and ships nothing — and intake would have queued the +// draft as if it were intent. `up` must say so before it starts anything. +func TestUpRefusesToStartBehindADirtyStateDoc(t *testing.T) { + _, opts, probe := upFixture(t) + opts.Dirty = func(string, string) (bool, error) { return true, nil } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if res.OK { + t.Errorf("up started over a dirty docs/PRD.md: %+v", res.Steps) + } + if len(probe.launched) != 0 { + t.Errorf("started %v behind the dirty PRD", names(probe.launched)) + } + for _, s := range res.Steps { + if s.Name == "intake" { + t.Errorf("intake ran on a draft PRD: %+v", s) + } + } + var hint string + for _, s := range res.Steps { + if s.Name == "prd" { + hint = s.Hint + } + } + if !strings.Contains(hint, "git commit") { + t.Errorf("prd hint = %q, want the commit line", hint) + } +} + +// TestUpProceedsOnACleanStateDoc pins the other direction, including that an +// unanswerable probe (not a git checkout) must not block the start. +func TestUpProceedsOnACleanStateDoc(t *testing.T) { + _, opts, _ := upFixture(t) + opts.Dirty = func(string, string) (bool, error) { return false, nil } + if res, err := RunUp(opts); err != nil || !res.OK { + t.Errorf("clean PRD refused the start: %+v (%v)", res.Steps, err) + } + + _, opts2, probe2 := upFixture(t) + opts2.Dirty = func(string, string) (bool, error) { return false, errors.New("not a git repository") } + res2, err := RunUp(opts2) + if err != nil { + t.Fatal(err) + } + if !res2.OK || len(probe2.launched) == 0 { + t.Errorf("an unknown PRD state blocked the start: %+v", res2.Steps) + } +} + +// TestUpScoutLaneIsASecondRecordedChild: `--scout` runs the researcher as +// its own detached child with its own pid file and log, so `down` stops both +// lanes and neither hides the other. +func TestUpScoutLaneIsASecondRecordedChild(t *testing.T) { + repo, opts, probe := upFixture(t) + opts.Scout = true + opts.ScoutIntervalMinutes = 45 + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if !res.OK { + t.Fatalf("up failed: %+v", res.Steps) + } + scout := probe.child(t, "scout") + if strings.Join(scout.Args, " ") != "scout --repo "+repo+" --interval 45" { + t.Errorf("scout args = %q", strings.Join(scout.Args, " ")) + } + if scout.LogPath != filepath.Join(repo, ".selfbuild", "logs", "scout.log") || scout.PidFile == "" { + t.Errorf("scout child = %+v, want its own log + pid record", scout) + } + if res.Logs["scout"] == "" { + t.Errorf("the report must name the scout log") + } + + // `down` stops it, and only reports the lanes it has a record for. + down, err := RunDown(UpOptions{ + RepoPath: repo, + Alive: func(int) bool { return true }, + Terminate: func(int) error { return nil }, + }) + if err != nil { + t.Fatal(err) + } + var sawScout bool + for _, s := range down.Stopped { + if strings.Contains(s, "scout") { + sawScout = true + } + } + if !sawScout { + t.Errorf("down stopped %v, want the scout lane included", down.Stopped) + } +} + +func names(children []Child) []string { + var out []string + for _, c := range children { + out = append(out, c.Name) + } + return out +} + +// proveDriverAlive writes the two artifacts a live driver produces — the loop +// lock record and a heartbeat naming a phase — so a test can assert `up`'s +// health receipt against real evidence rather than a stub return value. +func proveDriverAlive(t *testing.T, repo string, pid int) { + t.Helper() + lockDir := filepath.Join(repo, ".selfbuild", "loop.lock.d") + writePid(t, lockDir, "pid", strconv.Itoa(pid)) + hb := `{"iteration":12,"phase":"research","pid":` + strconv.Itoa(pid) + `,"updatedAt":"2026-09-14T00:00:00Z"}` + if err := os.WriteFile(driverHeartbeatPath(repo), []byte(hb), 0o644); err != nil { + t.Fatal(err) + } +} + +// TestUpHealthReceiptRejectsADriverThatDied: a green `up` must mean a running +// factory. A driver that takes the lock and then exits — an iteration cap +// already reached, a starvation halt — exits 0, so no supervisor tells anyone; +// `up` has to, and it has to name the cause from the driver's own log. +func TestUpHealthReceiptRejectsADriverThatDied(t *testing.T) { + repo, opts, probe := upFixture(t) + opts.Spawn = func(c Child) (ChildHandle, error) { + probe.launched = append(probe.launched, c) + return ChildHandle{Pid: 5555}, nil // alive for a moment, then gone + } + opts.Alive = func(int) bool { return false } + // The driver's last verdict, as it lands in its own iteration log. + writeRepoFileForUp(t, repo, "logs/loop-12.log", "=== self-build loop 12 start ===\n[starvation] 5 consecutive non-productive iterations — halting loop\n") + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if res.OK { + t.Fatalf("a dead driver was reported healthy: %+v", res.Steps) + } + var detail string + for _, s := range res.Steps { + if s.Name == "loop" { + detail = s.Detail + } + } + if !strings.Contains(detail, "halting loop") { + t.Errorf("loop detail = %q, want the halt reason from the driver log", detail) + } +} + +// TestUpHealthReceiptNeedsProof: a pid that is alive but has taken neither the +// loop lock nor written a heartbeat is a process, not a factory — the wait is +// bounded and the report says what it looked for. +func TestUpHealthReceiptNeedsProof(t *testing.T) { + repo, opts, probe := upFixture(t) + opts.Spawn = func(c Child) (ChildHandle, error) { + probe.launched = append(probe.launched, c) + return ChildHandle{Pid: 6666}, nil + } + opts.Alive = func(int) bool { return true } + opts.HealthWindow = 20 * time.Millisecond + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if res.OK { + t.Errorf("an unproven driver was reported healthy: %+v", res.Steps) + } + var detail string + for _, s := range res.Steps { + if s.Name == "loop" { + detail = s.Detail + } + } + if !strings.Contains(detail, "no loop lock or heartbeat") { + t.Errorf("loop detail = %q, want what the health check looked for", detail) + } + _ = repo +} + +// TestUpReportsALiveDriverInsteadOfProvingItsOwn: when a driver already holds +// the lock and has published a heartbeat before `up` runs, `up` must report +// that holder and start nothing — the health receipt is for the driver `up` +// itself launched, and a second driver in one repo is the race #354 is about. +func TestUpReportsALiveDriverInsteadOfProvingItsOwn(t *testing.T) { + repo, opts, probe := upFixture(t) + // A driver that is already live, started by some other route. + proveDriverAlive(t, repo, 4243) + opts.Alive = func(pid int) bool { return pid == 4243 } + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if !res.OK { + t.Fatalf("up failed: %+v", res.Steps) + } + for _, c := range probe.launched { + if c.Name == "loop" { + t.Errorf("a live driver was double-started: %v", names(probe.launched)) + } + } + if res.PIDs["loop"] != 4243 { + t.Errorf("report loop pid = %d, want the existing holder 4243", res.PIDs["loop"]) + } +} + +// TestUpHealthProofCanBeSkipped: `--wait 0` is the escape hatch for a caller +// that starts the factory from a script and does its own watching. +func TestUpHealthProofCanBeSkipped(t *testing.T) { + _, opts, probe := upFixture(t) + opts.Spawn = func(c Child) (ChildHandle, error) { + probe.launched = append(probe.launched, c) + return ChildHandle{Pid: 7777}, nil + } + opts.Alive = func(int) bool { return true } + opts.HealthWindow = -1 + + res, err := RunUp(opts) + if err != nil { + t.Fatal(err) + } + if !res.OK { + t.Errorf("--wait 0 must skip the proof instead of failing it: %+v", res.Steps) + } +} + +func writeRepoFileForUp(t *testing.T, repo, rel, content string) { + t.Helper() + path := filepath.Join(repo, ".selfbuild", rel) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} diff --git a/internal/commands/updown_unix.go b/internal/commands/updown_unix.go new file mode 100644 index 00000000..9d495439 --- /dev/null +++ b/internal/commands/updown_unix.go @@ -0,0 +1,31 @@ +//go:build unix + +package commands + +import ( + "fmt" + "os/exec" + "syscall" +) + +// detachAttrs puts a child in its own session and process group: it survives +// `devagent up` exiting, and — because the driver becomes the group leader — +// one group signal from `devagent down` reaps the driver together with every +// worker it dispatched. The daemon detaches the same way (detach_unix.go). +func detachAttrs(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} +} + +// terminateTree signals the child's whole process group, falling back to the +// bare pid when the child was not a group leader — the shape of a driver +// started by `make loop-start` or a bare `devagent loop`, whose lock record +// `down` can still find. +func terminateTree(pid int) error { + if err := syscall.Kill(-pid, syscall.SIGTERM); err == nil { + return nil + } + if err := syscall.Kill(pid, syscall.SIGTERM); err != nil { + return fmt.Errorf("kill %d: %w", pid, err) + } + return nil +} diff --git a/internal/loopdriver/config.go b/internal/loopdriver/config.go index fc8a5912..f99095fe 100644 --- a/internal/loopdriver/config.go +++ b/internal/loopdriver/config.go @@ -7,6 +7,7 @@ import ( "time" "github.com/FreePeak/devagent/internal/orchestrator" + "github.com/FreePeak/devagent/internal/prdintake" ) // LoopConfig carries every knob the bash driver reads from the SELFBUILD_* @@ -90,6 +91,17 @@ type LoopConfig struct { GhBin string // LockDir overrides the default Repo/.selfbuild/loop.lock.d; test seam. LockDir string + // PRDIntake runs docs/PRD.md intake at the head of every iteration + // (SELFBUILD_PRD_INTAKE, issue #370). nil = enabled: the operator's open + // `- [ ]` items are the lane's best work source, and an empty lane is + // what makes the driver ship loop plumbing (issue #355). + PRDIntake *bool + // PRDIntakeMax caps how many new rows one pass may enqueue + // (SELFBUILD_PRD_INTAKE_MAX; 0 = prdintake.DefaultMaxItems, negative = + // unbounded). + PRDIntakeMax int + // Intake is the seam hermetic tests inject; nil = prdintake.Ingest. + Intake func(prdintake.Options) (prdintake.Report, error) // ExitWatchdogDelay bounds how long a terminal loop verdict may take // to actually leave the process (issue #286): the driver printed its // starvation halt and then hung forever in a wedged exec teardown @@ -152,6 +164,18 @@ func getenvSet(getenv func(string) string, key string) bool { return getenv(key) == "1" } +// getenvFlag reads a tri-state 0/1 knob: unset is nil (the default applies), +// "0" disables, anything else enables. SELFBUILD_PRD_INTAKE needs the +// three-way because its default is on, which a plain bool cannot express. +func getenvFlag(getenv func(string) string, key string) *bool { + v := getenv(key) + if v == "" { + return nil + } + on := v != "0" + return &on +} + // WithDefaults fills zero-valued fields with the bash defaults. GHRepo // stays empty here: RunLoop derives it from `git remote get-url origin` // via ghRepoFromRemote when unset (the bash fallback). @@ -231,6 +255,12 @@ func (c LoopConfig) WithDefaults() LoopConfig { if c.Stderr == nil { c.Stderr = os.Stderr } + // PRD intake is on unless SELFBUILD_PRD_INTAKE=0 said otherwise + // (issue #370). + if c.PRDIntake == nil { + on := true + c.PRDIntake = &on + } return c } @@ -276,5 +306,7 @@ func ConfigFromGetenv(repo string, getenv func(string) string) LoopConfig { NoProgressTimeoutMS: getintOr(getenv, "SELFBUILD_NO_PROGRESS_TIMEOUT_MS", defaultNoProgressMS), SyncRetrySecs: getintOr(getenv, "SELFBUILD_SYNC_RETRY_SECS", defaultSyncRetrySecs), DevagentBin: getenv("SELFBUILD_DEVAGENT_BIN"), + PRDIntake: getenvFlag(getenv, "SELFBUILD_PRD_INTAKE"), + PRDIntakeMax: getintOr(getenv, "SELFBUILD_PRD_INTAKE_MAX", 0), } } diff --git a/internal/loopdriver/intake.go b/internal/loopdriver/intake.go new file mode 100644 index 00000000..58285420 --- /dev/null +++ b/internal/loopdriver/intake.go @@ -0,0 +1,72 @@ +package loopdriver + +// PRD intake (issue #370): the operator's open `- [ ]` items in docs/PRD.md +// become queue rows the same iteration can claim. +// +// This is the sanctioned exception to "the PRD is a state document, never a +// backlog" (docs/SELF-BUILD-LOOP.md, 2026-09-07 policy): the lane has been +// empty since that policy landed, and an empty lane is precisely what makes +// the driver spend its runtime repairing itself instead of shipping product +// (issue #355). A checkbox is not prose — it is the operator typing "build +// this" — and intake is deterministic (no LLM, no network), so it costs one +// file read per iteration. +// +// Placement is load-bearing: intake runs after the PRD-currency gate (which +// guarantees the file matches a commit, so a mid-edit draft is never read as +// intent) and before pickIssue/claimQueueTask (so a fresh item is built by +// this iteration, not the next one). A failure here is logged and never +// fails the iteration: intake is a work source, not a dependency. + +import ( + "fmt" + "io" + "os" + + "github.com/FreePeak/devagent/internal/prdintake" +) + +// runPrdIntake ingests docs/PRD.md into the queue and breadcrumbs the result +// into the iteration log. Returns the number of rows newly queued. +func (d *driver) runPrdIntake(n int, logF io.Writer) int { + cfg := d.cfg + if cfg.PRDIntake != nil && !*cfg.PRDIntake { + return 0 + } + ingest := cfg.Intake + if ingest == nil { + ingest = prdintake.Ingest + } + report, err := ingest(prdintake.Options{ + RepoPath: cfg.Repo, + MaxItems: cfg.PRDIntakeMax, + // A dry run rehearses the verdict: parse and report, write nothing. + DryRun: cfg.DryRun, + }) + if err != nil { + // A repo without docs/PRD.md, or an unreadable one, is a work source + // that is simply unavailable — never an iteration failure. + if os.IsNotExist(err) { + _, _ = fmt.Fprintln(logF, "[prd-intake] no docs/PRD.md to ingest — continuing") + } else { + _, _ = fmt.Fprintf(logF, "[prd-intake] skipped: %v\n", err) + } + return 0 + } + if len(report.Queued) == 0 { + if report.Open > 0 { + // Every open item is already a queue row; saying so keeps + // "nothing queued" from reading as "nothing found". + _, _ = fmt.Fprintf(logF, "[prd-intake] %d open PRD item(s) already in the queue\n", report.Open) + } + return 0 + } + verb := "queued" + if cfg.DryRun { + verb = "would queue" + } + for _, it := range report.Queued { + _, _ = fmt.Fprintf(logF, "[prd-intake] %s %s: %s (docs/PRD.md:%d)\n", verb, it.ID, it.Title, it.Line) + } + d.phase(n, "prd-intake", fmt.Sprintf("%s %d operator item(s) from docs/PRD.md", verb, len(report.Queued))) + return len(report.Queued) +} diff --git a/internal/loopdriver/intake_test.go b/internal/loopdriver/intake_test.go new file mode 100644 index 00000000..dead1204 --- /dev/null +++ b/internal/loopdriver/intake_test.go @@ -0,0 +1,166 @@ +package loopdriver + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/FreePeak/devagent/internal/prdintake" + "github.com/FreePeak/devagent/internal/queue" +) + +// The intake step is the route from an operator's PRD edit into the same +// iteration's claim (issue #370). These tests pin the three properties that +// make it safe to run unattended at the head of every iteration: it queues +// intent, it can be switched off, and it can never be the reason an +// iteration fails. + +func intakeDriver(t *testing.T, repo string, mutate func(*LoopConfig)) *driver { + t.Helper() + cfg := loopConfigFor(t, repo, mutate) + return &driver{cfg: cfg, stateDir: filepath.Join(repo, ".selfbuild")} +} + +// TestRunPrdIntakeQueuesForTheSameIteration: an open `- [ ]` item in +// docs/PRD.md becomes a claimable queue row before phase 2a runs, so the +// iteration that reads the operator's edit is the iteration that builds it. +func TestRunPrdIntakeQueuesForTheSameIteration(t *testing.T) { + repo := initFixtureRepo(t) + writeRepoFile(t, repo, "docs/PRD.md", "## 12. CLI\n\n- [ ] Add the intake lane\n - queues the item\n") + + var log bytes.Buffer + d := intakeDriver(t, repo, func(c *LoopConfig) { c.DryRun = false }) + if n := d.runPrdIntake(1, &log); n != 1 { + t.Fatalf("runPrdIntake queued %d, want 1 (log: %s)", n, log.String()) + } + + rows := queue.ListTasks(repo, queue.StatusPending) + if len(rows) != 1 { + t.Fatalf("queue rows = %d, want 1", len(rows)) + } + if !strings.HasPrefix(rows[0].Goal, "Goal: ") { + t.Errorf("goal is off the dispatch contract: %q", rows[0].Goal) + } + if got := *rows[0].Source; got != "prd" { + t.Errorf("source = %q, want prd", got) + } + if !strings.Contains(log.String(), "queued PRD-") { + t.Errorf("log = %q, want the queued row named", log.String()) + } + + // The claim the driver makes right after intake is that row. + claimed := claimQueueTask(repo) + if claimed == nil || claimed.ID != rows[0].ID { + t.Fatalf("claim = %+v, want the intake row %s", claimed, rows[0].ID) + } +} + +// TestRunPrdIntakeIsIdempotentAcrossIterations: every iteration runs intake, +// so a second pass over an unchanged PRD must queue nothing — a re-queued +// item would re-burn a dispatch on work already shipped. +func TestRunPrdIntakeIsIdempotentAcrossIterations(t *testing.T) { + repo := initFixtureRepo(t) + writeRepoFile(t, repo, "docs/PRD.md", "## Scope\n\n- [ ] Build the lane\n") + + var log bytes.Buffer + d := intakeDriver(t, repo, func(c *LoopConfig) { c.DryRun = false }) + if n := d.runPrdIntake(1, &log); n != 1 { + t.Fatalf("first pass queued %d, want 1", n) + } + log.Reset() + if n := d.runPrdIntake(2, &log); n != 0 { + t.Fatalf("second pass queued %d, want 0", n) + } + if !strings.Contains(log.String(), "already in the queue") { + t.Errorf("second pass log = %q, want the already-known report", log.String()) + } + if len(queue.ListTasks(repo, "")) != 1 { + t.Errorf("queue holds %d rows, want 1", len(queue.ListTasks(repo, ""))) + } +} + +// TestRunPrdIntakeKnobAndFailurePaths: SELFBUILD_PRD_INTAKE=0 is the operator +// off-switch, and an intake that cannot read the PRD is a missing work source +// — never an iteration failure, and never a starvation/breaker increment. +func TestRunPrdIntakeKnobAndFailurePaths(t *testing.T) { + repo := initFixtureRepo(t) + writeRepoFile(t, repo, "docs/PRD.md", "## Scope\n\n- [ ] Build the lane\n") + + off := false + var log bytes.Buffer + d := intakeDriver(t, repo, func(c *LoopConfig) { c.PRDIntake = &off; c.DryRun = false }) + if n := d.runPrdIntake(1, &log); n != 0 || log.Len() != 0 { + t.Errorf("disabled intake queued %d and logged %q, want neither", n, log.String()) + } + if len(queue.ListTasks(repo, "")) != 0 { + t.Errorf("SELFBUILD_PRD_INTAKE=0 still wrote queue rows") + } + + // A repo with no PRD at all. + empty := initFixtureRepo(t) + log.Reset() + d = intakeDriver(t, empty, func(c *LoopConfig) { c.DryRun = false }) + if n := d.runPrdIntake(1, &log); n != 0 { + t.Errorf("missing PRD queued %d, want 0", n) + } + if !strings.Contains(log.String(), "no docs/PRD.md") { + t.Errorf("log = %q, want the reason", log.String()) + } + + // A seam that errors for another reason still cannot fail the iteration. + log.Reset() + d = intakeDriver(t, repo, func(c *LoopConfig) { + c.DryRun = false + c.Intake = func(prdintake.Options) (prdintake.Report, error) { + return prdintake.Report{}, os.ErrPermission + } + }) + if n := d.runPrdIntake(1, &log); n != 0 { + t.Errorf("failing intake queued %d, want 0", n) + } + if !strings.Contains(log.String(), "[prd-intake] skipped") { + t.Errorf("log = %q, want the skip report", log.String()) + } +} + +// TestRunPrdIntakeHonoursDryRun: a dry run rehearses the verdict and touches +// nothing, so the row it reports is the row it would have written. +func TestRunPrdIntakeHonoursDryRun(t *testing.T) { + repo := initFixtureRepo(t) + writeRepoFile(t, repo, "docs/PRD.md", "## Scope\n\n- [ ] Build the lane\n") + + var log bytes.Buffer + d := intakeDriver(t, repo, nil) // loopConfigFor defaults DryRun = true + if n := d.runPrdIntake(1, &log); n != 1 { + t.Fatalf("dry run reported %d queued, want 1", n) + } + if !strings.Contains(log.String(), "would queue") { + t.Errorf("log = %q, want the would-queue wording", log.String()) + } + if len(queue.ListTasks(repo, "")) != 0 { + t.Errorf("dry run wrote queue rows") + } +} + +// TestConfigHonoursPRDIntakeKnob: the tri-state env knob must keep "unset" +// distinct from "0", because the default is on — a plain bool would silently +// turn the lane off for every driver that never mentioned it. +func TestConfigHonoursPRDIntakeKnob(t *testing.T) { + env := func(m map[string]string) func(string) string { + return func(k string) string { return m[k] } + } + cfg := ConfigFromGetenv("/r", env(map[string]string{})).WithDefaults() + if cfg.PRDIntake == nil || !*cfg.PRDIntake { + t.Errorf("unset env must default intake on") + } + cfg = ConfigFromGetenv("/r", env(map[string]string{"SELFBUILD_PRD_INTAKE": "0"})).WithDefaults() + if cfg.PRDIntake == nil || *cfg.PRDIntake { + t.Errorf("SELFBUILD_PRD_INTAKE=0 must disable intake") + } + cfg = ConfigFromGetenv("/r", env(map[string]string{"SELFBUILD_PRD_INTAKE_MAX": "2"})) + if cfg.PRDIntakeMax != 2 { + t.Errorf("PRDIntakeMax = %d, want 2", cfg.PRDIntakeMax) + } +} diff --git a/internal/loopdriver/run.go b/internal/loopdriver/run.go index 7bd2bec4..edbd943b 100644 --- a/internal/loopdriver/run.go +++ b/internal/loopdriver/run.go @@ -238,6 +238,13 @@ func (d *driver) runIteration(n int, logF io.Writer, gradient, clusters string) } } + // PRD intake (issue #370): operator-authored open checkboxes become + // queue rows before this iteration picks anything, so the deterministic + // lane carries real intent instead of falling through to LLM + // self-selection. Runs after the PRD-currency gate above: the file now + // matches a commit, so a draft mid-edit can never be read as intent. + d.runPrdIntake(n, logF) + // Tracker snapshot (issue-first): one gh call per iteration; a failed // listing degrades to empty and the LLM selection path runs instead. issuePick := "" @@ -311,7 +318,15 @@ func (d *driver) runIteration(n int, logF io.Writer, gradient, clusters string) } d.phase(n, "issue", detail) } else if queued == nil { - _, _ = fmt.Fprintln(logF, "[issue] no open selfbuild issue found — falling back to LLM selection") + // The empty lane is the finding, not a detail (issue #355): every + // recent iteration shipped loop plumbing because this branch ran + // silently and the LLM could only find itself to work on. The + // ledger row shape is byte-identical to the bash driver's and must + // stay that way, so the breadcrumb goes to events.jsonl where the + // TUI and `devagent status` read it, and the remedy is named in the + // log line. + _, _ = fmt.Fprintln(logF, "[issue] empty lane: no open selfbuild issue and no queue row — falling back to LLM selection (refill: label real work `selfbuild`, or open a `- [ ]` item in docs/PRD.md for prd-intake)") + d.phase(n, "queue-empty", "label selfbuild issues or add docs/PRD.md - [ ] items") } // Phases 2-3: PO/LLM fallback (empty tracker + empty queue only). diff --git a/internal/pipeline/create.go b/internal/pipeline/create.go index ac17bdf2..3bef0066 100644 --- a/internal/pipeline/create.go +++ b/internal/pipeline/create.go @@ -161,14 +161,17 @@ func BuildLaunchAgentPlist(spec plistSpec) string { func RolePlistSpecs(opts CreateOptions, intervalMinutes int, scoutWorker string, trackIntervalMinutes int) []plistSpec { var specs []plistSpec selfExe := createSelfExePath() - devagentBin := filepath.Join(opts.RepoPath, "dist", "src", "cli.js") + // The Go build is one self-contained binary, so argv is [selfExe, + // , …]. The retired Node tree needed a second slot for + // dist/src/cli.js; leaving that entry in made every installed agent die + // at cobra's command lookup (issue #371). if opts.Scout { specs = append(specs, plistSpec{ label: "com.devagent.scout", logName: "devagent-scout.log", repoPath: opts.RepoPath, programArgs: []string{ - selfExe, devagentBin, "scout", "--repo", opts.RepoPath, + selfExe, "scout", "--repo", opts.RepoPath, "--interval", fmt.Sprintf("%d", intervalMinutes), "--worker", scoutWorker, "--timeout", "30", }, @@ -180,7 +183,7 @@ func RolePlistSpecs(opts CreateOptions, intervalMinutes int, scoutWorker string, logName: "devagent-tracker.log", repoPath: opts.RepoPath, programArgs: []string{ - selfExe, devagentBin, "track", "--repo", opts.RepoPath, + selfExe, "track", "--repo", opts.RepoPath, "--interval", fmt.Sprintf("%d", trackIntervalMinutes), }, }) diff --git a/internal/prdintake/prdintake.go b/internal/prdintake/prdintake.go new file mode 100644 index 00000000..d1560263 --- /dev/null +++ b/internal/prdintake/prdintake.go @@ -0,0 +1,399 @@ +// Package prdintake turns operator-authored PRD intent into factory work. +// +// docs/PRD.md is a state document (docs/SELF-BUILD-LOOP.md "Tracker + PRD +// policy"): it records what the repo IS, and the loop treats an uncommitted +// edit to it as the operator pausing the factory. That left every real +// "build this next" the operator wrote with no route into the deterministic +// lane — the "long runtime, little value" half of issue #355: an empty lane +// makes the driver invent its own plumbing work. +// +// Intake is the one sanctioned exception, and it is deliberately narrow: an +// OPEN markdown checkbox in the PRD (`- [ ] `) is an explicit request +// to build that item. Parsing is deterministic — no LLM, no network — and +// idempotent: an item's queue id is a hash of its normalized text, so +// re-running intake over an unchanged PRD enqueues nothing, and a shipped +// item the worker ticked to `- [x]` is state, not work. +package prdintake + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/FreePeak/devagent/internal/queue" +) + +// DefaultMaxItems caps how many new rows one pass may enqueue. An operator +// who drops a 40-item wishlist into the PRD gets the first DefaultMaxItems +// built, in document order (top of the file = highest priority), instead of +// a queue flood that starves the tracker lane. +const DefaultMaxItems = 5 + +// goalWordCap mirrors the loop's dispatch contract (internal/loopdriver +// validateGoalShape): a goal over 120 words is refused at the dispatch +// boundary and its queue claim retired, so intake truncates the item text +// into shape instead of handing the loop an undeliverable row. +const goalWordCap = 120 + +// TickCriterion is carried by every intake row: the shipped PR ticks its own +// source checkbox, so an item cannot be built twice. +const TickCriterion = "Tick the source checkbox in docs/PRD.md from `- [ ]` to `- [x]` in the same PR, so the item never re-enters the intake lane." + +// Item is one open checkbox in the PRD, with the context a worker needs to +// implement it without re-reading the whole document. +type Item struct { + // ID is the deterministic queue id: "PRD-" + 8 hex of the normalized text. + ID string `json:"id"` + // Title is the item text with inline markdown stripped. + Title string `json:"title"` + // Goal is the queue goal text: "Goal: " prefixed and inside goalWordCap. + Goal string `json:"goal"` + // Criteria are the item's sub-bullets plus TickCriterion. + Criteria []string `json:"acceptanceCriteria,omitempty"` + // Section is the nearest enclosing heading (the "where did this come + // from" line). + Section string `json:"section,omitempty"` + // Line is the 1-based line of the checkbox in the PRD. + Line int `json:"line"` + // PRDMarkdown is the enclosing section, stored as the queue task-PRD + // sidecar so the dispatch reads the real spec, not just one bullet. + PRDMarkdown string `json:"-"` +} + +// Options configures one Ingest pass. +type Options struct { + // RepoPath is the checkout whose docs/PRD.md is read. + RepoPath string + // MaxItems bounds new rows per pass; 0 = DefaultMaxItems, negative = + // unbounded. + MaxItems int + // DryRun parses and reports without writing queue rows. + DryRun bool +} + +// Report is one Ingest pass outcome. +type Report struct { + // Open counts every open checkbox found (including ones this pass did + // not enqueue). + Open int `json:"open"` + // Queued lists the rows this pass created (or, with DryRun, would). + Queued []Item `json:"queued"` + // Known lists open items the queue store already holds — pending, + // claimed, done or failed — so "nothing queued" never reads as "nothing + // found". + Known []Item `json:"known"` + // QueueDepth is the row count in the queue store after this pass. + QueueDepth int `json:"queueDepth"` + // PRDPath is the file that was read. + PRDPath string `json:"prdPath"` +} + +// DefaultPRDPath is the state document every repo convention points at. +func DefaultPRDPath(repoPath string) string { + return filepath.Join(repoPath, "docs", "PRD.md") +} + +var ( + // A checkbox item: ` - [ ] text` / `* [x] text`. + checkboxRe = regexp.MustCompile(`^([ \t]*)([-*])[ \t]+\[([ xX])\][ \t]*(.*)$`) + // A bullet that is not a checkbox (sub-bullet criteria, continuation). + bulletRe = regexp.MustCompile(`^([ \t]*)([-*])[ \t]+(.*)$`) + // Any heading, for the section context. + headingRe = regexp.MustCompile(`^#{1,6}[ \t]+(.*)$`) + // A code-fence marker (``` or ~~~), optionally indented. + fenceRe = regexp.MustCompile("^[ \\t]*(`{3,}|~{3,})") + + linkRe = regexp.MustCompile(`\[([^\]]*)\]\([^)]*\)`) + inlineRe = regexp.MustCompile("[*_`~]") +) + +// Parse extracts the open checkbox items from PRD markdown, in document +// order — the PRD's own top-to-bottom reading order is the priority order. +// +// Never work: `- [x]` (shipped state), struck items, checkboxes inside a +// code fence or a blockquote (a `>` line is a state note, not an +// instruction), and bullets without a checkbox (prose). +func Parse(prd string) []Item { + lines := strings.Split(prd, "\n") + var items []Item + section := "" + fence := "" + + // The cursor advances past a consumed item body, so a nested checkbox + // can only ever be that item's criterion, never an item of its own. + for i := 0; i < len(lines); i++ { + line := strings.TrimRight(lines[i], "\r") + + if mark := fenceMark(line); mark != "" { + switch { + case fence == "": + fence = mark + case strings.HasPrefix(mark, fence): + fence = "" + } + continue + } + if fence != "" { + continue + } + + trimmed := strings.TrimSpace(line) + if h := headingRe.FindStringSubmatch(trimmed); h != nil { + section = strings.TrimSpace(h[1]) + continue + } + if strings.HasPrefix(trimmed, ">") { + continue + } + m := checkboxRe.FindStringSubmatch(line) + if m == nil || m[3] != " " { + continue + } + + text, sub, last := collectItemBody(lines, i, len(m[1]), m[4]) + title := cleanTitle(text) + lineNo := i + 1 + i = last + if title == "" { + continue + } + items = append(items, Item{ + ID: StableID(title), + Title: title, + Goal: BuildGoal(title, section, lineNo), + Criteria: criteria(sub), + Section: section, + Line: lineNo, + PRDMarkdown: sectionBody(lines, lineNo-1), + }) + } + return items +} + +// collectItemBody folds the lines that belong to the item at start into its +// text and criteria, returning the index of the last line it consumed. A line +// belongs while it is more indented than the checkbox marker: a nested +// checkbox or plain bullet is a criterion, other text is wrapped prose +// continuing the item. A blank line, a heading, a fence, or any line at or +// below the marker's indent ends the item. +func collectItemBody(lines []string, start, markerIndent int, first string) (string, []string, int) { + text := strings.TrimSpace(first) + var sub []string + last := start + for j := start + 1; j < len(lines); j++ { + line := strings.TrimRight(lines[j], "\r") + if strings.TrimSpace(line) == "" { + break + } + if fenceMark(line) != "" || headingRe.MatchString(strings.TrimSpace(line)) { + break + } + indent := indentWidth(line) + if indent <= markerIndent { + break + } + if cb := checkboxRe.FindStringSubmatch(line); cb != nil { + if t := cleanTitle(cb[4]); t != "" { + sub = append(sub, t) + } + last = j + continue + } + if b := bulletRe.FindStringSubmatch(line); b != nil { + if t := cleanTitle(b[3]); t != "" { + sub = append(sub, t) + } + last = j + continue + } + text += " " + strings.TrimSpace(line) + last = j + } + return text, sub, last +} + +// criteria renders a row's acceptance criteria: the item's own sub-bullets, +// then the tick-the-box rule every intake row carries. +func criteria(sub []string) []string { + out := make([]string, 0, len(sub)+1) + out = append(out, sub...) + return append(out, TickCriterion) +} + +// sectionBody returns the enclosing section (its heading through the next +// heading, or EOF), capped so a 200-line section cannot ride into every +// dispatch. Returns "" when the item sits above the first heading. +func sectionBody(lines []string, itemLine int) string { + start := -1 + for i := itemLine; i >= 0; i-- { + if headingRe.MatchString(strings.TrimSpace(strings.TrimRight(lines[i], "\r"))) { + start = i + break + } + } + if start < 0 { + return "" + } + end := len(lines) + for i := itemLine + 1; i < len(lines); i++ { + if headingRe.MatchString(strings.TrimSpace(strings.TrimRight(lines[i], "\r"))) { + end = i + break + } + } + body := strings.TrimSpace(strings.Join(lines[start:end], "\n")) + // Capped so a 200-line section cannot ride into every dispatch. + const cap = 6000 + if len(body) > cap { + body = strings.TrimSpace(body[:cap]) + "\n…" + } + return body +} + +// Ingest reads the repo's docs/PRD.md and enqueues every open item the queue +// store does not already hold. Re-running over an unchanged PRD enqueues +// nothing: the queue row's id is the item's content hash, and an existing row +// in any status (pending, claimed, done, failed) counts as known. +func Ingest(opts Options) (Report, error) { + repo := opts.RepoPath + if repo == "" { + repo = "." + } + prdPath := DefaultPRDPath(repo) + raw, err := os.ReadFile(prdPath) + if err != nil { + return Report{}, err + } + report := Report{PRDPath: prdPath} + items := Parse(string(raw)) + report.Open = len(items) + + max := opts.MaxItems + if max == 0 { + max = DefaultMaxItems + } + + known := map[string]bool{} + for _, t := range queue.ListTasks(repo, "") { + known[t.ID] = true + } + + for _, it := range items { + if known[it.ID] { + report.Known = append(report.Known, it) + continue + } + if max >= 0 && len(report.Queued) >= max { + continue + } + if !opts.DryRun { + if _, err := queue.EnqueueTask(repo, queue.EnqueueInput{ + ID: it.ID, + Title: it.Title, + Goal: it.Goal, + AcceptanceCriteria: it.Criteria, + PrdMarkdown: it.PRDMarkdown, + Source: "prd", + }); err != nil { + if _, dup := err.(queue.ErrAlreadyQueued); dup { + report.Known = append(report.Known, it) + continue + } + return report, fmt.Errorf("enqueue %s: %w", it.ID, err) + } + known[it.ID] = true + } + report.Queued = append(report.Queued, it) + } + + // Projected depth in a dry run: the point of the number is "what will the + // lane hold after this pass", and a preview that reports the pre-pass + // depth contradicts the "would queue N" line beside it. + if opts.DryRun { + report.QueueDepth = len(known) + len(report.Queued) + } else { + report.QueueDepth = len(queue.ListTasks(repo, "")) + } + return report, nil +} + +// BuildGoal renders the dispatch statement: "Goal: " prefixed (the contract +// every producer meets) and inside the word cap, pointing the worker at the +// exact PRD line for the full spec. The item text is what shrinks when the +// statement runs long — the reference and the ask never do. +func BuildGoal(title, section string, line int) string { + suffix := "" + if section != "" { + suffix = fmt.Sprintf(", section %q", section) + } + format := "Goal: Implement the operator's PRD item %q (docs/PRD.md:%d%s) in full and verifiably, and ship it as a PR." + for words := 0; ; words++ { + g := fmt.Sprintf(format, title, line, suffix) + if len(strings.Fields(g)) <= goalWordCap || words > 40 || title == "" { + return g + } + title = truncateWords(title, titleWords(title)-5) + if title == "" { + return fmt.Sprintf(format, "see the referenced PRD item", line, suffix) + } + } +} + +// StableID is the queue id: "PRD-" + 8 hex of the normalized item text. An +// edit to the text changes the id, so a rewritten item is fresh work and an +// untouched one never double-enqueues. +func StableID(title string) string { + sum := sha256.Sum256([]byte(strings.ToLower(strings.Join(strings.Fields(title), " ")))) + return "PRD-" + hex.EncodeToString(sum[:4]) +} + +func titleWords(s string) int { return len(strings.Fields(s)) } + +// truncateWords keeps the first n whitespace-separated words. +func truncateWords(s string, n int) string { + if n <= 0 { + return "" + } + w := strings.Fields(s) + if len(w) <= n { + return s + } + return strings.Join(w[:n], " ") + "…" +} + +// cleanTitle strips inline markdown so the queue row and the hash see the +// item's words, not its decoration. +func cleanTitle(s string) string { + s = linkRe.ReplaceAllString(strings.TrimSpace(s), "$1") + s = inlineRe.ReplaceAllString(s, "") + s = strings.Join(strings.Fields(s), " ") + return strings.TrimSpace(strings.Trim(s, ".,;:")) +} + +func indentWidth(line string) int { + n := 0 + for _, r := range line { + switch r { + case ' ': + n++ + case '\t': + n += 4 + default: + return n + } + } + return n +} + +// fenceMark returns a line's fence marker, or "" when it is not one. +func fenceMark(line string) string { + m := fenceRe.FindString(strings.TrimRight(line, "\r")) + if m == "" { + return "" + } + return strings.TrimSpace(m) +} diff --git a/internal/prdintake/prdintake_test.go b/internal/prdintake/prdintake_test.go new file mode 100644 index 00000000..8bfa6ccb --- /dev/null +++ b/internal/prdintake/prdintake_test.go @@ -0,0 +1,255 @@ +package prdintake + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/FreePeak/devagent/internal/queue" +) + +// spec is a PRD excerpt exercising every shape the parser must +// distinguish: intent, shipped state, prose, quotes and fence examples. +var spec = strings.Join([]string{ + "# DevAgent PRD", + "", + "## 12. CLI Specification", + "", + "- [ ] Make `devagent up` start the driver", + " - prints the running pid and log path", + " - is idempotent when a driver already holds the loop lock", + "- [x] Ship the loop driver (done — state, not work)", + "", + "## 17. Roadmap", + "", + "> - [ ] a checkbox inside a blockquote is a state note, not an instruction", + "", + "Some prose bullet that is not intent:", + "", + "- plain bullet, never work", + "", + "~~- [ ] struck item is shipped state~~", + "", + "```markdown", + "- [ ] example inside a code fence is never work", + "```", + "", + "## 21. Simplicity First", + "", + "- [ ] Long item: " + strings.Repeat("word ", 200), + "", +}, "\n") + +// TestParseIntentContract pins what counts as operator intent: an open +// checkbox with its heading context and indented sub-bullets; everything the +// PRD already uses for state (done boxes, blockquotes, struck lines, plain +// bullets, fence examples) must never become work. +func TestParseIntentContract(t *testing.T) { + items := Parse(spec) + if len(items) != 2 { + t.Fatalf("parsed %d items, want 2 (intent only): %v", len(items), titles(items)) + } + + first := items[0] + if first.Title != "Make devagent up start the driver" { + t.Errorf("title = %q, want inline markdown stripped", first.Title) + } + if first.Section != "12. CLI Specification" { + t.Errorf("section = %q", first.Section) + } + if first.Line != 5 { + t.Errorf("line = %d, want 5 (the checkbox line, not its last sub-bullet)", first.Line) + } + want := []string{ + "prints the running pid and log path", + "is idempotent when a driver already holds the loop lock", + TickCriterion, + } + if strings.Join(first.Criteria, "|") != strings.Join(want, "|") { + t.Errorf("criteria = %q, want %q", first.Criteria, want) + } + if !strings.HasPrefix(first.Goal, "Goal: ") { + t.Errorf("goal is not Goal: prefixed: %q", first.Goal) + } + if !strings.Contains(first.Goal, "docs/PRD.md:5") { + t.Errorf("goal must cite the PRD line: %q", first.Goal) + } + if !strings.Contains(first.Goal, "12. CLI Specification") { + t.Errorf("goal must cite the section: %q", first.Goal) + } + if !strings.Contains(first.PRDMarkdown, "## 12. CLI Specification") { + t.Errorf("task-PRD sidecar must carry the section heading") + } + + // Document order is priority order: the Simplicity First item is last. + if items[1].Section != "21. Simplicity First" { + t.Errorf("second item section = %q", items[1].Section) + } +} + +// TestGoalShapeAtTheDispatchBoundary: the loop refuses an over-length goal +// and retires its queue claim, so an enormous PRD item must still produce a +// dispatchable statement inside the 120-word cap. +func TestGoalShapeAtTheDispatchBoundary(t *testing.T) { + items := Parse(spec) + goal := items[len(items)-1].Goal + if n := len(strings.Fields(goal)); n > 120 { + t.Errorf("goal carries %d words, want <= 120: %q", n, goal) + } + if !strings.HasPrefix(goal, "Goal: ") || !strings.Contains(goal, "ship it as a PR") { + t.Errorf("goal lost its contract while shortening: %q", goal) + } +} + +// TestIngestIsIdempotent: the queue id is a content hash, so re-running +// intake over an unchanged PRD enqueues nothing and reports the rows it +// already knew — "nothing queued" must never read as "nothing found". +func TestIngestIsIdempotent(t *testing.T) { + repo := t.TempDir() + writePRD(t, repo, spec) + + first, err := Ingest(Options{RepoPath: repo}) + if err != nil { + t.Fatal(err) + } + if len(first.Queued) != 2 || first.Open != 2 { + t.Fatalf("first pass queued %d of %d", len(first.Queued), first.Open) + } + second, err := Ingest(Options{RepoPath: repo}) + if err != nil { + t.Fatal(err) + } + if len(second.Queued) != 0 { + t.Errorf("second pass queued %d rows, want 0", len(second.Queued)) + } + if len(second.Known) != 2 { + t.Errorf("second pass knew %d rows, want 2", len(second.Known)) + } + if second.QueueDepth != 2 { + t.Errorf("queue depth = %d, want 2", second.QueueDepth) + } + + rows := queue.ListTasks(repo, "") + if len(rows) != 2 { + t.Fatalf("queue holds %d rows, want 2", len(rows)) + } + if got := *rows[0].Source; got != "prd" { + t.Errorf("source = %q, want prd", got) + } + if rows[0].PrdPath == nil { + t.Errorf("a prd-sourced row must carry its section sidecar") + } else if raw, err := os.ReadFile(*rows[0].PrdPath); err != nil || !strings.Contains(string(raw), "## 12. CLI Specification") { + t.Errorf("sidecar %v: %v", rows[0].PrdPath, err) + } +} + +// TestIngestRespectsMaxItems: a wishlist cannot flood the lane — the cap +// keeps document order (top of the PRD wins) and leaves the rest for later +// passes. +func TestIngestRespectsMaxItems(t *testing.T) { + repo := t.TempDir() + writePRD(t, repo, "- [ ] one\n- [ ] two\n- [ ] three\n") + + rep, err := Ingest(Options{RepoPath: repo, MaxItems: 2}) + if err != nil { + t.Fatal(err) + } + if got := titles(rep.Queued); strings.Join(got, ",") != "one,two" { + t.Errorf("queued %v, want the first two in document order", got) + } + if len(queue.ListTasks(repo, "")) != 2 { + t.Errorf("queue holds %d rows, want 2", len(queue.ListTasks(repo, ""))) + } +} + +// TestIngestReclaimsEditedItem: rewording an item is new intent. The old row +// stays retired and the edited text enqueues once under a fresh id. +func TestIngestReclaimsEditedItem(t *testing.T) { + repo := t.TempDir() + writePRD(t, repo, "## Scope\n\n- [ ] Add flag --a\n") + if _, err := Ingest(Options{RepoPath: repo}); err != nil { + t.Fatal(err) + } + writePRD(t, repo, "## Scope\n\n- [ ] Add flag --b\n") + rep, err := Ingest(Options{RepoPath: repo}) + if err != nil { + t.Fatal(err) + } + if len(rep.Queued) != 1 || rep.Queued[0].Title != "Add flag --b" { + t.Fatalf("edited item must re-queue once: %v", titles(rep.Queued)) + } + if StableID("Add flag --a") == StableID("Add flag --b") { + t.Errorf("ids collide across different text") + } +} + +// TestIngestMissingPRDIsAnError: intake must say why it found nothing rather +// than report a clean empty lane. +func TestIngestMissingPRDIsAnError(t *testing.T) { + if _, err := Ingest(Options{RepoPath: t.TempDir()}); err == nil { + t.Fatal("want an error when docs/PRD.md is absent") + } +} + +// TestStableIDIsStableUnderRewrap: the hash reads the item's words, so +// re-wrapping a bullet across lines or changing its emphasis cannot fork a +// second queue row for the same request. +func TestStableIDIsStableUnderRewrap(t *testing.T) { + a := Parse("- [ ] build the thing\n")[0] + b := Parse("- [ ] build the **thing**\n")[0] + if a.ID != b.ID { + t.Errorf("ids differ for the same wording: %s vs %s", a.ID, b.ID) + } + if !strings.HasPrefix(a.ID, "PRD-") || len(a.ID) != 12 { + t.Errorf("id %q is not a PRD-<8hex> queue id", a.ID) + } + if _, err := queue.SanitizeID(a.ID); err != nil { + t.Errorf("id %q is not a legal queue id: %v", a.ID, err) + } +} + +func titles(items []Item) []string { + out := make([]string, len(items)) + for i, it := range items { + out[i] = it.Title + } + return out +} + +func writePRD(t *testing.T, repo, body string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(repo, "docs"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(DefaultPRDPath(repo), []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +// Report is JSON-serializable for `prd-intake --json`; pin the shape a script +// consumes. +func TestReportJSONShape(t *testing.T) { + repo := t.TempDir() + writePRD(t, repo, "## Scope\n\n- [ ] do it\n") + rep, err := Ingest(Options{RepoPath: repo, DryRun: true}) + if err != nil { + t.Fatal(err) + } + if len(rep.Queued) != 1 { + t.Fatalf("dry run must report the item it would queue") + } + if len(queue.ListTasks(repo, "")) != 0 { + t.Fatalf("dry run wrote queue rows") + } + blob, err := json.Marshal(rep) + if err != nil { + t.Fatal(err) + } + for _, key := range []string{"open", "queued", "known", "queueDepth", "prdPath"} { + if !strings.Contains(string(blob), `"`+key+`"`) { + t.Errorf("report json missing %q: %s", key, blob) + } + } +} diff --git a/internal/scout/extract.go b/internal/scout/extract.go index e4f12c30..2b53d6cc 100644 --- a/internal/scout/extract.go +++ b/internal/scout/extract.go @@ -5,11 +5,12 @@ // (compact-context splice, lessons digest, knowledge context) and // src/planner.ts (ticket classification + plan outline). // -// Live scout dispatch (runScoutOnce / runScoutLoop) is intentionally NOT -// ported here: it depends on the queue (enqueueTask, FR-GO-04), the doc -// sync gate (FR-GO-05) and the worker CLI runtime — sibling wave-1 -// packages. The CLI commands `scout` (with --replay) and `scout-status` -// become wireable from this package's exported surface. +// Live scout dispatch (RunOnce/RunLoop, the runScoutOnce/runScoutLoop +// port) lands in run.go: it builds on the queue package (FR-GO-04 #194) +// and the worker runtime (internal/workers). The doc-sync gate (FR-GO-05) +// stays a separate command (`devagent sync-docs`) that operators wrap the +// cycle with. The CLI commands `scout` (live + --replay) and `scout-status` +// wire from this package's exported surface. package scout import ( diff --git a/internal/scout/run.go b/internal/scout/run.go new file mode 100644 index 00000000..dedeb65e --- /dev/null +++ b/internal/scout/run.go @@ -0,0 +1,350 @@ +package scout + +import ( + "errors" + "fmt" + "os" + "strings" + "time" + + "github.com/FreePeak/devagent/internal/config" + "github.com/FreePeak/devagent/internal/queue" + "github.com/FreePeak/devagent/internal/workers" +) + +// This file is the Go port of runScoutOnce/runScoutLoop (FR-SCOUT-01): the +// live scout cycle the `com.devagent.scout` LaunchAgent has been invoking +// since the factory bootstrap. Until this port landed the shipped binary +// exited 3 for every non-replay mode while the LaunchAgent kept launching +// `devagent scout --repo … --interval …` — a silently dead queue writer +// (2026-09-14 audit, the FR-SCOUT-01 revival). The queue half is +// internal/queue (FR-GO-04 #194) and the worker runtime half is +// internal/workers, so the cycle is now fully expressible here. + +// DispatchFn runs one worker invocation and returns its raw output stream +// (the text `ExtractScoutPayload` consumes). It is THE test seam: every +// hermetic test injects a canned payload instead of spawning a real CLI. +type DispatchFn func(worker, model, prompt string, timeout time.Duration) (string, error) + +// RunOptions carries one live cycle's inputs. Zero values defer to +// config.Load(repo): flags/env the CLI resolved win here, config fills the +// rest, and the constants below are the last-resort defaults. +type RunOptions struct { + RepoPath string + Worker string + Model string + // Timeout is the worker dispatch budget (--timeout, minutes at the CLI). + Timeout time.Duration + DryRun bool + // Dispatch overrides the real worker spawn. nil + DryRun => NO dispatch + // at all (see RunOnce: a dry run must never spend a worker turn); + // nil otherwise => the defaultDispatch path (workers.GetWorker + Spawn). + Dispatch DispatchFn + Now func() time.Time +} + +// RunResult is the observable outcome of one cycle — what `scout --once` +// prints, what the heartbeat records, and what the exit code derives from. +type RunResult struct { + OK bool + Queued bool + Skipped bool + TaskID string + Title string + Status string + Detail string +} + +// Cycle statuses: the heartbeat `lastStatus` vocabulary. scout-status +// renders them and operators triage on them, so the strings are part of the +// contract. +const ( + StatusQueued = "queued" + StatusDeduped = "deduped" + StatusLocked = "locked" + StatusQueueFull = "queue-full" + StatusDryRun = "dry-run" +) + +// defaultMaxQueued caps the scout backlog when the operator never set +// scout.maxQueued. Five is a deliberate ceiling, not a measurement: the +// consume loop works FIFO through one task per PR cycle, and the 2026-09-01 +// incident (12 same-goal duplicates in one day) proved an uncapped scout +// manufactures duplicates the builder cannot drain. +const defaultMaxQueued = 5 + +// defaultScoutTimeoutMinutes is the worker budget when neither --timeout +// nor config.timeoutMinutes speaks. 30 mirrors +// scripts/install-scout-launchagent.sh's TIMEOUT_MIN default so the +// LaunchAgent and a bare run behave identically. +const defaultScoutTimeoutMinutes = 30 + +// defaultScoutIntervalMinutes only guards RunLoop against interval<=0 (the +// CLI resolves --interval/config before calling). Same 30-minute default as +// the LaunchAgent's `--interval 30`. +const defaultScoutIntervalMinutes = 30 + +// RunOnce executes exactly one scout cycle. The ordering is load-bearing, +// mirrored from the TS original, and each guard names its incident: +// +// 1. lock — two overlapping scouts double-enqueue (LaunchAgent + +// manual run); a LIVE foreign holder means someone else is mid-cycle, so +// we return Status "locked", Skipped, OK — and write NOTHING, because +// clobbering the holder's heartbeat would lie to scout-status about who +// ran. +// 2. config — Scout.{Worker,Model,MaxQueued,IntervalMinutes} +// defaults; flags/env passed in opts win. +// 3. queue depth — checked BEFORE dispatch on purpose: dispatch is the +// expensive step (a real worker run costs minutes and tokens), and a +// queue-full cycle that dispatched first paid full price to throw the +// answer away. +// 4. prompt → dispatch → extract → parse — a dispatch ERROR or an +// unparseable payload falls back to FallbackTask (docs/SCOUT.md: "so +// the queue never starves"); only the queue write can fail the cycle. +// 5. enqueue — ErrAlreadyQueued is a DEDUP HIT, not a failure: the +// deterministic per-UTC-day fallback id relies on it (a random id per +// cycle accumulated 12 same-goal duplicates on 2026-09-01). +// 6. heartbeat — every non-locked outcome records itself, including +// queue-full and enqueue failures: scout-status's staleness gate must +// see that the scout ALIVE-checked the queue, not guess death from +// silence. +// +// DryRun performs every step except EnqueueTask/WriteHeartbeat and reports +// Status "dry-run"; with no injected Dispatch it also skips the worker call +// entirely — `scout --once --dry-run` is documented (docs/SCOUT.md) as the +// deterministic-fallback preview with NO AI call, and a preview must never +// spend a real dispatch when no seam provides a fake one. +func RunOnce(opts RunOptions) (RunResult, error) { + failed := RunResult{Status: "failed"} + repo := strings.TrimSpace(opts.RepoPath) + if repo == "" { + return failed, errors.New("scout: RepoPath is required") + } + now := time.Now + if opts.Now != nil { + now = opts.Now + } + + if !AcquireScoutLock(repo, now()) { + // Deliberately no ReleaseScoutLock (we do not own it) and no + // heartbeat (see guard 1): the holder owns both files this window. + return RunResult{OK: true, Skipped: true, Status: StatusLocked, + Detail: fmt.Sprintf("live scout lock held by pid %d", ReadScoutLockPid(repo))}, nil + } + defer ReleaseScoutLock(repo) + + cfg, err := config.Load(repo) + if err != nil { + return failed, err + } + var sc *config.ScoutConfig + if cfg.Scout != nil { + sc = cfg.Scout + } + worker := opts.Worker + if worker == "" && sc != nil { + worker = sc.Worker + } + if worker == "" { + worker = cfg.Worker // repo-wide default ("omp"); never empty in practice + } + model := opts.Model + if model == "" && sc != nil { + model = sc.Model + } + timeout := opts.Timeout + if timeout <= 0 { + minutes := cfg.TimeoutMinutes + if minutes <= 0 { + minutes = defaultScoutTimeoutMinutes + } + timeout = time.Duration(minutes) * time.Minute + } + maxQueued := defaultMaxQueued + if sc != nil && sc.MaxQueued != nil && *sc.MaxQueued >= 1 { + maxQueued = int(*sc.MaxQueued) + } + + // done/failed rows are excluded on purpose: the cap protects the + // consume backlog, and terminal rows accumulate forever otherwise. + depth := len(queue.ListTasks(repo, queue.StatusPending)) + len(queue.ListTasks(repo, queue.StatusClaimed)) + if depth >= maxQueued { + res := RunResult{OK: true, Skipped: true, Status: StatusQueueFull, + Detail: fmt.Sprintf("queue depth %d >= maxQueued %d (pending+claimed)", depth, maxQueued)} + writeCycleHeartbeat(repo, res, worker, sc) + return res, nil + } + + prompt := BuildScoutPrompt(repo, cfg, nil) + + // Dispatch resolution (see RunOptions.Dispatch): the live path binds a + // repo-aware closure over the worker runtime; dry-run without a seam + // dispatches nothing. + dispatch := opts.Dispatch + if dispatch == nil && !opts.DryRun { + w, t := worker, timeout + dispatch = func(_, model, prompt string, _ time.Duration) (string, error) { + return defaultDispatch(repo, w, model, prompt, t) + } + } + var whys []string + var task *ScoutTask + if dispatch != nil { + raw, derr := dispatch(worker, model, prompt, timeout) + if derr != nil { + // Fallback policy (docs/SCOUT.md): a missing binary / dead + // worker must not starve the queue; record WHY so the heartbeat + // stays diagnosable. + whys = append(whys, fmt.Sprintf("dispatch failed: %v", derr)) + } else if payload := ExtractScoutPayload(raw, worker); payload != nil { + task = ParseScoutOutput(*payload) + } + if task == nil && derr == nil { + whys = append(whys, "unparseable scout output") + } + } else { + whys = append(whys, "no dispatch (dry-run)") + } + if task == nil { + fb := FallbackTask(prompt, now()) + task = &fb + } + + res := RunResult{OK: true, TaskID: task.ID, Title: task.Title} + if opts.DryRun { + res.Status = StatusDryRun + res.Detail = strings.Join(whys, "; ") + appendDetail(&res, fmt.Sprintf("prompt %d chars, would enqueue %s %q (no queue/heartbeat writes)", + len(prompt), task.ID, task.Title)) + return res, nil + } + + _, qerr := queue.EnqueueTask(repo, queue.EnqueueInput{ + ID: task.ID, + Title: task.Title, + Goal: task.Goal, + AcceptanceCriteria: task.Criteria, + PrdMarkdown: task.PRDMarkdown, + Source: "scout", + }) + var already queue.ErrAlreadyQueued + switch { + case qerr == nil: + res.Queued = true + res.Status = StatusQueued + res.Detail = fmt.Sprintf("enqueued %s %q", task.ID, task.Title) + case errors.As(qerr, &already): + // Dedup hit: the per-day fallback id (or a manual re-run of the + // same id) already owns the slot. Success, not failure — the queue + // holds the task; the NEXT day's fallback id gets a fresh slot. + res.Skipped = true + res.Status = StatusDeduped + res.Detail = fmt.Sprintf("task %s already queued", task.ID) + default: + res.OK = false + res.Status = "enqueue-failed" + res.Detail = qerr.Error() + writeCycleHeartbeat(repo, res, worker, sc) + return res, fmt.Errorf("scout enqueue %s: %w", task.ID, qerr) + } + for _, w := range whys { + res.Detail += "; " + w + } + writeCycleHeartbeat(repo, res, worker, sc) + return res, nil +} + +// appendDetail joins cycle diagnostics with "; " without ever leading with +// a separator. +func appendDetail(res *RunResult, s string) { + if res.Detail == "" { + res.Detail = s + return + } + res.Detail += "; " + s +} + +// writeCycleHeartbeat records the cycle. Best-effort by design: a heartbeat +// write failure must not flip an otherwise-good cycle to failed — the queue +// row is the deliverable, the heartbeat is telemetry. +func writeCycleHeartbeat(repo string, res RunResult, worker string, sc *config.ScoutConfig) { + patch := HeartbeatPatch{ + LastTaskID: ptrIfNotEmpty(res.TaskID), + LastStatus: ptrIfNotEmpty(res.Status), + LastDetail: ptrIfNotEmpty(res.Detail), + Worker: ptrIfNotEmpty(worker), + } + if sc != nil { + patch.IntervalMinutes = sc.IntervalMinutes + } + _, _ = WriteHeartbeat(repo, patch) +} + +// ptrIfNotEmpty models JSON.stringify's drop-on-undefined for the heartbeat +// pointers (an empty Detail must OMIT the key, not serialize ""). +func ptrIfNotEmpty(s string) *string { + if s == "" { + return nil + } + return &s +} + +// defaultDispatch is the real worker path: GetWorker, then Spawn one +// headless run in the repo. An empty ResultText with a nonzero ExitCode is +// an ERROR, never a silent empty payload — swallowing it would route a dead +// worker (stale binary, missing CLI, killed child) through the same +// fallback branch as chatty-but-useless output and hide the exit code from +// the heartbeat detail (the stale-binary class, loops 285-290). +func defaultDispatch(repo, worker, model, prompt string, timeout time.Duration) (string, error) { + w, err := workers.GetWorker(worker) + if err != nil { + return "", err + } + res := w.Spawn(workers.WorkerSpawnOptions{ + Prompt: prompt, + Cwd: repo, + Model: model, + TimeoutMs: int(timeout / time.Millisecond), + }) + if res.ResultText == "" && res.ExitCode != 0 { + detail := res.ErrorText + if detail == "" { + detail = "no output" + } + return "", fmt.Errorf("worker %q exited %d (timedOut=%v): %s", worker, res.ExitCode, res.TimedOut, detail) + } + return res.ResultText, nil +} + +// RunLoop is the `--interval` daemon: one immediate cycle, then one per +// tick until stop closes. The scout's resilience model is "keep ticking": a +// failed cycle logs and waits for the next tick — the lock, the per-day +// dedup id, and maxQueued bound any real damage, while exiting on the first +// error would hand operators a dead LaunchAgent with one log line to show +// for it. A slow cycle does not stack work: time.Ticker drops ticks while +// RunOnce runs, which is exactly right for a fixed-cadence researcher. +func RunLoop(repo string, interval time.Duration, stop <-chan struct{}, opts RunOptions) { + if interval <= 0 { + interval = defaultScoutIntervalMinutes * time.Minute + } + opts.RepoPath = repo + cycle := func() { + res, err := RunOnce(opts) + if err != nil { + fmt.Fprintf(os.Stderr, "[scout] %s: %v\n", res.Status, err) + return + } + fmt.Printf("[scout] %s %s\n", res.Status, res.Detail) + } + cycle() + t := time.NewTicker(interval) + defer t.Stop() + for { + select { + case <-stop: + return + case <-t.C: + cycle() + } + } +} diff --git a/internal/scout/run_test.go b/internal/scout/run_test.go new file mode 100644 index 00000000..22b3ea1a --- /dev/null +++ b/internal/scout/run_test.go @@ -0,0 +1,315 @@ +package scout + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/FreePeak/devagent/internal/queue" +) + +// Ported behavior tests for the live cycle (run.go, FR-SCOUT-01 revival). +// Every test injects Dispatch — the real worker CLI is NEVER spawned here +// (the repo's hermetic rule); defaultDispatch's contract is checked at its +// seam, not against a live omp. + +var scoutTestNow = time.Date(2026, 9, 14, 10, 0, 0, 0, time.UTC) + +// cannedScoutPayload is one valid ---TASK---/---PRD--- block in the exact +// shape ParseScoutOutput demands (goal with the "Goal:" prefix, id/title +// present). +const cannedScoutPayload = `---TASK--- +id: SCOUT-TEST-IDEA +title: Add scout dry-run summary line +goal: Goal: Print a one-line summary of what a dry-run cycle would enqueue. +criteria: summary printed to stdout; no files written +---PRD--- +# Add scout dry-run summary line +## Goal +One cycle, one line. +## Acceptance criteria +- summary printed to stdout +` + +func fixedDispatch(raw string) DispatchFn { + return func(worker, model, prompt string, timeout time.Duration) (string, error) { + return raw, nil + } +} + +func testOpts(repo string, dispatch DispatchFn) RunOptions { + return RunOptions{ + RepoPath: repo, + Worker: "opencode", + Dispatch: dispatch, + Now: func() time.Time { return scoutTestNow }, + } +} + +func mustRun(t *testing.T, opts RunOptions) RunResult { + t.Helper() + res, err := RunOnce(opts) + if err != nil { + t.Fatalf("RunOnce: %v (res=%+v)", err, res) + } + return res +} + +func countQueue(t *testing.T, repo string) int { + t.Helper() + return len(queue.ListTasks(repo, "")) +} + +// devagentFiles counts everything under .devagent: the "nothing written" +// assertions go on real files, not on our own struct echo. +func devagentFiles(t *testing.T, repo string) []string { + t.Helper() + var out []string + root := filepath.Join(repo, ".devagent") + err := filepath.WalkDir(root, func(p string, d os.DirEntry, err error) error { + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + if !d.IsDir() { + out = append(out, p) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + return out +} + +func TestRunOnceEnqueuesOnceWithSidecarAndHeartbeat(t *testing.T) { + repo := tmpRepo(t) + var gotWorker, gotModel string + var gotPrompt string + var gotTimeout time.Duration + opts := testOpts(repo, func(worker, model, prompt string, timeout time.Duration) (string, error) { + gotWorker, gotModel, gotPrompt, gotTimeout = worker, model, prompt, timeout + return cannedScoutPayload, nil + }) + res := mustRun(t, opts) + + if !res.OK || !res.Queued || res.Status != StatusQueued { + t.Fatalf("res = %+v", res) + } + if res.TaskID != "SCOUT-TEST-IDEA" || res.Title != "Add scout dry-run summary line" { + t.Errorf("task = %q / %q", res.TaskID, res.Title) + } + tasks := queue.ListTasks(repo, queue.StatusPending) + if len(tasks) != 1 { + t.Fatalf("pending tasks = %d, want 1", len(tasks)) + } + task := tasks[0] + if task.Source == nil || *task.Source != "scout" { + t.Errorf("source = %v, want scout", task.Source) + } + if len(task.AcceptanceCriteria) != 2 { + t.Errorf("criteria = %v", task.AcceptanceCriteria) + } + if task.PrdPath == nil { + t.Fatal("PRD sidecar path missing on the queue row") + } + sidecar := filepath.Join(repo, ".devagent", "prds", "SCOUT-TEST-IDEA.md") + if raw, err := os.ReadFile(sidecar); err != nil || !strings.Contains(string(raw), "## Goal") { + t.Fatalf("sidecar %s: %v", sidecar, err) + } + // The seam received the resolved worker + the built prompt + the + // default 30-minute budget (config timeoutMinutes). + if gotWorker != "opencode" || gotModel != "" || !strings.Contains(gotPrompt, "You are the DevAgent SCOUT") { + t.Errorf("dispatch args: worker=%q model=%q prompt has scout header=%v", gotWorker, gotModel, strings.Contains(gotPrompt, "SCOUT")) + } + if gotTimeout != 30*time.Minute { + t.Errorf("timeout = %v, want 30m", gotTimeout) + } + hb := ReadHeartbeat(repo) + if hb == nil || hb.LastStatus == nil || *hb.LastStatus != StatusQueued || + hb.LastTaskID == nil || *hb.LastTaskID != "SCOUT-TEST-IDEA" || + hb.Worker == nil || *hb.Worker != "opencode" { + t.Fatalf("heartbeat = %+v", hb) + } + // Cycle 6 of the contract: the lock is released for the next cycle. + if _, err := os.Stat(scoutLockPath(repo)); !os.IsNotExist(err) { + t.Errorf("scout lock still present after cycle: %v", err) + } +} + +func TestRunOnceSecondCycleIsDedupedNotFailed(t *testing.T) { + repo := tmpRepo(t) + // Same payload twice: the second enqueue must surface ErrAlreadyQueued + // as a DEDUP HIT (OK, Skipped) — the deterministic per-day fallback id + // depends on this being a non-failure (2026-09-01 incident). + first := mustRun(t, testOpts(repo, fixedDispatch(cannedScoutPayload))) + if !first.Queued { + t.Fatalf("first cycle = %+v", first) + } + second := mustRun(t, testOpts(repo, fixedDispatch(cannedScoutPayload))) + if !second.OK || !second.Skipped || second.Queued || second.Status != StatusDeduped { + t.Fatalf("second cycle = %+v", second) + } + if second.TaskID != "SCOUT-TEST-IDEA" { + t.Errorf("dedup must still name the task: %+v", second) + } + if n := countQueue(t, repo); n != 1 { + t.Errorf("queue rows = %d, want 1", n) + } +} + +func TestRunOnceLiveForeignLockWritesNothing(t *testing.T) { + repo := tmpRepo(t) + // A LIVE holder (this test pid — AcquireScoutLock checks liveness, not + // ownership) must yield Status locked with ZERO writes: clobbering the + // holder's heartbeat would misattribute the run in scout-status. + lock := scoutLockPath(repo) + if err := os.MkdirAll(filepath.Dir(lock), 0o755); err != nil { + t.Fatal(err) + } + line := fmt.Sprintf(`{"pid":%d,"at":%d}`, os.Getpid(), scoutTestNow.UnixMilli()) + if err := os.WriteFile(lock, []byte(line+"\n"), 0o644); err != nil { + t.Fatal(err) + } + res := mustRun(t, testOpts(repo, fixedDispatch(cannedScoutPayload))) + if !res.OK || !res.Skipped || res.Status != StatusLocked { + t.Fatalf("res = %+v", res) + } + if n := countQueue(t, repo); n != 0 { + t.Errorf("locked cycle enqueued %d tasks", n) + } + if hb := ReadHeartbeat(repo); hb != nil { + t.Errorf("locked cycle wrote a heartbeat: %+v", hb) + } + // The cycle must not steal or delete the holder's lock either. + if _, err := os.Stat(lock); err != nil { + t.Errorf("foreign lock must survive: %v", err) + } +} + +func TestRunOnceQueueFullNeverDispatches(t *testing.T) { + repo := tmpRepo(t) + // Dispatch is the expensive step (minutes + tokens); a queue-full + // cycle must decide BEFORE paying for it. + if err := os.WriteFile(filepath.Join(repo, "devagent.json"), + []byte(`{"scout":{"maxQueued":1}}`), 0o644); err != nil { + t.Fatal(err) + } + if _, err := queue.EnqueueTask(repo, queue.EnqueueInput{ID: "OLD-1", Title: "old", Goal: "Goal: old"}); err != nil { + t.Fatal(err) + } + calls := 0 + res := mustRun(t, testOpts(repo, func(worker, model, prompt string, timeout time.Duration) (string, error) { + calls++ + return cannedScoutPayload, nil + })) + if !res.OK || !res.Skipped || res.Status != StatusQueueFull { + t.Fatalf("res = %+v", res) + } + if calls != 0 { + t.Errorf("queue-full cycle dispatched %d times", calls) + } + if n := countQueue(t, repo); n != 1 { + t.Errorf("queue rows = %d, want 1", n) + } + // queue-full still heartbeats: scout-status's staleness gate must see + // the alive-check, not read a skip as a death. + hb := ReadHeartbeat(repo) + if hb == nil || hb.LastStatus == nil || *hb.LastStatus != StatusQueueFull { + t.Fatalf("heartbeat = %+v", hb) + } +} + +func TestRunOnceUnparseablePayloadEnqueuesDailyFallback(t *testing.T) { + repo := tmpRepo(t) + res := mustRun(t, testOpts(repo, fixedDispatch("I feel inspired today, no markers here."))) + wantID := "SCOUT-20260914-fallback" + if !res.OK || !res.Queued || res.Status != StatusQueued || res.TaskID != wantID { + t.Fatalf("res = %+v, want queued %s", res, wantID) + } + if !strings.Contains(res.Detail, "unparseable") { + t.Errorf("detail must record WHY: %q", res.Detail) + } + if n := countQueue(t, repo); n != 1 { + t.Errorf("queue rows = %d, want 1", n) + } +} + +func TestRunOnceDispatchErrorFallsBackNotStarves(t *testing.T) { + repo := tmpRepo(t) + // docs/SCOUT.md failure policy: a dead worker must not starve the + // queue; the heartbeat detail keeps the cause diagnosable. + res := mustRun(t, testOpts(repo, func(worker, model, prompt string, timeout time.Duration) (string, error) { + return "", errors.New("Unknown worker: nope") + })) + if !res.OK || !res.Queued || res.TaskID != "SCOUT-20260914-fallback" { + t.Fatalf("res = %+v", res) + } + if !strings.Contains(res.Detail, "dispatch failed") { + t.Errorf("detail = %q", res.Detail) + } +} + +func TestRunOnceDryRunWritesNothing(t *testing.T) { + repo := tmpRepo(t) + opts := testOpts(repo, fixedDispatch(cannedScoutPayload)) + opts.DryRun = true + res := mustRun(t, opts) + if !res.OK || res.Queued || res.Status != StatusDryRun { + t.Fatalf("res = %+v", res) + } + if !strings.Contains(res.Detail, "prompt ") || !strings.Contains(res.Detail, "SCOUT-TEST-IDEA") { + t.Errorf("dry-run must preview prompt + would-enqueue: %q", res.Detail) + } + if files := devagentFiles(t, repo); len(files) != 0 { + t.Errorf("dry-run left files: %v", files) + } +} + +func TestRunOnceDryRunWithoutSeamSkipsDispatch(t *testing.T) { + repo := tmpRepo(t) + // `scout --once --dry-run` is documented (docs/SCOUT.md) as the + // deterministic-fallback preview with NO AI call: with no injected + // seam RunOnce must not resolve the real defaultDispatch either. + opts := RunOptions{RepoPath: repo, Worker: "opencode", DryRun: true, + Now: func() time.Time { return scoutTestNow }} + res := mustRun(t, opts) + if res.Status != StatusDryRun || !strings.Contains(res.Detail, "no dispatch") { + t.Fatalf("res = %+v", res) + } + if !strings.Contains(res.Detail, "SCOUT-20260914-fallback") { + t.Errorf("no-dispatch dry-run previews the fallback: %q", res.Detail) + } + if files := devagentFiles(t, repo); len(files) != 0 { + t.Errorf("dry-run left files: %v", files) + } +} + +func TestRunLoopTicksUntilStop(t *testing.T) { + repo := tmpRepo(t) + // RunLoop is synchronous over RunOnce, so the counter needs no lock: + // dispatch #2 closes stop and the next select returns. Same payload + // twice also proves the loop keeps ticking past a deduped cycle. + stop := make(chan struct{}) + n := 0 + dispatch := func(worker, model, prompt string, timeout time.Duration) (string, error) { + n++ + if n == 2 { + close(stop) + } + return cannedScoutPayload, nil + } + RunLoop(repo, 5*time.Millisecond, stop, testOpts("", dispatch)) + if n != 2 { + t.Errorf("cycles = %d, want 2", n) + } + if count := len(queue.ListTasks(repo, queue.StatusPending)); count != 1 { + t.Errorf("queue rows = %d, want 1", count) + } +} diff --git a/internal/scout/scout.go b/internal/scout/scout.go index de712319..9681937e 100644 --- a/internal/scout/scout.go +++ b/internal/scout/scout.go @@ -1,9 +1,9 @@ // scout.go is the deterministic core of src/scout.ts: task-id resolution, // payload extraction, the golden replay harness, heartbeat persistence, the // single-instance lock, and prompt assembly. Live worker dispatch -// (runScoutOnce/runScoutLoop) stays in TS until the queue (FR-GO-04) and -// doc-sync (FR-GO-05) ports land. See extract.go for the canonical package -// comment. +// (runScoutOnce/runScoutLoop) is ported in run.go (FR-SCOUT-01 revival, +// 2026-09-14) on top of the queue (FR-GO-04 #194) and worker-runtime +// packages. See extract.go for the canonical package comment. package scout