diff --git a/CLAUDE.md b/CLAUDE.md index cb03a3e..fc1298b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,9 +44,10 @@ every documented path goes through the script. ### op-agent -Polls work packages, driven by `@opilot` comments. There are two command words — -**`@opilot build`** (alias `fix`) and **`@opilot create wp`** (long form -`create work package`); every other word is chat. The words `build` replaced — +Polls work packages, driven by `@opilot` comments. There are three command words — +**`@opilot build`** (alias `fix`), **`@opilot create wp`** (long form +`create work package`) and **`@opilot health`** (see "Health check" below); every +other word is chat. The words `build` replaced — `ship`, `plan`, `approve`, `prototype`, `pr`, `implement` — are chat too, so an old habit gets an answer that names the real command rather than silence. `create` without the noun is chat as well: alone it could mean a branch, a PR or a comment, @@ -84,7 +85,7 @@ has read. **`ship` always names the approach before it writes code.** The writer opens every invited plan call with a third first-line sentinel beside `NEEDS_INFO` and `REPOS:` — `OPTIONS`, then one pipe-delimited line per approach -(`Prompts::OPTIONS_CONTRACT`). Most tickets have exactly one sensible approach: +(`Prompts::Planner::OPTIONS_CONTRACT`). Most tickets have exactly one sensible approach: the writer names it in a single option line, then continues straight into the plan in the *same* response, so a one-shape ticket still costs exactly one plan call — just with a stated approach instead of a silent one. @@ -161,7 +162,7 @@ id names no WP dir. It also **auto-fixes failed CI** (always on). Once checks complete with ≥1 failure, the detail (annotations, output summaries, failed-job log tails) is cached to -`ci.json`, fixed with `Prompts.fix_ci`, committed and pushed. The trigger is the +`ci.json`, fixed with `Prompts::PrAuthor.fix_ci`, committed and pushed. The trigger is the **head SHA**: `gh_pr.json` tracks `ci_acted_sha` (once per commit) and `ci_attempts` (`OPILOT_CI_MAX_ATTEMPTS`, default 5; past the cap it posts a one-time "needs a human" note and sets `ci_gave_up`). It acts on the *first* failure rather than @@ -177,9 +178,9 @@ which would truncate a big CI matrix). an LLM call is spent only on real mentions. The trigger is a prompt addressed to opilot, not a review pass over other people's work; what differs is write access, so these intents are `reply_only` — read-only fetch, answered in text -(`Prompts.pr_review`), never pushed. Applicable code still lands: for lines already +(`Prompts::PrAdvisor.pr_review`), never pushed. Applicable code still lands: for lines already in the diff the review emits a `SUGGESTIONS:` block -(`Prompts::SUGGESTION_CONTRACT`) that `GhAgent#post_suggestions` posts as a review +(`Prompts::PrAdvisor::SUGGESTION_CONTRACT`) that `GhAgent#post_suggestions` posts as a review of inline `suggestion` comments (anchored to the head SHA, `event: COMMENT`) — the author applies each with one click; a bad line range 422s and falls back to prose. A failing CI run is read too (keyed by head SHA), so "why is CI red?" gets an @@ -205,6 +206,9 @@ nothing" and "not scanning" look identical in the log. - **`dev commit ...`** — stop after the local commit. A later `ship` finds the branch (`branch_has_commits?`) and goes straight to publish. - **`dev plan ...`** — stop at the approved plan. +- **`dev health ...`** — the `@opilot health` check (`HealthRunner` → + `HealthCheck`), printed to the terminal; it posts nothing, so a prompt change can + be tuned on a real work package first. - **`dev refresh ...`** — refresh shipped PRs (`PrRunner`). A URL is matched against local state, else *adopted* via the OpenProject ticket link in the description's top 15 lines; a WP id with no state is *discovered* by searching each @@ -221,7 +225,7 @@ nothing" and "not scanning" look identical in the log. is posted as a 🤖 comment and the cutoff advances so gh-agent doesn't re-handle it. - **`chat [message]`** — free read-only conversation over the local mirrors, never fetching, planning, or shipping. `.opilot/` is mounted read-only at `/state`, so - `Prompts.free_chat` orients the LLM at the layout and it Greps/Reads from there. + `Prompts::Advisor.free_chat` orients the LLM at the layout and it Greps/Reads from there. Fresh per-run session; needs no tokens or allowlist. **It reads only what another run already mirrored** — a `dev` verb or an agent tick. Nothing seeds the cache on its own: `op wp get` prints a work package but caches nothing, so a WP opilot has @@ -323,6 +327,7 @@ before touching anything under `lib/opilot/pd/`. ./opilot dev build ... ./opilot dev commit ... # stop after the local commit — no push, no PR ./opilot dev plan ... # stop at the approved plan +./opilot dev health ... # check the WP for drift; prints, posts nothing # Refresh shipped PRs: merge base, fix CI, address new comments, push (confirmed) ./opilot dev refresh ... @@ -490,13 +495,16 @@ bare `docker compose run …` works from the repo root. | `gh_pr_cache.rb` | PR-content cache (`pr.json`, keyed by `updated_at`), mention matching, fresh-comment filtering, CI cache (`ci.json`, keyed by head SHA) | | `gh_agent.rb` | `gh-agent` loop — own PRs: reply + code + push; upstream: read-only. `#sources` keeps the banner honest | | `fix_runner.rb` | Terminal `dev build`/`commit`/`plan` — one pipeline named by where it stops | +| `health_check.rb` | `@opilot health` and `dev health`: the fact rules, the one LLM call, the composed report | +| `health_runner.rb` | Terminal `dev health` — prints `HealthCheck`'s report, posts nothing | | `pr_runner.rb` | Terminal `dev refresh`, and gh-agent's `@opilot refresh` via `#refresh_one` | | `op_runner.rb` | Terminal `op` — one command per `Clients::OpenProject` method it exposes. Three rules hold: **stdout is data** (JSON only, diagnostics to stderr, never `log_script`), every action **reads except `wp create`**, and **`--type` is required of every payload**. `wp form --required` is how you learn what else a project demands. The file header argues all three — read it there rather than re-deriving them | | `harness.rb` | HTTP client to the harness container; per-WP session IDs | +| `roles.rb` | Loads `prompts/*.yml`, the roles the model plays (grant, model, memory) — every LLM call names one via `Helpers#llm` | | `appsignal_runner.rb` | Terminal `appsignal` — incident → work package, then hands off to `FixRunner#ship_ids`. Owns the local-model guard, and every preflight runs before the create | | `clients/appsignal.rb` | AppSignal's GraphQL + V2 tracing APIs, assembled into one incident: metadata, the request payload, and the backtrace. The runner's client, never a tool for the model | | `clients/inference_gw.rb` | inference-gw's `GET /upstream` — the pinned inference address, which is what `Context#inference_privacy` judges | -| `prompts.rb` | All LLM prompts in one place. Everything opilot publishes (WP comments, PR replies and descriptions, plans, spec proposals) is written in ASD-STE100 Simplified Technical English — stated once in `Prompts::PLAIN_ENGLISH` and pulled into the shared blocks (`OP_COMMENT_FORMAT`, `REPLY_CONTRACT`, `TERMINAL_REPLY`, `#plan_skeleton`), never re-worded per prompt. Code and commit messages are out of scope | +| `prompts.rb`, `prompts/` | All LLM prompts. Each role is a pair in `prompts/`: `.yml` (grant, model, memory and charter) and `.rb`, the module holding that role's builders and whatever only that role uses (`Prompts::Planner.plan`, `Planner::OPTIONS_CONTRACT`, `Auditor::HEALTH_CONTRACT`, `Advisor::LENSES`). What several roles share is in `prompts/_shared.rb`: the text blocks (from `prompts/_blocks/`), `Prompt`, `Prompts.charter`, and the `Sections` helpers; `prompts.rb` only loads them. A builder returns a `Prompts::Prompt` tagged with its role, and `Helpers#llm` refuses one sent under another role. Everything opilot publishes (WP comments, PR replies and descriptions, plans, spec proposals) is written in ASD-STE100 Simplified Technical English — stated once in `Prompts::PLAIN_ENGLISH` and pulled into the shared blocks (`OP_COMMENT_FORMAT`, `REPLY_CONTRACT`, `TERMINAL_REPLY`, `Planner.plan_skeleton`), never re-worded per prompt. Code and commit messages are out of scope | | `publish.rb` | Pushes branches to the fork; opens cross-repo draft PRs via Octokit | | `clients/openproject.rb` | OpenProject REST API. `#add_comment` is the funnel every WP comment passes through, so it demotes markdown headings to bold — the activity tab is a narrow column | | `clients/github.rb` | GitHub API (Octokit) | @@ -658,7 +666,7 @@ up front rather than after a full plan and implement run (`build`/`plan` need no `Helpers#require_clone!` is called from **`Helpers#worktree`**, the funnel every git operation goes through, because a per-command check gets forgotten — `./opilot` only *warns* when a clone fails, and `Git.open`'s error names neither the repo nor -the fix. `#ensure_claude!` fails with "start the container" at every entry point that +the fix. `#ensure_harness!` fails with "start the container" at every entry point that will call the LLM, not mid-run with a connection error. `:ship` (`@opilot build`, alias `fix`) is the fix intent: it plans and @@ -670,12 +678,46 @@ does; only the word people type is `build`, and `./opilot dev build` takes the s word for the same operation, so one thing has one name wherever it is typed. The difference is who is watching: the terminal verbs have an operator at the console. Chat lenses (`grill`, `summarize`) are preset instructions over -the ordinary `:chat` intent (`Prompts::LENSES`), with trailing text as a focus hint. +the ordinary `:chat` intent (`Prompts::Advisor::LENSES`), with trailing text as a focus hint. + +**Health check (`@opilot health [focus]`, intent `:health`)** reports where a work +package is inconsistent with itself. It is **not a lens**: it needs a fact pass, its +own prompt, a parser and a Ruby-composed reply (`HealthCheck`). Two layers: + +- **Facts** (`HealthCheck#facts_for`, no LLM) — only rules on exact, + language-independent data, because the prompt tells the model not to dispute them: + status meaning from `GET /statuses` (`isClosed`, `isDefault`, looked up by name — + names are unique), relation labels, timestamps, linked-PR flags + (`/work_packages/:id/github_pull_requests`, which needs `show_github_content`), + and commits naming the WP on each registry base (`git log --grep` as a prefilter, + then a word-boundary match, so `#5994` never matches `#59942`). PR and commit rules + fire only on the **default** status: "Developed" is open and has merged PRs. + Anything heuristic — a reopen report, which status-name matching would confuse + with a subject edit — goes to the model as `history[]` instead. +- **Descendants** (`HealthCheck#descendants`) — the whole subtree at any depth from + ONE paginated `ancestor` filter query, capped at `MAX_DESCENDANTS`, written to + `descendants.json`. A list element embeds nothing, so status and type come from + `_links..title`. The tree rules (closed over open at any depth, a subtree all + closed, stale open descendants) **replace** the direct-child rules; a failed read + falls back to them and says so. The model gets the tree to check the description's + scope against the descendants' subjects — no per-descendant LLM call. +- **Judgement** — one read-only call **without the WP session**, so a check is + independent of earlier chat turns. The answer is `Prompts::Auditor::HEALTH_CONTRACT` + (`BEGIN HEALTH` … `END HEALTH`, `FINDING:`/`GAP:` lines), read by + `Helpers.parse_health`: the END marker detects truncation (one retry, then a + failure note), and a finding without evidence is dropped. + +The reply is composed in Ruby (`HealthCheck#report`), for `#post_options`' reason, +and always lists **Not checked** — skipped attachments, an unreadable PR list or +status list, the model's own gaps — because silence about an input reads as "it is +fine". A **public** reply drops any finding whose evidence names an internal +comment's timestamp: the evidence is printed verbatim, so the prompt rule alone +is not enough. Like `create wp`, the handler answers its own failure. **A chat answer can carry an ARTIFACT — a diagram or a long report — published as a secret gist and linked from the comment.** Three surfaces offer one, and they are deliberately **asymmetric**: a gh-agent PR reply (`Prompts::MERMAID_NOTE` in -`gh_reply`/`pr_review`) and a plan's Approach section (`Prompts#plan_skeleton`) are +`gh_reply`/`pr_review`) and a plan's Approach section (`Prompts::Planner.plan_skeleton`) are **prompt-only**, because GitHub and gists render a ```mermaid fence themselves. Only op-agent chat has machinery behind it. @@ -695,7 +737,7 @@ attached a flowchart of the same thing, and the reader read it twice. `Harness::TOOLS_READ` and `pi-guards.ts` confines writes to `/repos`, so the model cannot write a file — and that read-only contract for prompt-injectable phases is enforced in the guard, not in the prompt, so this is not a limitation to route -around. `Prompts.artifact_block` states the `BEGIN ARTIFACT` … `END ARTIFACT` shape, +around. `Prompts::Advisor.artifact_block` states the `BEGIN ARTIFACT` … `END ARTIFACT` shape, `Helpers.parse_artifacts` reads it, and `Agent#publish_artifacts` mirrors, caps and publishes. The block sits at the **END** of the answer: everything shares one output budget, so a cut-off response loses the artifact and keeps the comment. The parser @@ -736,7 +778,7 @@ duplicate create. `Pull#intent_from_comments` drops a non-allowlisted trigger whenever a list exists, so every create that reaches the handler is from a listed user. - **Every work package it will create comes out of ONE LLM call, gated by - `NEEDS_INFO`** (`Prompts.create_wp`, `Agent#write_work_packages`). When the request + `NEEDS_INFO`** (`Prompts::WpWriter.create_wp`, `Agent#write_work_packages`). When the request points at nothing in the thread, questions are the only acceptable answer. N answers cost the same one call as one, and a call per work package would not see the others — two of them could write the same suggestion, and the duplicate could not be deleted. @@ -852,12 +894,16 @@ globally unique, so `pr_reviews/` is flat. │ ├── item.json # WP metadata + poll cache + acted_at + item_version │ │ # + refusal_noted_at (the one allowlist note per WP) │ │ # + create_wp_refusal_noted_at (the one `create wp` off note) -│ │ # + pictures[] / pictures_skipped[] (see pictures/ below) +│ │ # + pictures[] (with created_at) / pictures_skipped[] +│ │ # + history[] (field changes, no comment text) and +│ │ # description_changed_at, for `health` │ │ # + pictures_pending (an attachment read failed — │ │ # suppresses the cache until a run finishes) │ ├── pictures/ # every picture the WP shows, mirrored so the LLM can `read` │ │ # one; -., pruned to match the WP │ ├── related.json # related WPs pulled in at plan time +│ ├── health.json # the last health check's facts (HealthCheck#facts_for) +│ ├── descendants.json # the subtree the last health check read (HealthCheck#descendants) │ ├── plan.md # implementation plan (shared across target repos) │ ├── artifacts// # markdown a chat answer produced, keyed by the trigger's │ │ # comment_at — the local copy of what was gisted @@ -896,6 +942,15 @@ globally unique, so `pr_reviews/` is flat. Runner POSTs to `http://harness:47291` with headers: +- `X-Harness-Role` — **required**. The role the call is made as (`Helpers#llm`). + `server.js` loads the same `prompts/*.yml` role files at boot (`loadRoles`, copied into + the image by `Dockerfile.harness`) and refuses a missing role (400), an unknown + one, a request with no tool grant — which would give pi its default tools, + write included — and a grant the role does not allow (403): a role without + `mcp` accepts only its base grant, a role with it accepts the base plus the + `op_query`/`gh_query` variants. The runner still chooses both the role and the + grant, so this does not stop a compromised runner; it stops a runner bug from + sending a write grant under a read role. - `X-Harness-Tools` — built by `Harness.tools_for`, which appends `op_query` then `gh_query` in that fixed order when each flag is on. `server.js` allowlists the resulting **eight exact strings** (`ALLOWED_TOOL_GRANTS`); @@ -904,11 +959,33 @@ Runner POSTs to `http://harness:47291` with headers: `test/js/models_json_test.js` makes adding one deliberate. The base is `"read,grep,find,ls,bash"` (planning/chat) or `"read,grep,find,ls,bash,write,edit"` (implementation), each with a `,op_query` - variant sent by the specific call sites `Helpers#read_tools`/`#impl_tools` - cover when `Context#op_mcp?` is on (see `MCP.md`) — most `TOOLS_READ`/`TOOLS_IMPL` - call sites keep the plain grant regardless. `server.js` rejects any other + variant sent only by the roles marked `mcp` when `Context#op_mcp?` is on + (see `MCP.md`) — the other roles keep the plain grant regardless. `server.js` rejects any other grant, so its allowlist (`ALLOWED_TOOL_GRANTS`, four strings) must stay in sync with `TOOLS_READ`/`TOOLS_IMPL`/`TOOLS_READ_OP`/`TOOLS_IMPL_OP`. +- **Every call names a role** (`Helpers#llm`). A role is a YAML file, + `lib/opilot/prompts/.yml`: `tools` (`read`/`write`), `mcp` (whether the + MCP tools join the grant), `model` (`heavy`/`light`) and `memory` + (`session`/`none`), and the role's **charter** as a `charter: |` block. `lib/opilot/roles.rb` loads them strictly — + an unknown key or value fails at boot — and a `memory: none` role given a + `session_file` raises, so health's independence from chat is structural. + + **The charter opens every prompt that orients the model** (`Prompts.charter`, + called by each builder in `lib/opilot/prompts/.rb`, beside it), followed by the rules + the **grant** carries: `READ_ONLY` for a read role, `WRITE_GRANT` (no commit, no + command, and the `git rm`/`git clean` exception) for a write role. Derived, never + pasted, so a prompt cannot state a grant its role does not hold. A follow-up turn + in the same session (`propose_revise`, a chat's second message) carries no + charter. Shared prompt text lives in `prompts/_blocks/*.md`; the reason for each + block stays on the Ruby constant that loads it, and compositions + (`REPLY_CONTRACT` + `PLAIN_ENGLISH`) stay in Ruby. + + Tests: `roles_test.rb` pins every role's tuple, checks each against + `ALLOWED_TOOL_GRANTS`, and fails if anything outside `helpers.rb`/`harness.rb` + calls `@harness.run` or names `Harness::TOOLS_*`; `prompts_test.rb` renders every + builder and checks its charter and its grant block (once, and never the other + one). Roles that look alike but differ in grant (`pr_author` vs `pr_refresher`) + stay two roles until someone decides to merge them. - `X-Harness-Model` — one model per WP for every session-bound phase (`MODEL_HEAVY`), plus `MODEL_LIGHT` for stateless one-shots (a commit subject, a PR description) — always `/` (`openrouter/anthropic/claude-sonnet-5.5`, @@ -946,7 +1023,7 @@ of it, and a runner that gives up first turns a named timeout into a bare | Variable | Purpose | |----------|---------| | `OPENPROJECT_URL` | OpenProject instance URL | -| `OPENPROJECT_TOKEN` | API token. Read access suffices for `op`/`chat` (except `op wp create`); agent mode needs write (to comment), plus `:add_work_packages` once `@opilot create wp` is enabled, `:manage_work_package_relations` for its backlink and `:manage_subtasks` to make several of them children of the source (without either link permission the work packages are still created, only unlinked — or related instead of parented); `pd` needs `:add_work_packages` | +| `OPENPROJECT_TOKEN` | API token. Read access suffices for `op`/`chat` (except `op wp create`); agent mode needs write (to comment), plus `:add_work_packages` once `@opilot create wp` is enabled, `:manage_work_package_relations` for its backlink and `:manage_subtasks` to make several of them children of the source (without either link permission the work packages are still created, only unlinked — or related instead of parented); `pd` needs `:add_work_packages`. `health` reads linked PRs only with `:show_github_content` (without it, that input is listed as not checked) | | `HARNESS_URL` | Optional; where the runner reaches the harness container (default `http://harness:47291`) | | `OP_REPO_PATH` | Optional; local openproject checkout to seed that clone from. openproject-only — other repos are configured in `repos.json` | | `GITHUB_CONTRIBUTOR_TOKEN` | The **contributor identity** — a bot account that is **not a collaborator on the canonical repos** (that lack of access is what enforces isolation). Classic token with `public_repo`, `workflow` (the lagging fork re-introduces upstream's `.github/workflows/*`, rejected without it) and `gist` (the plan gist and chat artifacts; both skipped if absent). Fine-grained tokens can't open fork→upstream PRs | diff --git a/Dockerfile.harness b/Dockerfile.harness index 5741c25..37166a2 100644 --- a/Dockerfile.harness +++ b/Dockerfile.harness @@ -12,6 +12,8 @@ RUN git config --system --add safe.directory '*' \ && git config --system core.crossFS true COPY --chown=node:node server.js pi-guards.ts pi-mcp.ts op-mcp-client.js gh-mcp-client.js pi-settings.json pi-models.json /app/ +# The role files: server.js checks each request's grant against its role. +COPY --chown=node:node lib/opilot/prompts/*.yml /app/lib/opilot/prompts/ USER node diff --git a/README.md b/README.md index b6aa638..b91f63a 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ Use OPilot to remove that friction from your development workflow. It can automa ### General audience -* **Refine work packages**: Discuss work packages in the chat via free-form chatting or preset commands like `@OPilot grill`. +* **Refine work packages**: Discuss work packages in the chat via free-form chatting or preset commands like `@OPilot grill`. `@OPilot health` checks the description against the comments, pictures, related work packages, status, linked PRs and commits, and lists what it could not check. * **Draw diagrams and write reports**: Ask for a diagram or a long report in the chat. OPilot publishes it as a secret gist and links it from the comment — a diagram is a mermaid fence, which the gist renders as a picture. Needs `OPILOT_ALLOWED_OP_USER_IDS`. * **Create work packages**: Turn a suggestion made in a comment into its own work package(s) with `@OPilot create wp `. OPilot builds the package content, creates them in the same project, and relates them back. * **[Enterprise] Run project-wide discovery**: Ask `@OPilot` anything about the reachable projects' data -- it will leverage the instance's MCP server to give you a fresh answer. diff --git a/TODO.md b/TODO.md index f79bc65..6563982 100644 --- a/TODO.md +++ b/TODO.md @@ -25,7 +25,6 @@ OPilot's roadmap. See [README.md](README.md) for what the project already does. * Inspiration: https://andrewpatterson.dev/posts/token-savings-rtk-headroom/ ## Feature ideas -* WP health check command * Replace OpenSpec with a simple list of acceptance criteria * Matrix/Element integration for a better interface & activity tracking * Nextcloud integration, so that we can load relevant data during designs diff --git a/bin/opilot b/bin/opilot index 2790819..78f7beb 100755 --- a/bin/opilot +++ b/bin/opilot @@ -21,6 +21,8 @@ require "opilot/gh_pull" require "opilot/gh_agent" require "opilot/combined_agent" require "opilot/fix_runner" +require "opilot/health_check" +require "opilot/health_runner" require "opilot/pr_runner" require "opilot/chat_runner" require "opilot/usage_runner" diff --git a/lib/opilot/agent.rb b/lib/opilot/agent.rb index 6ef47d6..4188dc3 100644 --- a/lib/opilot/agent.rb +++ b/lib/opilot/agent.rb @@ -96,6 +96,7 @@ def handle(intent) when :chat then handle_chat(intent) when :ship then handle_ship(intent) when :create_wp then handle_create_wp(intent) + when :health then handle_health(intent) end end @@ -123,13 +124,13 @@ def handle_chat(intent) # Pass the plan's path, not its text: a resumed session already holds the # plan, so re-embedding it every turn just burns tokens. plan_ref = st.plan_file.exist? ? container_path(st.plan_file) : "(no plan yet)" - prompt = Prompts.chat(item_id: st.item_id, subject: st.subject, + prompt = Prompts::Advisor.chat(item_id: st.item_id, subject: st.subject, item: container_path(st.item_file), plan: plan_ref, message: intent.text.to_s, related: related_ref(st), can_create_wp: create_wp_enabled?, can_make_artifact: artifacts_enabled?, max_artifacts: MAX_ARTIFACTS, op_mcp: @ctx.op_mcp?) - reply = @harness.run(prompt, tools: read_tools, session_file: st.session_file) + reply = llm(:advisor, prompt, session_file: st.session_file) # Only when artifacts are on: with the instructions never given, a BEGIN # ARTIFACT line is text the writer invented or quoted, and stripping it # would delete content from someone's reply. @@ -137,6 +138,19 @@ def handle_chat(intent) post_note(st.item_id, addressed(reply.strip)) unless reply.strip.empty? end + # Answers its own failure, like create wp: the reader waits for a report. + def handle_health(intent) + check = HealthCheck.new(@ctx, pull: @pull, harness: @harness, api: @api) + report = begin + check.run(intent.item_id, focus: intent.text.to_s, internal: intent.internal != false) || + "The health check could not read this work package." + rescue Harness::Error => e + log_script "Health check failed on #{wp_label(intent.item_id)}: #{e.message}" + "The health check did not finish: the model run failed. Ask again with `@opilot health`." + end + post_note(intent.item_id, addressed(report)) + end + # Take the artifacts out of a chat answer, mirror them, publish them as one # gist, and return the comment to post — the answer without the blocks, plus a # line naming what was published. @@ -284,7 +298,7 @@ def artifacts_enabled? # enforced HERE, because a prompt limit drifts and a work package can never # be deleted: "create one for every suggestion in this thread" must not be # able to mint twenty rows nobody can remove. Five also sits well inside one - # output budget — see Prompts.create_wp on why a cut-off answer is the + # output budget — see Prompts::WpWriter.create_wp on why a cut-off answer is the # failure mode to fear. MAX_CREATE_WP = 5 @@ -373,13 +387,13 @@ def project_type_names(project_id) # is a lost request, not a duplicate work package. def write_work_packages(st, request, project_name, types, related, retry_bad: true, format_note: nil) log_script "Writer: drafting work packages from #{wp_label(st.item_id)} — #{request}" - prompt = Prompts.create_wp(item_id: st.item_id, subject: st.subject, + prompt = Prompts::WpWriter.create_wp(item_id: st.item_id, subject: st.subject, item: container_path(st.item_file), request: request, project: project_name, types: Helpers.types_for_prompt(types), max: MAX_CREATE_WP, related: related, format_note: format_note) - reply = @harness.run(prompt, tools: Harness::TOOLS_READ, session_file: st.session_file).to_s + reply = llm(:wp_writer, prompt, session_file: st.session_file).to_s # Only what follows the last `ANSWER:` marker; the writer's own deliberation - # is scratch (Prompts.create_wp). Text with no marker is read whole, so an + # is scratch (Prompts::WpWriter.create_wp). Text with no marker is read whole, so an # answer that skips it still works. answer = Helpers.after_marker(reply, "ANSWER") @@ -490,7 +504,7 @@ def payloads_accepted?(st, payloads, types) # customer?"), a work package can never be deleted, and a guess would be # permanent. So the fields are named back to the reader, who can create it in # OpenProject or NAME A DIFFERENT TYPE — required-ness is per type, and their - # answer lands in this thread, which the next draft reads (Prompts.create_wp's + # answer lands in this thread, which the next draft reads (Prompts::WpWriter.create_wp's # TYPE line). Choosing another type here instead would be opilot re-classifying # somebody's work to get past a validation, on a work package nobody can delete. # @@ -740,7 +754,7 @@ def already_created_note(records) # # The shape is always STATED, never implied: whether an offshoot is a child of # this work package or a peer beside it is the writer's per-block decision - # (Prompts.create_wp's LINK line), so the reader cannot work it out from the + # (Prompts::WpWriter.create_wp's LINK line), so the reader cannot work it out from the # count and must be told. def single_notes(record) notes = +"" @@ -801,7 +815,7 @@ def substituted_type(record) # # This is where every `build` trigger lands (alias `fix`). There is # no separate plan-and-wait command any more: a fix with more than one defensible - # shape stops and offers numbered options (Prompts::OPTIONS_CONTRACT), and a + # shape stops and offers numbered options (Prompts::Planner::OPTIONS_CONTRACT), and a # fix with one shape is announced (#post_approach_note) and shipped in the # same call — so a simple ticket still costs exactly one plan call, just # with a stated approach instead of a silent one. NEEDS_INFO still guards @@ -849,7 +863,7 @@ def handle_ship(intent) # # `allow_options:` is the caller's judgment that no human has picked an # approach yet; the writer's judgment is whether the fix really has more than - # one shape (Prompts::OPTIONS_CONTRACT). `:failed` means the call produced + # one shape (Prompts::Planner::OPTIONS_CONTRACT). `:failed` means the call produced # neither a plan nor a usable options answer, and is handled like any other # failed run — logged, never commented. def produce_plan(st, feedback, allow_options: false, retry_bad_options: true) @@ -868,21 +882,19 @@ def produce_plan(st, feedback, allow_options: false, retry_bad_options: true) if feedback && !feedback.empty? && st.plan_file.exist? log_script "Writer: revising plan for #{wp_label(st.item_id)} from feedback" - prompt = Prompts.replan(repos_summary: @ctx.repos.summary, repos: menu, item: item_c, plan: plan_c, + prompt = Prompts::Planner.replan(repos_summary: @ctx.repos.summary, repos: menu, item: item_c, plan: plan_c, feedback: feedback, item_id: st.item_id, title: st.subject, resumed: session_resumable?(st), related: related, op_mcp: @ctx.op_mcp?) - @harness.capture(prompt, tools: read_tools, outfile: st.plan_file, - session_file: st.session_file) + llm(:planner, prompt, outfile: st.plan_file, session_file: st.session_file) record_chosen_repos(st) return :ok end log_script "Writer: generating plan for #{wp_label(st.item_id)} — #{st.subject}" - prompt = Prompts.plan(repos_summary: @ctx.repos.summary, repos: menu, item: item_c, + prompt = Prompts::Planner.plan(repos_summary: @ctx.repos.summary, repos: menu, item: item_c, item_id: st.item_id, title: st.subject, hint: feedback.to_s, related: related, allow_options: allow_options, op_mcp: @ctx.op_mcp?) - @harness.capture(prompt, tools: read_tools, outfile: st.plan_file, - session_file: st.session_file) + llm(:planner, prompt, outfile: st.plan_file, session_file: st.session_file) if st.plan_file.read.lstrip.start_with?("NEEDS_INFO") questions = st.plan_file.read.sub(/\A\s*NEEDS_INFO\s*\n?/, "").strip diff --git a/lib/opilot/appsignal_runner.rb b/lib/opilot/appsignal_runner.rb index ce3a1c0..255758b 100644 --- a/lib/opilot/appsignal_runner.rb +++ b/lib/opilot/appsignal_runner.rb @@ -197,11 +197,11 @@ def drafted_work_package(dir, number) # request rather than a duplicate work package. def write_work_package(number, incident_file, retry_bad: true, format_note: nil) log_script "Drafting a work package from AppSignal incident ##{number}…" - prompt = Prompts.appsignal_wp( + prompt = Prompts::Triager.appsignal_wp( incident: container_path(incident_file), number: number, app: @app, repos: repos_for_prompt(@ctx.repos.all), types: Helpers.types_for_prompt(project_types), format_note: format_note ) - reply = @harness.run(prompt, tools: read_tools, model: Harness::MODEL_HEAVY).to_s + reply = llm(:triager, prompt).to_s answer = Helpers.after_marker(reply, "ANSWER") if answer.lstrip.start_with?("NEEDS_INFO") diff --git a/lib/opilot/chat_runner.rb b/lib/opilot/chat_runner.rb index e0a2128..065577e 100644 --- a/lib/opilot/chat_runner.rb +++ b/lib/opilot/chat_runner.rb @@ -55,10 +55,10 @@ def run(initial_message = nil) prompt = if oriented pending else - Prompts.free_chat(state: @ctx.state_container, wp_root: wp_root, repos: repos, + Prompts::Advisor.free_chat(state: @ctx.state_container, wp_root: wp_root, repos: repos, message: pending, op_mcp: @ctx.op_mcp?, gh_mcp: @ctx.gh_mcp?) end - @harness.run(prompt, tools: read_tools, session_file: session_file) + llm(:advisor, prompt, session_file: session_file) # Set only after the run returns: a failed turn never reached the model, # so the next one still has to orient it. oriented = true diff --git a/lib/opilot/cli.rb b/lib/opilot/cli.rb index 0005b54..0317efb 100644 --- a/lib/opilot/cli.rb +++ b/lib/opilot/cli.rb @@ -89,6 +89,7 @@ def dev(args) when "build", "fix" then with_ids("dev build", rest) { |ids| FixRunner.new(@ctx).ship_ids(*ids) } when "commit" then with_ids("dev commit", rest) { |ids| FixRunner.new(@ctx).commit_ids(*ids) } when "plan" then with_ids("dev plan", rest) { |ids| FixRunner.new(@ctx).plan_ids(*ids) } + when "health" then with_ids("dev health", rest) { |ids| HealthRunner.new(@ctx).run_ids(*ids) } when "refresh" then refresh(rest) # Reads .opilot/ only — no config, no network, no log header. when "status" then @ui.status diff --git a/lib/opilot/clients/openproject.rb b/lib/opilot/clients/openproject.rb index 4b494d4..9ca4512 100644 --- a/lib/opilot/clients/openproject.rb +++ b/lib/opilot/clients/openproject.rb @@ -94,6 +94,13 @@ def work_package_emoji_reactions(wp_id) HTTP.get_json("#{@base}/api/v3/work_packages/#{wp_id}/activities_emoji_reactions", token: @token) end + # PRs the GitHub integration linked to a work package. Needs + # :show_github_content and the project's `github` module, so a 403 or 404 + # is a normal answer. `merged` is a boolean; `state` is open/closed/deployed. + def work_package_github_pull_requests(wp_id) + HTTP.get_json("#{@base}/api/v3/work_packages/#{wp_id}/github_pull_requests", token: @token) + end + def me HTTP.get_json("#{@base}/api/v3/users/me", token: @token) end diff --git a/lib/opilot/fix_runner.rb b/lib/opilot/fix_runner.rb index c5b6bad..3a73914 100644 --- a/lib/opilot/fix_runner.rb +++ b/lib/opilot/fix_runner.rb @@ -95,11 +95,6 @@ def process_item(item_data, mode:) # One guard for all three verbs — `plan` stops here too, which the old # per-verb check inside #commit/#ship could not do. return if report_already_shipped(st) - # One model for every session-bound phase of this WP (plan, chat, replan, - # implement) — switching mid-session would drop the context. The PR - # description is a separate, stateless call and picks its own model. - model = Harness::MODEL_HEAVY - replan_feedback = nil # Set once the operator picks an option (or types their own direction) at # the options prompt below. Present means the approach is settled, so the @@ -122,13 +117,14 @@ def process_item(item_data, mode:) begin # Read-only across all repos; the LLM re-declares the target repo(s) in # the revised plan. Branch checkout waits until #ship. - @harness.capture( - Prompts.replan(repos_summary: @ctx.repos.summary, repos: repos_for_prompt(@ctx.repos.all), + llm( + :planner, + Prompts::Planner.replan(repos_summary: @ctx.repos.summary, repos: repos_for_prompt(@ctx.repos.all), item: container_path(st.item_file), plan: container_path(st.plan_file), feedback: replan_feedback, item_id: id, title: subject, resumed: session_resumable?(st), related: related_ref(st), op_mcp: @ctx.op_mcp?), - tools: read_tools, model: model, outfile: st.plan_file, session_file: st.session_file + outfile: st.plan_file, session_file: st.session_file ) record_chosen_repos(st) rescue Harness::Error @@ -141,12 +137,13 @@ def process_item(item_data, mode:) log_script "Planning #{wp_label(id)} — #{subject}" begin # Pass session_file so a prior chat's context carries into the (re-)plan. - @harness.capture( - Prompts.plan(repos_summary: @ctx.repos.summary, repos: repos_for_prompt(@ctx.repos.all), + llm( + :planner, + Prompts::Planner.plan(repos_summary: @ctx.repos.summary, repos: repos_for_prompt(@ctx.repos.all), item: container_path(st.item_file), item_id: id, title: subject, hint: option_focus.to_s, related: related_ref(st), allow_options: option_focus.nil?, op_mcp: @ctx.op_mcp?), - tools: read_tools, model: model, outfile: st.plan_file, session_file: st.session_file + outfile: st.plan_file, session_file: st.session_file ) record_chosen_repos(st) if plan_present?(st) rescue Harness::Error @@ -194,7 +191,7 @@ def process_item(item_data, mode:) puts " ⚠ Plan generation failed — no plan came back." case prompt_plan_failed(id) when :retry then next # plan.md is gone, so the loop regenerates it - when :chat then run_chat(st, model); next + when :chat then run_chat(st); next when :skip then log_script "#{wp_label(id)} skipped."; break when :drop then log_script "#{wp_label(id)} dropped."; break end @@ -211,7 +208,7 @@ def process_item(item_data, mode:) puts "" case prompt_needs_info(id) when :chat - run_chat(st, model) + run_chat(st) next # retry planning — chat session carries context forward when :skip log_script "#{wp_label(id)} skipped (needs info)." @@ -238,11 +235,11 @@ def process_item(item_data, mode:) # `build`/`ship` picks it up. log_script "#{wp_label(id)} — plan approved; build or ship it later with " \ "`dev commit #{id}` / `dev build #{id}`." - when :commit then commit(st, model) - when :ship then ship(st, model) + when :commit then commit(st) + when :ship then ship(st) end break - when :chat then run_chat(st, model) + when :chat then run_chat(st) when :replan then replan_feedback = prompt_replan_feedback when :skip then log_script "#{wp_label(id)} skipped."; break when :drop then safe_rm(st.plan_file); log_script "#{wp_label(id)} dropped."; break @@ -271,7 +268,7 @@ def prompt_needs_info(id) end # Offer the implementation options at the console, the same list a work - # package gets (Prompts::OPTIONS_CONTRACT). Returns the plan focus for the + # package gets (Prompts::Planner::OPTIONS_CONTRACT). Returns the plan focus for the # chosen option, free text as its own direction (so the operator is never # forced to pick one of the three), :skip, or :drop. def prompt_option_choice(id, options) @@ -317,7 +314,7 @@ def prompt_replan_feedback msg.empty? ? "Revise the plan to incorporate the changes requested in the preceding conversation." : msg end - def run_chat(st, model = Harness::MODEL_HEAVY) + def run_chat(st) # The session already holds the plan (just generated/revised), so pass its # path as a fallback rather than re-embedding the full text on every turn. plan_ref = st.plan_file.exist? ? container_path(st.plan_file) : "(no plan yet)" @@ -335,12 +332,12 @@ def run_chat(st, model = Harness::MODEL_HEAVY) prompt = if oriented msg else - Prompts.plan_chat( + Prompts::Advisor.plan_chat( item_id: st.item_id, subject: st.subject, item: container_path(st.item_file), plan: plan_ref, message: msg ) end - @harness.run(prompt, tools: read_tools, model: model, session_file: st.session_file) + llm(:advisor, prompt, session_file: st.session_file) oriented = true # Ring after the reply, not before the first message: the user just # chose [c]hat and is present; it's the LLM's answers they wander off on. @@ -351,8 +348,8 @@ def run_chat(st, model = Harness::MODEL_HEAVY) # Helpers#implement_plan plus the console's own report of a no-op plan (the # agent answers that on the work package instead). - def implement(st, model = Harness::MODEL_HEAVY) - changed = implement_plan(st, model: model) + def implement(st) + changed = implement_plan(st) if changed.empty? log_script "#{wp_label(st.item_id)} — no changes produced." puts " ⚠ No changes produced — plan may be a no-op or already applied." @@ -363,15 +360,15 @@ def implement(st, model = Harness::MODEL_HEAVY) # `commit`: implement and commit, then stop — nothing is pushed and no PR is # opened. The committed branch sits in the local clone for review; a later # `dev build ` finds it via branch_has_commits? and goes straight to publish. - def commit(st, model = Harness::MODEL_HEAVY) - implement(st, model).each do |repo| + def commit(st) + implement(st).each do |repo| record_progress(st.item_id, st.branch, "built:#{repo.name}") puts " ✓ Committed #{st.branch} (#{repo.name}) — review it in the clone, then ship it with `./opilot dev build #{st.item_id}`" end end - def ship(st, model = Harness::MODEL_HEAVY) - implement(st, model).each do |repo| + def ship(st) + implement(st).each do |repo| generate_pr_description(st, repo) url = @publish.open_pr(st.item_id, st.subject, st.branch, repo) if url diff --git a/lib/opilot/gh_agent.rb b/lib/opilot/gh_agent.rb index 564a9af..b28cef9 100644 --- a/lib/opilot/gh_agent.rb +++ b/lib/opilot/gh_agent.rb @@ -143,14 +143,14 @@ def handle_review(intent) @github.fetch_branch(head_repo(intent), branch: intent.branch, worktree_path: repo.worktree_host) checkout_pr_branch(repo, intent.branch) - prompt = Prompts.pr_review( + prompt = Prompts::PrAdvisor.pr_review( repo: intent.repo, pr_number: intent.pr_number, title: intent.subject, worktree: repo.worktree_container, base: repo.base, pr_thread: container_path(pr_file), comment: intent.text.to_s, author: intent.user_login.to_s, comment_id: intent.comment_id, in_reply_to: intent.in_reply_to, ci: review_ci_ref(ci_file, intent.head_sha) ) - reply = @harness.run(prompt, tools: Harness::TOOLS_READ, session_file: session_file) + reply = llm(:pr_advisor, prompt, session_file: session_file) post_suggestions(intent, reply) post_reply(intent, reply) end @@ -244,7 +244,7 @@ def run_on_pr_head(intent) worktree_path: paths.repo.worktree_host) checkout_pr_branch(paths.repo, intent.branch) - reply = @harness.run(yield(paths), tools: impl_tools, session_file: paths.session_file) + reply = llm(:pr_author, yield(paths), session_file: paths.session_file) post_reply(intent, reply) push_followup(intent, paths.repo) if commit_followup(intent, paths.repo) @@ -252,7 +252,7 @@ def run_on_pr_head(intent) def handle_own(intent) run_on_pr_head(intent) do |p| - Prompts.gh_reply( + Prompts::PrAuthor.gh_reply( worktree: p.repo.worktree_container, repo: intent.repo, pr_number: intent.pr_number, title: intent.subject, item: p.item_ref, plan: p.plan_ref, pr_thread: container_path(p.pr_file), comment: intent.text.to_s, @@ -267,7 +267,7 @@ def handle_own(intent) # it in the worktree, then commit and push to update the draft PR. def handle_ci(intent) run_on_pr_head(intent) do |p| - Prompts.fix_ci( + Prompts::PrAuthor.fix_ci( op_mcp: @ctx.op_mcp?, worktree: p.repo.worktree_container, repo: intent.repo, pr_number: intent.pr_number, title: intent.subject, item: p.item_ref, plan: p.plan_ref, diff --git a/lib/opilot/harness.rb b/lib/opilot/harness.rb index 6febb8c..0408f3f 100644 --- a/lib/opilot/harness.rb +++ b/lib/opilot/harness.rb @@ -31,10 +31,8 @@ class Harness TOOLS_READ = "read,grep,find,ls,bash" TOOLS_IMPL = "read,grep,find,ls,bash,write,edit" - # The op_query variants (see MCP.md), granted only at the call sites named - # in Context#op_mcp?'s call table via Helpers#read_tools/#impl_tools — most - # TOOLS_READ/TOOLS_IMPL call sites keep the plain constant even when the - # flag is on. Must stay in sync with ALLOWED_TOOL_GRANTS in server.js. + # The op_query variants (see MCP.md), granted only to the roles marked + # `mcp` (roles.rb). Must stay in sync with ALLOWED_TOOL_GRANTS in server.js. TOOLS_READ_OP = "#{TOOLS_READ},op_query" TOOLS_IMPL_OP = "#{TOOLS_IMPL},op_query" @@ -85,6 +83,8 @@ def self.env_minutes(name, fallback) env_minutes("OPILOT_PI_IDLE_TIMEOUT_MIN", 5) + 2) * 60 ).round + require_relative "roles" + def initialize(ctx) @ctx = ctx @uri = URI(@ctx.harness_url) @@ -116,7 +116,7 @@ def ensure_available! # Runs the LLM with the given prompt. Streams tool-use lines to tty, returns text output. # Pass session_file: (a Pathname) to enable per-WP session continuity — the file is # read for the session ID before the call and updated with the new ID after. - def run(prompt, tools: nil, model: MODEL_HEAVY, session_file: nil) + def run(prompt, role:, tools: nil, model: MODEL_HEAVY, session_file: nil) session_id = session_file&.exist? ? session_file.read.strip : nil header = Rainbow("#{log_prefix} PI PROMPT (model: #{model}, session: #{session_id || "fresh"})").bold @@ -129,7 +129,7 @@ def run(prompt, tools: nil, model: MODEL_HEAVY, session_file: nil) puts resp_header log_append(resp_header) - text, new_session_id, error = http_stream(prompt, tools: tools, model: model, session_id: session_id) + text, new_session_id, error = http_stream(prompt, role: role, tools: tools, model: model, session_id: session_id) # A resumed session the CLI no longer has (e.g. the harness container was # recreated/killed before the transcript was durably written) makes @@ -140,7 +140,7 @@ def run(prompt, tools: nil, model: MODEL_HEAVY, session_file: nil) if error && session_id && lost_session?(error) log_append("session #{session_id} is gone — retrying fresh") $stdout.puts Rainbow(" ⚠ session #{session_id} not found — starting fresh").yellow - text, new_session_id, error = http_stream(prompt, tools: tools, model: model, session_id: nil) + text, new_session_id, error = http_stream(prompt, role: role, tools: tools, model: model, session_id: nil) end # Save the session even on error, so a retry can resume with context. @@ -158,15 +158,15 @@ def run(prompt, tools: nil, model: MODEL_HEAVY, session_file: nil) end # Like run, but also writes ANSI-stripped output to outfile. - def capture(prompt, tools: nil, model: MODEL_HEAVY, outfile:, session_file: nil) - text = run(prompt, tools: tools, model: model, session_file: session_file) + def capture(prompt, role:, outfile:, tools: nil, model: MODEL_HEAVY, session_file: nil) + text = run(prompt, role: role, tools: tools, model: model, session_file: session_file) Pathname(outfile).write(strip_ansi(text)) text end private - def http_stream(prompt, tools:, model:, session_id: nil) + def http_stream(prompt, role:, tools:, model:, session_id: nil) attempts = 0 begin attempts += 1 @@ -181,6 +181,7 @@ def http_stream(prompt, tools:, model:, session_id: nil) exit_info = nil req = Net::HTTP::Post.new(@uri) + req["X-Harness-Role"] = role.to_s req["X-Harness-Tools"] = tools if tools req["X-Harness-Model"] = model if model req["X-Harness-Session"] = session_id if session_id diff --git a/lib/opilot/health_check.rb b/lib/opilot/health_check.rb new file mode 100644 index 0000000..151218f --- /dev/null +++ b/lib/opilot/health_check.rb @@ -0,0 +1,411 @@ +require "json" +require "time" + +module OPilot + # The work package health check, shared by `@opilot health` (Agent) and + # `./opilot dev health` (HealthRunner). Two layers: + # + # - Facts: rules on exact, language-independent data (status flags from + # /statuses, relation labels, timestamps, PR flags, commit ids). Written to + # health.json, and the prompt tells the model not to dispute them — so a + # heuristic never belongs here. + # - Judgement: one read-only LLM call, answered in Prompts::Auditor::HEALTH_CONTRACT. + # + # The reply is composed here, not by the model, so it states what was checked. + class HealthCheck + include Helpers + + STALE_DAYS = 60 + MAX_COMMITS = 10 + MAX_DESCENDANTS = 200 + MAX_TREE_FINDINGS = 5 # per rule; the rest are counted in the text + SEVERITY_ORDER = Prompts::Auditor::HEALTH_SEVERITIES + + # Labels from this work package's own side (Pull#relation_pairs): the other + # one must finish first. + PREREQUISITES = %w[blocked follows requires].freeze + + def initialize(ctx, pull:, harness:, api: nil) + @ctx = ctx + @pull = pull + @harness = harness + @api = api || Clients::OpenProject.new(ctx.op_url, ctx.token) + end + + # The report text, or nil when the work package cannot be fetched. + # `internal:` is the visibility the report is posted with. + def run(item_id, focus: "", internal: true) + item = @pull.fetch_single_item(item_id) + return nil unless item + + st = state_for(item["id"], item["subject"], item["type"]) + related_path = related_ref(st) + related = related_path ? (Helpers.safe_json_read(st.related_file) || []) : [] + + tree = descendants(item["id"]) + tree_file = st.item_dir / "descendants.json" + tree_file.write(JSON.pretty_generate(tree["nodes"])) if tree["nodes"]&.any? + tree_ref = if tree["nodes"].nil? then nil + elsif tree["nodes"].empty? then :none + else container_path(tree_file) + end + + facts = facts_for(item, related, status_map, linked_prs(item["id"]), commits(item["id"]), tree: tree) + facts["inputs"]["opilot_user_href"] = opilot_user_href + facts["inputs"]["opilot_prs"] = st.repos.filter_map { |r| st.pr_url_file(r).read.strip if st.pr_url_file(r).exist? } + facts_file = st.item_dir / "health.json" + facts_file.write(JSON.pretty_generate(facts)) + + prompt = Prompts::Auditor.health(item_id: st.item_id, subject: st.subject, + item: container_path(st.item_file), facts: container_path(facts_file), + related: related_path, descendants: tree_ref, focus: focus, internal: internal, + op_mcp: @ctx.op_mcp?) + answer = ask(prompt) + return failed_note unless answer + + report(facts, answer, item, internal: internal) + end + + # ── facts ──────────────────────────────────────────────────────────────── + + # name => { "closed", "default" }, or nil when /statuses cannot be read. + # Status names are unique on an instance, so the lookup by name is exact. + def status_map + _code, body = @api.statuses + (body&.dig("_embedded", "elements") || []).to_h do |s| + [s["name"].to_s, { "closed" => s["isClosed"] == true, "default" => s["isDefault"] == true }] + end + rescue StandardError => e + log_script "Health: could not read statuses (#{e.message})" + nil + end + + # Every descendant at any depth, from one paginated `ancestor` query: + # { "nodes" => [...] | nil (not read), "truncated", "code" }. Each node is + # { id, parent, depth, subject, type, status, updated_at }, ids as displayed. + def descendants(item_id) + code, wp = @api.work_package(item_id) + return { "nodes" => nil, "truncated" => false, "code" => code } unless code == 200 && wp + root = wp["id"].to_s + filter = Clients::OpenProject.filter("ancestor", "=", root) + raw = [] + total = 0 + (1..).each do |page| + code, resp = @api.work_packages(filters_json: filter, page: page, page_size: 100, sort_by: '[["id","asc"]]') + return { "nodes" => nil, "truncated" => false, "code" => code } unless code == 200 && resp + total = resp["total"].to_i + elements = resp.dig("_embedded", "elements") || [] + raw.concat(elements) + break if elements.empty? || raw.length >= total || raw.length >= MAX_DESCENDANTS + end + { "nodes" => tree_nodes(raw.first(MAX_DESCENDANTS), root, Helpers.display_id(wp)), + "truncated" => total > MAX_DESCENDANTS, "code" => 200 } + end + + private def tree_nodes(raw, root_numeric, root_display) + shown = raw.to_h { |w| [w["id"].to_s, Helpers.display_id(w)] }.merge(root_numeric => root_display) + parent_of = raw.to_h { |w| [w["id"].to_s, w.dig("_links", "parent", "href").to_s.split("/").last.to_s] } + depth = lambda do |id, seen = 0| + up = parent_of[id] + up.nil? || up == root_numeric || seen > MAX_DESCENDANTS ? 1 : 1 + depth.(up, seen + 1) + end + raw.map do |w| + id = w["id"].to_s + { "id" => shown[id], "parent" => shown[parent_of[id]] || parent_of[id], "depth" => depth.(id), + "subject" => w["subject"], "type" => link_title(w, "type"), + "status" => link_title(w, "status"), "updated_at" => w["updatedAt"] } + end + end + + # A list element embeds nothing; the name is the link's title. + private def link_title(wp, key) + wp.dig("_links", key, "title") || wp.dig("_embedded", key, "name") + end + + # So the model can tell opilot's own comments from the thread. + private def opilot_user_href + id = @pull.own_user_id if @pull.respond_to?(:own_user_id) + id.to_s.empty? ? nil : "/api/v3/users/#{id}" + rescue StandardError + nil + end + + # [prs, code]. The GitHub integration answers 403/404 when it is off. + def linked_prs(item_id) + code, body = @api.work_package_github_pull_requests(item_id) + return [nil, code] unless code == 200 + prs = (body&.dig("_embedded", "elements") || []).map do |pr| + { "url" => pr["htmlUrl"], "repository" => pr["repository"], "number" => pr["number"], + "title" => pr["title"], "state" => pr["state"], "merged" => pr["merged"] == true, + "merged_at" => pr["mergedAt"], "draft" => pr["draft"] == true } + end + [prs, code] + end + + # PR numbers share the `#N` form with work package ids ("Merge pull request + # #25183", a squash's "(#25183)"), so they are removed before matching. + PR_NUMBER = /Merge pull request #\d+|\(#\d+\)/ + + # How a commit names a work package: the id in a branch name + # (`bug/op-123-slug`, `bug/59942-slug`), a work package URL, `[#N]`, `OP#N`. + # Never a prefix of a longer id. + def self.commit_pattern(item_id) + id = Regexp.escape(item_id.to_s) + return /(? [{ "sha", "subject" }] } for commits on each registry base + # that name the work package, or { repo_name => nil } when a clone cannot + # be read. + def commits(item_id) + exact = HealthCheck.commit_pattern(item_id) + @ctx.repos.all.to_h do |repo| + sync_base!(repo) + found = worktree(repo).log(500).object("origin/#{repo.base}").grep(HealthCheck.commit_prefilter(item_id)).execute + hits = found.select { |c| c.message.to_s.gsub(PR_NUMBER, "").match?(exact) }.first(MAX_COMMITS) + [repo.name, hits.map { |c| { "sha" => c.sha[0, 12], "subject" => c.message.to_s.lines.first.to_s.strip } }] + rescue StandardError => e + log_script "Health: could not read commits in #{repo.name} (#{e.message})" + [repo.name, nil] + end + end + + # Pure: every input already fetched, so the rules test without HTTP or git. + # `tree` is #descendants' answer; nil (not asked) keeps the direct-child + # rules on related.json, which is what a failed subtree read falls back to. + def facts_for(item, related, statuses, prs_and_code, commits, tree: nil, now: Time.now) + prs, pr_code = prs_and_code + findings = [] + not_checked = [] + nodes = tree&.dig("nodes") + if tree && nodes.nil? + not_checked << "The descendants could not be read (HTTP #{tree["code"]}); only direct children were checked." + elsif tree&.dig("truncated") + not_checked << "The subtree has more than #{MAX_DESCENDANTS} work packages; only the first #{MAX_DESCENDANTS} were checked." + end + # Children are part of the tree, so the tree rules replace the child rules. + related = related.reject { |r| r["relation"] == "child" } if nodes + + own = statuses && statuses[item["status"].to_s] + if own.nil? + not_checked << (statuses ? "The status \"#{item["status"]}\" is not in the status list, so no status rule ran." : + "The status list could not be read, so no status rule ran.") + else + findings.concat(relation_findings(item, related, statuses, own)) + findings.concat(tree_findings(nodes, statuses, own, now)) if nodes&.any? + findings.concat(pr_findings(item, prs, own)) if prs + findings.concat(commit_findings(item, commits, own)) + stale = item["history"].nil? ? nil : stale_finding(item, own, now) + findings << stale if stale + end + findings.concat(design_findings(item)) + if item["history"].nil? + not_checked << "The activities could not be read: comments and field changes are missing, " \ + "and the staleness and design rules did not run." + end + + Array(item["pictures_skipped"]).each do |p| + not_checked << "Attachment \"#{p["name"]}\" (#{p["where"] || "attached"}): #{p["reason"]}." + end + not_checked << "Linked pull requests could not be read (HTTP #{pr_code})." unless prs + commits.each { |repo, list| not_checked << "Commits in #{repo} could not be read." if list.nil? } + + { "findings" => findings, "not_checked" => not_checked, + "inputs" => { "status" => item["status"], "status_closed" => own&.dig("closed"), + "status_default" => own&.dig("default"), "pull_requests" => prs, + "commits" => commits, "descendant_count" => nodes&.length } } + end + + # Rules across the whole subtree. A node whose status is not in the list + # is neither open nor closed, so no rule uses it. + private def tree_findings(nodes, statuses, own, now) + closed = ->(n) { statuses.dig(n["status"].to_s, "closed") } + open_nodes = nodes.select { |n| closed.(n) == false } + ids = ->(list) { list.first(MAX_TREE_FINDINGS).map { |n| wp_label(n["id"]) }.join(", ") + (list.length > MAX_TREE_FINDINGS ? ", …" : "") } + out = [] + + if own["closed"] && open_nodes.any? + out << fact("high", "relations", "This work package is closed, but #{open_nodes.length} descendant(s) are still open.", ids.(open_nodes)) + elsif !own["closed"] && nodes.all? { |n| closed.(n) == true } + out << fact("low", "status", "All #{nodes.length} descendants are closed, but this work package is still open.", ids.(nodes)) + end + + # A closed node inside the tree with an open node under it. + by_id = nodes.to_h { |n| [n["id"], n] } + ancestors = lambda do |n| + chain = [] + while (up = by_id[n["parent"]]) && chain.length <= nodes.length + chain << up + n = up + end + chain + end + closed_over_open = open_nodes.flat_map { |n| ancestors.(n).select { |a| closed.(a) == true } }.uniq + closed_over_open.first(MAX_TREE_FINDINGS).each do |a| + out << fact("medium", "relations", "Descendant #{wp_label(a["id"])} is closed, but a work package under it is still open.", wp_label(a["id"])) + end + + stale = open_nodes.select { |n| (t = parse_time(n["updated_at"])) && (now - t) > STALE_DAYS * 86_400 } + if stale.any? + out << fact("low", "status", "#{stale.length} open descendant(s) did not change for #{STALE_DAYS} days.", ids.(stale)) + end + out + end + + private def relation_findings(item, related, statuses, own) + id = ->(r) { wp_label(r["id"]) } + closed = ->(r) { statuses.dig(r["status"].to_s, "closed") } + children = related.select { |r| r["relation"] == "child" } + out = [] + + if own["closed"] + related.each do |r| + next unless closed.(r) == false + if PREREQUISITES.include?(r["relation"]) + out << fact("medium", "relations", "This work package is closed, but #{id.(r)} (#{r["relation"]}) is still open.", id.(r)) + elsif r["relation"] == "child" + out << fact("high", "relations", "This work package is closed, but its child #{id.(r)} is still open.", id.(r)) + end + end + else + if children.any? && children.all? { |r| closed.(r) == true } + out << fact("low", "status", "All children are closed, but this work package is still open.", + children.map(&id).join(", ")) + end + related.select { |r| r["relation"] == "duplicates" && closed.(r) == true }.each do |r| + out << fact("medium", "relations", "This work package duplicates #{id.(r)}, which is closed, but it is still open.", id.(r)) + end + end + out + end + + # Only the DEFAULT status (the one a new work package starts in) is a clear + # contradiction: "Developed" or "In testing" is open and has merged PRs. + private def pr_findings(_item, prs, own) + out = [] + prs.each do |pr| + if pr["merged"] && own["default"] + out << fact("high", "prs", "A linked pull request is merged, but the status is still the initial one.", pr["url"]) + elsif pr["state"] == "open" && !pr["draft"] && own["closed"] + out << fact("medium", "prs", "This work package is closed, but a linked pull request is still open.", pr["url"]) + end + end + out + end + + private def commit_findings(_item, commits, own) + return [] unless own["default"] + hits = commits.flat_map { |repo, list| Array(list).map { |c| "#{repo}@#{c["sha"]}" } } + return [] if hits.empty? + [fact("low", "status", "Commits name this work package, but the status is still the initial one.", hits.first(3).join(", "))] + end + + private def stale_finding(item, own, now) + return nil if own["closed"] + times = [item["created_at"], *Array(item["comments"]).map { |c| c["created_at"] }, + *Array(item["history"]).map { |h| h["created_at"] }] + last = times.filter_map { |t| parse_time(t) }.max + return nil unless last && (now - last) > STALE_DAYS * 86_400 + fact("low", "status", "This work package is open, but nothing changed for #{((now - last) / 86_400).floor} days.", + "last activity #{last.utc.iso8601}") + end + + # A picture in the description arrives with that same edit, so only one in a + # comment or attached on its own can be newer than the text. + private def design_findings(item) + edited = parse_time(item["description_changed_at"]) + return [] unless edited + Array(item["pictures"]).filter_map do |p| + next if p["where"] == "description" + added = parse_time(p["created_at"]) + next unless added && added > edited + fact("medium", "designs", "The picture \"#{p["name"]}\" was added after the last description edit.", + "#{p["name"]}, #{added.utc.iso8601} (#{p["where"]})") + end + end + + private def fact(severity, area, text, evidence) + { "severity" => severity, "area" => area, "text" => text, "evidence" => evidence.to_s } + end + + private def parse_time(value) + value.to_s.empty? ? nil : Time.parse(value.to_s) + rescue ArgumentError + nil + end + + # ── judgement ──────────────────────────────────────────────────────────── + + RETRY_NOTE = "\n\nYour last answer had no complete BEGIN HEALTH … END HEALTH block. " \ + "Answer again, and end with that block exactly as described." + + private def ask(prompt) + answer = Helpers.parse_health(llm(:auditor, prompt)) + answer || Helpers.parse_health(llm(:auditor, prompt + RETRY_NOTE)) + end + + private def failed_note + "The health check did not finish: the answer had no report that I could read. " \ + "Ask again with `@opilot health`." + end + + # ── report ─────────────────────────────────────────────────────────────── + + # The whole reply. A public reply drops a finding whose evidence cites an + # internal comment: the evidence is printed verbatim. + def report(facts, answer, item, internal: true) + findings = facts["findings"] + answer["findings"] + hidden = 0 + unless internal + stamps = Array(item["comments"]).select { |c| c["internal"] }.filter_map { |c| c["created_at"].to_s[0, 16] } + stamps.reject!(&:empty?) + kept = findings.reject { |f| stamps.any? { |s| f["evidence"].include?(s) } } + hidden = findings.length - kept.length + findings = kept + end + findings = findings.sort_by.with_index { |f, i| [SEVERITY_ORDER.index(f["severity"]), i] } + + gaps = answer["gaps"].reject { |g| already_not_checked?(g, facts["not_checked"]) } + not_checked = facts["not_checked"] + gaps.map { |g| g["why"].empty? ? "#{g["what"]}." : "#{g["what"]}: #{g["why"]}" } + not_checked << "#{hidden} finding(s) cite an internal comment and are not shown in this public reply." if hidden.positive? + + lines = [summary_line(findings)] + SEVERITY_ORDER.each do |sev| + group = findings.select { |f| f["severity"] == sev } + next if group.empty? + lines << "" << "**#{sev.capitalize}**" + group.each { |f| lines << "- #{f["text"]} (#{f["evidence"]})" } + end + unless not_checked.empty? + lines << "" << "**Not checked**" + not_checked.each { |n| lines << "- #{n}" } + end + lines.join("\n") + end + + # The prompt forbids a GAP that repeats not_checked, and a real run wrote one + # anyway. A gap is a repeat when a fact line holds every word of its subject. + private def already_not_checked?(gap, lines) + words = gap["what"].to_s.downcase.scan(/[[:alnum:]]{4,}/) + return false if words.empty? + lines.any? { |line| words.all? { |w| line.downcase.include?(w) } } + end + + private def summary_line(findings) + return "**Health check: no findings.**" if findings.empty? + counts = SEVERITY_ORDER.filter_map do |sev| + n = findings.count { |f| f["severity"] == sev } + "#{n} #{sev}" if n.positive? + end + noun = findings.length == 1 ? "finding" : "findings" + "**Health check: #{findings.length} #{noun}** (#{counts.join(", ")})" + end + end +end diff --git a/lib/opilot/health_runner.rb b/lib/opilot/health_runner.rb new file mode 100644 index 0000000..43d05ab --- /dev/null +++ b/lib/opilot/health_runner.rb @@ -0,0 +1,35 @@ +module OPilot + # The terminal `dev health`: run HealthCheck on each id and print the report. + # It posts nothing, so a prompt change can be tuned on a real work package. + class HealthRunner + include Helpers + + def initialize(ctx, pull: Pull.new(ctx), harness: Harness.new(ctx), api: nil) + @ctx = ctx + @pull = pull + @harness = harness + @check = HealthCheck.new(ctx, pull: pull, harness: harness, api: api) + end + + # One failure does not stop the rest; with a single id it is fatal. + def run_ids(*wp_ids) + ensure_harness! + report_mcp_status + wp_ids.each do |wp_id| + log_script "Checking work package #{wp_label(wp_id)}…" + report = @check.run(wp_id) + unless report + msg = "could not fetch work package #{wp_label(wp_id)} — check the id and OPENPROJECT_TOKEN" + raise OPilot::FatalError, msg if wp_ids.length == 1 + log_script "#{wp_label(wp_id)} — #{msg}" + next + end + puts "" + puts render_markdown(report) + rescue Harness::Error => e + raise OPilot::FatalError, "The LLM run failed: #{e.message}" if wp_ids.length == 1 + log_script "#{wp_label(wp_id)} — The LLM run failed: #{e.message}" + end + end + end +end diff --git a/lib/opilot/helpers.rb b/lib/opilot/helpers.rb index acbd93c..94dc97c 100644 --- a/lib/opilot/helpers.rb +++ b/lib/opilot/helpers.rb @@ -108,7 +108,7 @@ def self.mention(name, user_href) # ── implementation options ────────────────────────────────────────────── # # A plan call may answer with implementation options instead of a plan (see - # Prompts::OPTIONS_CONTRACT). Both readers of that answer live here — the + # Prompts::Planner::OPTIONS_CONTRACT). Both readers of that answer live here — the # agent, which offers the options in a work-package comment, and the terminal # runner, which offers them at the console — so the parsing and the wording of # a chosen option are written once. @@ -135,7 +135,7 @@ def self.parse_options(body) .uniq { |o| o["n"] }.sort_by { |o| o["n"] } end - # Whether `text` answers with OPTIONS (Prompts::OPTIONS_CONTRACT) — tolerant + # Whether `text` answers with OPTIONS (Prompts::Planner::OPTIONS_CONTRACT) — tolerant # of a preamble sentence before the sentinel line, the same accommodation # #record_chosen_repos' REPOS: match already makes and the NEEDS_INFO check # below makes too: a local model in particular often reasons in prose before @@ -143,11 +143,11 @@ def self.parse_options(body) # Requires the sentinel ALONE on its own line, so an ordinary sentence that # happens to use the word "options" is never mistaken for the block. def self.options_sentinel?(text) - text.to_s.lines.any? { |l| l.strip == Prompts::OPTIONS_SENTINEL } + text.to_s.lines.any? { |l| l.strip == Prompts::Planner::OPTIONS_SENTINEL } end # Split a writer's answer into its OPTIONS line(s) and whatever follows - # (Prompts::OPTIONS_CONTRACT: name the approach, then — when there's only + # (Prompts::Planner::OPTIONS_CONTRACT: name the approach, then — when there's only # one — continue straight into the plan in the same response). The # sentinel is found anywhere, per #options_sentinel? above, and everything # before and including it is dropped along with it — a preamble sentence @@ -157,7 +157,7 @@ def self.options_sentinel?(text) # which a tolerant scan like parse_options' would misread as more options. def self.parse_leading_options(body) lines = body.to_s.lines - start = lines.index { |l| l.strip == Prompts::OPTIONS_SENTINEL } + start = lines.index { |l| l.strip == Prompts::Planner::OPTIONS_SENTINEL } return [[], body.to_s.lstrip] unless start lines = lines[(start + 1)..] || [] options = [] @@ -183,7 +183,7 @@ def self.parse_leading_options(body) WP_LINKS = %w[child related].freeze DEFAULT_WP_LINK = "related".freeze - # Read the work packages a `create wp` answer asks for (Prompts.create_wp). + # Read the work packages a `create wp` answer asks for (Prompts::WpWriter.create_wp). # Each one is a block — BEGIN WORK PACKAGE, a SUBJECT: line, an optional # TYPE: and LINK: line, the description, END WORK PACKAGE — and a request # that names several pieces of work answers with several blocks, in order. @@ -277,12 +277,63 @@ def self.work_package_fields(lines) end private_class_method :work_package_fields + HEALTH_BEGIN = /\A[ \t]*BEGIN HEALTH[ \t]*\z/ + HEALTH_END = /\A[ \t]*END HEALTH[ \t]*\z/ + HEALTH_FINDING_LINE = /\A[ \t]*FINDING:[ \t]*(.+)\z/i + HEALTH_GAP_LINE = /\A[ \t]*GAP:[ \t]*(.+)\z/i + HEALTH_CLEAN_LINE = /\A[ \t]*NO FINDINGS[ \t.]*\z/i + + # Read a health answer (Prompts::Auditor::HEALTH_CONTRACT) into + # { "findings" => [...], "gaps" => [...] }, or nil when there is no complete + # block — the answer was cut off, or ignored the format. The LAST complete + # block wins, so a format the writer rehearsed first does not count. A + # malformed line drops only itself; so does a finding with no evidence. + def self.parse_health(body) + block = nil + open = nil + fence = nil + body.to_s.lines.each do |raw| + line = raw.chomp + fence = fence_state(fence, line) + next unless fence.nil? + + if line.match?(HEALTH_BEGIN) then open = [] + elsif line.match?(HEALTH_END) + block = open if open + open = nil + else open&.<<(line) + end + end + return nil unless block + + findings = block.filter_map { |l| health_finding(l[HEALTH_FINDING_LINE, 1]) } + gaps = block.filter_map do |l| + what, why = l[HEALTH_GAP_LINE, 1]&.split("|", 2)&.map(&:strip) + { "what" => what, "why" => why.to_s } unless what.to_s.empty? + end + readable = findings.any? || gaps.any? || + block.any? { |l| l.match?(HEALTH_CLEAN_LINE) || l.match?(HEALTH_FINDING_LINE) } + return nil unless readable + { "findings" => findings.first(Prompts::Auditor::HEALTH_MAX_FINDINGS), "gaps" => gaps } + end + + def self.health_finding(text) + return nil unless text + severity, area, sentence, evidence = text.split("|", 4).map { |f| f.to_s.strip } + severity = severity.downcase + area = area.to_s.downcase + return nil unless Prompts::Auditor::HEALTH_SEVERITIES.include?(severity) && Prompts::Auditor::HEALTH_AREAS.include?(area) + return nil if sentence.to_s.empty? || evidence.to_s.empty? + { "severity" => severity, "area" => area, "text" => sentence, "evidence" => evidence } + end + private_class_method :health_finding + ARTIFACT_BEGIN = /\A[ \t]*BEGIN ARTIFACT[ \t]*\z/ ARTIFACT_END = /\A[ \t]*END ARTIFACT[ \t]*\z/ ARTIFACT_FILENAME_LINE = /\AFILENAME:[ \t]*(.+)\z/i ARTIFACT_TITLE_LINE = /\ATITLE:[ \t]*(.+)\z/i - # Read the artifacts a chat answer carries (Prompts.artifact_block), and + # Read the artifacts a chat answer carries (Prompts::Advisor.artifact_block), and # return [artifacts, remainder] — the remainder being the answer with every # block removed, which is what gets posted as the comment. # @@ -874,20 +925,16 @@ def ensure_harness! @harness.ensure_available! if @harness.respond_to?(:ensure_available!) end - # The tool grant for a read-only LLM phase — includes op_query when - # OPILOT_OP_MCP is on (see MCP.md). Use ONLY at the specific call sites - # named in that plan's Step 3 table; every other TOOLS_READ call site - # (upstream PR review, the `create wp` draft, light one-shot passes) keeps - # the plain constant even when the flag is on. - def read_tools - Harness.tools_for(Harness::TOOLS_READ, op_mcp: @ctx.op_mcp?, gh_mcp: @ctx.gh_mcp?) - end - - # As #read_tools, for the one write-enabled call site the plan grants the - # tool to: gh-agent's own-PR reply and CI fix. The fix implement run keeps - # no MCP tool at all — see MCP.md's Step 3 table for why. - def impl_tools - Harness.tools_for(Harness::TOOLS_IMPL, op_mcp: @ctx.op_mcp?, gh_mcp: @ctx.gh_mcp?) + # The one way to call the model: the role decides tools and model. + # A stateless role refuses a session, so its independence is structural. + def llm(role, prompt, session_file: nil, outfile: nil) + r = Harness.role(role) + raise ArgumentError, "role #{r.name} is stateless" if r.stateless && session_file + if prompt.is_a?(Prompts::Prompt) && prompt.role != r.name + raise ArgumentError, "a #{prompt.role} prompt sent as #{r.name}" + end + opts = { role: r.name, tools: r.tools(@ctx), model: r.model, session_file: session_file } + outfile ? @harness.capture(prompt, outfile: outfile, **opts) : @harness.run(prompt, **opts) end # One-time, best-effort report of what the instance's MCP server actually @@ -1269,16 +1316,15 @@ def commit_and_log(wt, message, where = nil) # # Reporting the result is deliberately left to the caller — a work-package # comment and a console line are not the same message. - def implement_plan(st, model: Harness::MODEL_HEAVY) + def implement_plan(st) st.repos.each { |r| checkout_branch(st, r) } unless st.repos.all? { |r| branch_has_commits?(st, r) } log_script "Implementing #{wp_label(st.item_id)} in #{st.repos.map(&:name).join(", ")}" - @harness.run( - Prompts.implement(repos: repos_for_prompt(st.repos), plan: container_path(st.plan_file), - resumed: session_resumable?(st)), - tools: Harness::TOOLS_IMPL, model: model, session_file: st.session_file - ) + llm(:implementer, + Prompts::Implementer.implement(repos: repos_for_prompt(st.repos), plan: container_path(st.plan_file), + resumed: session_resumable?(st)), + session_file: st.session_file) st.repos.each { |r| commit(st, r) } end @@ -1301,8 +1347,8 @@ def commit(st, repo) # on any failure so the caller can fall back to a generic subject. Shared by # gh-agent's follow-up commits and the terminal `pr` refresh. def generate_commit_subject(diff) - prompt = Prompts.commit_subject(diff: diff.patch.to_s[0, 6000]) - reply = @harness.run(prompt, tools: Harness::TOOLS_READ, model: Harness::MODEL_LIGHT) + prompt = Prompts::Scribe.commit_subject(diff: diff.patch.to_s[0, 6000]) + reply = llm(:scribe, prompt) strip_ansi(reply.to_s).lines.map(&:strip).find { |l| !l.empty? }.to_s .gsub(/\A["'`]+|["'`]+\z/, "") # strip wrapping quotes/backticks .sub(/\A\[[^\]]*\]\s*/, "") # drop any "[label]" the LLM prepended anyway @@ -1316,7 +1362,7 @@ def generate_commit_subject(diff) # Stateless — a fresh, cheap-model call rather than a resumed session, since # the item/plan/diff are all passed as file paths or plain text the model can # read itself, with nothing depending on the implement session's history. - def generate_pr_description(st, repo, model: Harness::MODEL_LIGHT) + def generate_pr_description(st, repo) pr_desc_file = st.pr_desc_file(repo) return if Helpers.file_has_content?(pr_desc_file) wt = worktree(repo) @@ -1325,11 +1371,11 @@ def generate_pr_description(st, repo, model: Harness::MODEL_LIGHT) diff_stat = wt.diff("HEAD~1", "HEAD").stats[:files] .map { |f, s| " #{f} | +#{s[:insertions]} -#{s[:deletions]}" } .join("\n") - prompt = Prompts.pr_description( + prompt = Prompts::Scribe.pr_description( item: container_path(st.item_file), plan: container_path(st.plan_file), diff_stat: diff_stat, template_section: template_section ) - pr_text = @harness.run(prompt, tools: Harness::TOOLS_READ, model: model) + pr_text = llm(:scribe, prompt) pr_body = pr_text[/^#.*/m] || pr_text pr_desc_file.write(strip_ansi(pr_body)) end diff --git a/lib/opilot/item_pictures.rb b/lib/opilot/item_pictures.rb index d3c0dbc..078cae1 100644 --- a/lib/opilot/item_pictures.rb +++ b/lib/opilot/item_pictures.rb @@ -185,6 +185,7 @@ def write_one(want, dir, api:, ctx:, budget:) [{ "id" => want["id"], "where" => want["where"], "name" => name, "content_type" => meta["contentType"], "bytes" => dest.size, + "created_at" => meta["createdAt"], "file" => Helpers.state_container_path(ctx, dest) }, false] end diff --git a/lib/opilot/pd/CLAUDE.md b/lib/opilot/pd/CLAUDE.md index a427e50..3e7b41b 100644 --- a/lib/opilot/pd/CLAUDE.md +++ b/lib/opilot/pd/CLAUDE.md @@ -71,7 +71,7 @@ PR needs the store's layout on every agent tick. `MAX_VALIDATE_ATTEMPTS` (2) revisions in the same session — the CLI isn't in the harness container, so "the agent iterates on its own output" is a runner-driven re-prompt loop. Intake spanning more than one atomic feature returns `TOO_BROAD` - with a suggested split (mirroring `Prompts.plan`'s `NEEDS_INFO`), since one change + with a suggested split (mirroring `Prompts::Planner.plan`'s `NEEDS_INFO`), since one change becomes exactly one FEATURE. **The write scope is enforced** (`#enforce_write_scope!`) — a planning stage must @@ -118,7 +118,7 @@ PR needs the store's layout on every agent tick. change in one pass produces the unreviewable PR the decomposition exists to avoid. Change and repo are resolved *from* the id via `reverse_index` (`--repo` narrows it); an unbound id lists what *is* bound instead of just failing. - `Prompts.implement_task` carries the spec paths, the WP's `item.json` (a human + `Prompts::Implementer.implement_task` carries the spec paths, the WP's `item.json` (a human comment added after the proposal qualifies or overrides it) and **only this section's checklist** — a sibling section is another work package's PR. The write scope is `propose`'s mirror image: there the spec was output and source off limits; @@ -210,7 +210,7 @@ produced a proposal with none of the template's four required headings and still passed validation. So `PD::Runner#artifact_instructions` calls `openspec instructions --change ` for each of `proposal`/`specs`/`design`/ `tasks` (dependency order — proposal `` the rest) and drops the result -straight into `Prompts.propose`. Each block carries the CLI's own ``, +straight into `Prompts::SpecWriter.propose`. Each block carries the CLI's own ``, ``, `