diff --git a/.agents/skills/matt-pocock-integration/SKILL.md b/.agents/skills/matt-pocock-integration/SKILL.md new file mode 100644 index 00000000000..2c3b7b089a2 --- /dev/null +++ b/.agents/skills/matt-pocock-integration/SKILL.md @@ -0,0 +1,66 @@ +--- +name: matt-pocock-integration +description: >- + Agent-only routing for combining FirstMate with Matt Pocock's engineering skills. + Use when the captain invokes one of those workflows, before translating it into FirstMate work, and before choosing a worker's required skills or agreed testing seams. +user-invocable: false +metadata: + internal: true +--- + +# Matt Pocock skills integration + +FirstMate owns the operating system of the work: captain interaction, authority, task classification, worker allocation, isolation, tracker mutation, supervision, delivery, and merge decisions. +Matt Pocock skills supply engineering methods inside that system. +When the two prescribe the same concern, keep FirstMate's lifecycle and use the upstream skill only for method. +For a captain-facing upstream workflow, load its installed skill and apply its interviewing, modeling, or writing method subject to the translations below. + +## Translate workflows + +| Upstream workflow | FirstMate route | +|---|---| +| `grill-me`, `grill-with-docs` | Keep the interview and decisions in the firstmate chat while delegating project investigation to scouts and approved repository changes to ships. | +| `wayfinder` | Keep the captain-facing decision map with firstmate while using scouts for research or prototypes, publishing planning artifacts through the project's approved path, and requiring separate implementation authorization. | +| `to-spec`, `to-tickets` | Preserve their planning output and dependency semantics because publishing a spec or tickets does not authorize implementation, while authorized tickets become ordinary FirstMate tasks. | +| `research`, `prototype`, `diagnosing-bugs` | Dispatch a scout when a separate evidence artifact is needed, retain the scout's report as durable evidence, and promote it only after implementation is authorized. | +| `implement` | Convert the accepted spec or tickets into ship tasks instead of invoking the upstream workflow inside a worker, where its delegation and final review steps would create a second lifecycle. | +| `code-review` | Use only when the captain explicitly requests a standalone review deliverable, dispatching it as dedicated review work rather than stacking it on a no-mistakes delivery. | +| `setup-matt-pocock-skills` | Treat setup as an explicit per-project change that inspects first, obtains the required project decisions, and ships the approved configuration through that project's delivery path. | + +If another upstream skill creates subagents, mutates the issue tracker, opens a PR, or owns final review, route those actions through the corresponding FirstMate scout, ship, tracker, or delivery contract instead of running a nested workflow. + +## Choose worker methods + +Name upstream skills by exact ID in the worker's `## Required skills` subsection. +Reference the installed skill rather than copying its procedure into the instructions. + +Use this starting map, then narrow it to what the task actually needs: + +| Work shape | Method skills | +|---|---| +| Bug or regression | `diagnosing-bugs`, then `tdd` when an executable regression seam exists | +| Feature behavior | `tdd` | +| New or changed interface | `codebase-design`, plus `tdd` when executable behavior changes | +| Domain terminology or decisions | `domain-modeling` | +| Agent-consumed instructions | `writing-for-agents` | + +Orchestration skills such as `wayfinder`, `research`, and `implement`, and final-review skills such as `code-review`, do not belong in a worker's required-method list. +The selected delivery contract remains the only final review owner, including no-mistakes when that mode is selected. + +## Record the engineering contract + +Before dispatch, fill every current ship or scout instruction scaffold as follows: + +1. `## Required skills` lists each exact method skill ID as a bullet, or contains only `None - .` when no upstream method applies. +2. `## Agreed testing seams` lists the public interfaces already made explicit or agreed with the captain, or `None - .` when no executable test is appropriate. +3. The Firstmate specification names relevant `CONTEXT.md` files and ADRs when they exist, without copying their contents. + +Do not silently replace a missing required skill with generic advice. +If no eligible worker can load it, report that blocker and ask whether to install the skill pack or revise the method. +Load `harness-adapters` before sending any harness-specific skill invocation to a live worker. + +## Completion + +The worker uses required skills during investigation and implementation, then follows only the existing Definition of done. +Planning and investigation findings are evidence, not implementation authority. +No upstream skill changes FirstMate's communication, approval, merge, or cleanup rules. diff --git a/.pi/extensions/fm-branch-supervision.ts b/.pi/extensions/fm-branch-supervision.ts index 682f0a087ab..d4fc8447437 100644 --- a/.pi/extensions/fm-branch-supervision.ts +++ b/.pi/extensions/fm-branch-supervision.ts @@ -2061,6 +2061,39 @@ ${context.command} return shell; }; + // Pi 0.99.0's stock call fallback now renders the tool's args beside the + // bold title (a muted collapsed preview) or as an indented per-arg block + // (expanded). The package does not export that formatter, so mirror its + // exact output here: calm-off rendering must be byte-identical to stock. + const STOCK_COLLAPSED_ARGS_CHARS = 100; + const stockToolCallComponent = ( + name: string, + args: unknown, + theme: Parameters>[1], + expanded: boolean, + ): Text => { + const header = theme.fg("toolTitle", theme.bold(name)); + if (args === null || args === undefined) return new Text(header, 0, 0); + const entries: [string, unknown][] = + typeof args === "object" && !Array.isArray(args) + ? Object.entries(args as Record) + : [["args", args]]; + if (entries.length === 0) return new Text(header, 0, 0); + if (expanded) { + const lines = entries.map(([key, value]) => { + const text = typeof value === "string" ? value : JSON.stringify(value, null, 2) ?? String(value); + return ` ${key}: ${text.replace(/\t/g, " ").replace(/\r/g, "").split("\n").join("\n ")}`; + }); + return new Text(`${header}\n${theme.fg("muted", lines.join("\n"))}`, 0, 0); + } + const pairs = entries.map(([key, value]) => `${key}=${JSON.stringify(value) ?? String(value)}`).join(" "); + const preview = + pairs.length > STOCK_COLLAPSED_ARGS_CHARS + ? `${pairs.slice(0, STOCK_COLLAPSED_ARGS_CHARS - 3)}...` + : pairs; + return new Text(`${header} ${theme.fg("muted", preview)}`, 0, 0); + }; + registerFirstmateTool(pi, { name: "fm_branch_outcomes", label: "Read supervision branch outcomes", @@ -2071,11 +2104,11 @@ ${context.command} recent: Type.Optional(Type.Number({ description: "How many most-recent outcomes to read (default 20)" })), }), renderShell: "self", - renderCall: (_args, theme, context) => { + renderCall: (args, theme, context) => { if (calmPresentation.stockExportRendering) throw new Error("Use Pi stock export rendering"); if (calmHides("assistant-tool-call")) return new Container(); const shellState = context.state as OutcomesToolShellState; - shellState.call = new Text(theme.fg("toolTitle", theme.bold("fm_branch_outcomes")), 0, 0); + shellState.call = stockToolCallComponent("fm_branch_outcomes", args, theme, context.expanded); return refreshOutcomesToolShell(shellState, theme, context); }, renderResult: (result, options, theme, context) => { @@ -2133,11 +2166,11 @@ ${context.command} through: Type.Number({ description: "The highest outcome sequence number this conversation has processed" }), }), renderShell: "self", - renderCall: (_args, theme, context) => { + renderCall: (args, theme, context) => { if (calmPresentation.stockExportRendering) throw new Error("Use Pi stock export rendering"); if (calmHides("assistant-tool-call")) return new Container(); const shellState = context.state as OutcomesToolShellState; - shellState.call = new Text(theme.fg("toolTitle", theme.bold("fm_branch_processed")), 0, 0); + shellState.call = stockToolCallComponent("fm_branch_processed", args, theme, context.expanded); return refreshOutcomesToolShell(shellState, theme, context); }, renderResult: (result, _options, theme, context) => { diff --git a/AGENTS.md b/AGENTS.md index 65a3197944d..3b3b5e16a4c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -543,6 +543,7 @@ Preserve durable structured identifiers, dependencies, and completion artifact l Use its scaffold as the contract, then fill `## Captain's intent` (`{TASK}`) with the captain's own ask and any boundary the captain stated, plus the context needed to read it, including the substance of any report, decision, or PR the ask refers to; never widen the ask there into a general goal or an enumerated coverage list, because the reviewer treats that subsection as acceptance criteria. Fill `## Firstmate spec` (`{FIRSTMATE_SPEC}`) with only the build instructions that ask requires, naming what stays out of scope when the ask is narrow; a generalization, consistency sweep, or extra hardening the captain did not ask for is follow-up work to note, not scope to add. `bin/fm-dod-lib.sh` owns intent authoring without added speaker labels or direct address, its provenance markers, what a no-mistakes worker may pass as `--intent`, and the string's self-sufficiency rule. +`matt-pocock-integration` owns the task-specific required-skill and agreed-testing-seam fields plus the precedence between external engineering methods and FirstMate's lifecycle. Keep additions task-specific rather than repeating lifecycle instructions, and alter generated sections only when the task genuinely differs from the standard shape. Every ship brief must retain the worktree-isolation assertion and stop if launched in the primary checkout. @@ -581,6 +582,7 @@ These skills are not captain-invocable; load them only at their precise triggers - `fmx-respond` - load on an `x-mention ` `check:` wake to handle the mention, on an `x-mode-error ...` `check:` wake to report the Relay configuration blocker, on a `public-followup ...` `check:` wake or a startup-surfaced public commitment, and on any milestone or terminal wake for a Relay-linked task before posting its completion follow-up; relevant only when Relay is on. - `firstmate-codexapp` - load before coordinating a visible Codex Desktop thread, evaluating a Codex App backend request, or reconciling Codex Desktop host-tool smoke evidence for Firstmate work. - `firstmate-coding-guidelines` - load before changing firstmate's shared, tracked material, as defined by section 1's list, whether editing directly or briefing a crewmate for a firstmate-repo task. +- `matt-pocock-integration` - load when the captain invokes a Matt Pocock workflow, before translating one into FirstMate work, and before selecting a ship or scout's required skills or agreed testing seams. ## 14. Relay diff --git a/README.md b/README.md index 89dc9a4fbaf..3d112826282 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ Launching a supported harness inside it for your primary session instantiates yo - **A visible crew** - every crewmate works in its own tmux window, Herdr tab, or experimental zellij tab, cmux workspace, or Orca terminal you can watch or type into; the first mate reconciles. - **Disposable worktrees** - each task runs in a clean [treehouse](https://github.com/kunchenguid/treehouse) git worktree, or an Orca-managed worktree when `backend=orca`, so parallel work on one repo never collides. - **Two task shapes** - ship tasks deliver authorized changes; scout tasks leave standalone investigation reports when the intake contract warrants separate research. +- **Composable engineering methods** - optional Matt Pocock skills can supply diagnosis, TDD, module-design, and domain-modeling methods while FirstMate keeps orchestration and delivery ownership. - **Explicit project modes** - each project ships via `no-mistakes`, `direct-PR`, or `local-only`, with an optional `+yolo` merge-autonomy flag. - **Optional secondmates** - opt in to persistent second mates that run from isolated firstmate homes with their own `FM_HOME`, state, projects, and session lock, either locally or as a whole home on an SSH-reachable host, with guarded updates and recovery that never turns an unavailable remote route into a local replacement. - **Event-driven, zero-token supervision** - a bash watcher sleeps on the fleet and wakes the first mate only when something needs you; verified primary harnesses also get a turn-end backstop that blocks or follows up on a blind stop when work is under way and supervision is not live. @@ -214,6 +215,7 @@ Firstmate's skills live in two separate places with different audiences: - [docs/architecture.md](docs/architecture.md) - maintainer architecture for the crew, supervision, worktrees, secondmates, and project modes. - [docs/configuration.md](docs/configuration.md) - environment variables, `FM_HOME`, runtime backend selection, optional Relay and its X and Discord setup steps, trusted external process-event adapter setup, the files you set, and harness support. +- [docs/matt-pocock-skills.md](docs/matt-pocock-skills.md) - install and use Matt Pocock's engineering skills without creating a second orchestration or review lifecycle. - [docs/extension-bindings.md](docs/extension-bindings.md) - maintainer architecture for the narrow trusted external `process-event-adapter/1` package, binding, handshake, and evidence boundary. - [docs/remote-secondmates.md](docs/remote-secondmates.md) - current setup, routing, transfer, recovery, and safety behavior for whole-home remote second mates. - [docs/calm.md](docs/calm.md) - current `/calm` behavior on Pi and Claude Code and its supported presentation limits. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index 7536546264e..f78e1b07612 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -8,8 +8,11 @@ # report, decision, or PR the ask refers to, without added speaker labels or # direct address) and `{FIRSTMATE_SPEC}` # under `## Firstmate spec` (build instructions, which are never the captain's -# intent). bin/fm-dod-lib.sh owns the no-mistakes `--intent` contract those -# subsections feed; bin/fm-spawn.sh refuses leftover placeholders and a +# intent). Their `# Engineering method` section has two more task-specific fill +# sites: `{REQUIRED_SKILLS}` names the method skills the worker must load, and +# `{TESTING_SEAMS}` names the agreed public test seams or gives the reason none +# applies. bin/fm-dod-lib.sh owns the no-mistakes `--intent` contract those +# Task subsections feed; bin/fm-spawn.sh refuses leftover placeholders and a # `## Captain's intent` line opening with a Captain label or address. Secondmate # charters still use a single `{TASK}` charter fill. Firstmate may adjust other # sections when the task genuinely deviates (e.g. working an existing external @@ -358,6 +361,32 @@ IFS= read -r -d '' TASK_SECTION <<'EOF' || true EOF TASK_SECTION=${TASK_SECTION%$'\n'} +IFS= read -r -d '' ENGINEERING_SECTION <<'EOF' || true +# Engineering method +Fill each task-specific subsection with one bullet per item, or write `None - .` + +## Required skills +{REQUIRED_SKILLS} + +## Agreed testing seams +{TESTING_SEAMS} + +## Execution +Each required-skill bullet is an exact skill ID unless the subsection contains only `None - .` +Load every named skill before using its method. +If a named skill is unavailable, report the missing skill as a blocker and stop. +Testing seams are the agreed public interfaces where behavior will be tested. +Write tests only at these seams. +For a bug, reproduce the end-user path first and observe the regression test fail before the fix. +When tests apply, work in vertical red-green slices rather than writing every test or every implementation change in one horizontal batch. + +## Ownership +FirstMate's selected delivery path is the only final review owner. +Use the required skills for engineering method while keeping orchestration, worker allocation, tracker mutation, review, PR, CI, and merge authority with FirstMate and this brief's Definition of done. +Do not invoke `code-review`, `implement`, `research`, `wayfinder`, or another workflow that delegates work or starts a second delivery path. +EOF +ENGINEERING_SECTION=${ENGINEERING_SECTION%$'\n'} + if [ "$KIND" = scout ]; then if "$SCRIPT_DIR/fm-bootstrap.sh" lavish-compatible >/dev/null 2>&1; then LAVISH_LINE='If your deliverable is a visual artifact the captain will review and iterate on, you may host the Lavish review loop yourself (poll, revise, re-serve, staying alive) instead of handing it back to firstmate.' @@ -369,6 +398,8 @@ You are a crewmate: an autonomous worker agent managed by firstmate. Work on you $TASK_SECTION +$ENGINEERING_SECTION + $HERDR_SECTION # Setup @@ -424,7 +455,7 @@ Before reporting done, read and follow \`$FM_ROOT/.agents/skills/captain-hold-li When the report is complete, append \`done: {one-line conclusion}\` to the status file and stop. If your findings reveal work that should ship (e.g. you reproduced a bug and the fix is clear), say so in the report; firstmate may promote this task in place, and you would then receive mode-specific ship instructions as a follow-up message. EOF -echo "scaffolded: $BRIEF (scout; replace {TASK} and {FIRSTMATE_SPEC})" +echo "scaffolded: $BRIEF (scout; replace {TASK}, {FIRSTMATE_SPEC}, {REQUIRED_SKILLS}, and {TESTING_SEAMS})" exit 0 fi @@ -453,6 +484,8 @@ You are a crewmate: an autonomous worker agent managed by firstmate. Work on you $TASK_SECTION +$ENGINEERING_SECTION + $HERDR_SECTION # Setup @@ -514,4 +547,4 @@ Keep it proportionate: skip \`AGENTS.md\` edits for trivial tasks that produced $DOD EOF -echo "scaffolded: $BRIEF (ship, mode=$MODE; replace {TASK} and {FIRSTMATE_SPEC})" +echo "scaffolded: $BRIEF (ship, mode=$MODE; replace {TASK}, {FIRSTMATE_SPEC}, {REQUIRED_SKILLS}, and {TESTING_SEAMS})" diff --git a/bin/fm-dod-lib.sh b/bin/fm-dod-lib.sh index 5dba2534734..4503927d491 100755 --- a/bin/fm-dod-lib.sh +++ b/bin/fm-dod-lib.sh @@ -23,10 +23,10 @@ # roughly the same specification - so a report, decision, or PR the intent # refers to is written into it as substance, never left as a pointer. # bin/fm-brief.sh scaffolds those two `# Task` subsections; bin/fm-spawn.sh and -# bin/fm-promote.sh refuse leftover `{TASK}` / `{FIRSTMATE_SPEC}` placeholders -# and a `## Captain's intent` line opening with a Captain label or address -# through the helpers below. Other mentions of `--intent` point here rather than -# restating the rule. +# bin/fm-promote.sh refuse leftover `{TASK}` / `{FIRSTMATE_SPEC}` placeholders, +# incomplete current Engineering method sections, and a `## Captain's intent` +# line opening with a Captain label or address through the helpers below. +# Other mentions of `--intent` point here rather than restating the rule. # Every heredoc here stays outside a command substitution: `VAR=$(cat < printf '%s\n' "$task" | fm_brief_heading_parse - "$2" present >/dev/null } +fm_brief_engineering_contract_present() { # + fm_brief_heading_present "$1" "# Engineering method" +} + +fm_brief_engineering_heading_body() { # + local method + method=$(fm_brief_heading_body "$1" "# Engineering method") + printf '%s\n' "$method" | fm_brief_heading_parse - "$2" body +} + +fm_brief_engineering_heading_present() { # + local method + method=$(fm_brief_heading_body "$1" "# Engineering method") + printf '%s\n' "$method" | fm_brief_heading_parse - "$2" present >/dev/null +} + +# Return 0 only when a current Engineering method subsection still consists of +# its scaffold placeholder. Literal examples elsewhere remain ordinary content. +fm_brief_engineering_placeholders_present() { # + local file=$1 skills seams + [ -f "$file" ] || return 1 + skills=$(fm_brief_engineering_heading_body "$file" "## Required skills") + seams=$(fm_brief_engineering_heading_body "$file" "## Agreed testing seams") + [ "$(printf '%s' "$skills" | tr -d '[:space:]')" = '{REQUIRED_SKILLS}' ] && return 0 + [ "$(printf '%s' "$seams" | tr -d '[:space:]')" = '{TESTING_SEAMS}' ] && return 0 + return 1 +} + +# A current Engineering method contract is complete only when both task-specific +# bodies have content. A brief without the top-level section predates the +# contract and remains launchable for recovery compatibility. +fm_brief_engineering_content_valid() { # + local file=$1 skills seams + [ -f "$file" ] && [ -r "$file" ] || return 1 + fm_brief_engineering_contract_present "$file" || return 1 + fm_brief_engineering_heading_present "$file" "## Required skills" || return 1 + fm_brief_engineering_heading_present "$file" "## Agreed testing seams" || return 1 + skills=$(fm_brief_engineering_heading_body "$file" "## Required skills") + seams=$(fm_brief_engineering_heading_body "$file" "## Agreed testing seams") + [ -n "$(printf '%s' "$skills" | tr -d '[:space:]')" ] || return 1 + [ -n "$(printf '%s' "$seams" | tr -d '[:space:]')" ] || return 1 +} + fm_brief_marked_captain_words() { # printf '%s\n' "$1" | awk ' match($0, /^[[:space:]]*(\[captain\]|Captain('\''s (words|ask|intent))?:)[[:space:]]*/) { diff --git a/bin/fm-promote.sh b/bin/fm-promote.sh index df6d6e4a3a3..a1fc6b1e4a7 100755 --- a/bin/fm-promote.sh +++ b/bin/fm-promote.sh @@ -11,12 +11,15 @@ # bin/fm-dod-lib.sh, the single owner an ordinary ship brief also uses - the # mode-specific Definition of done, so a promoted worker receives exactly the same # delivery contract as a briefed one, including the no-mistakes mode's ask-user -# escalation rule and --yes ban. The instructions also carry `# Task` with +# escalation rule and --yes ban. Current briefs also contribute their validated +# Engineering method section, so an updated promotion method reaches the live +# worker as well as any later replacement. The instructions also carry `# Task` with # `## Captain's intent` preserved from the scout brief and promotion's ship-time # instructions under `## Firstmate spec`; the scout-time spec remains context but # is not relabeled as the ship spec. Promotion refuses leftover `{TASK}` / -# `{FIRSTMATE_SPEC}` placeholders and a `## Captain's intent` line opening with -# a Captain label or address (bin/fm-dod-lib.sh). A pre-subsection scout +# `{FIRSTMATE_SPEC}`/`{REQUIRED_SKILLS}`/`{TESTING_SEAMS}` placeholders, +# an empty current Engineering method subsection, and a `## Captain's intent` +# line opening with a Captain label or address (bin/fm-dod-lib.sh). A pre-subsection scout # brief contributes only Task lines explicitly marked as captain words to intent. # A scout records no delivery posture, so promotion is where this task's delivery # contract is decided: --mode and --yolo are REQUIRED and written into the meta @@ -153,6 +156,16 @@ if ! fm_brief_task_content_valid "$SCOUT_BRIEF"; then echo "error: $SCOUT_BRIEF must contain nonempty ## Captain's intent and ## Firstmate spec subsections (or a nonempty legacy # Task body) before promotion" >&2 exit 1 fi +if fm_brief_engineering_contract_present "$SCOUT_BRIEF"; then + if fm_brief_engineering_placeholders_present "$SCOUT_BRIEF"; then + echo "error: $SCOUT_BRIEF still contains {REQUIRED_SKILLS} or {TESTING_SEAMS}; fill ## Required skills and ## Agreed testing seams before promotion" >&2 + exit 1 + fi + if ! fm_brief_engineering_content_valid "$SCOUT_BRIEF"; then + echo "error: $SCOUT_BRIEF must contain nonempty ## Required skills and ## Agreed testing seams under # Engineering method before promotion" >&2 + exit 1 + fi +fi if ADDRESS_LINE=$(fm_brief_intent_address_line "$SCOUT_BRIEF"); then echo "error: $SCOUT_BRIEF ## Captain's intent has an operator-address line: $ADDRESS_LINE; write the captain's actual words without a Captain label or address before promotion, since the heading already records provenance" >&2 exit 1 @@ -167,6 +180,15 @@ if [ -z "$(printf '%s' "$INTENT_BODY" | tr -d '[:space:]')" ]; then echo "error: $SCOUT_BRIEF has no provenance-marked Captain's intent; add the captain's actual words before promotion" >&2 exit 1 fi +ENGINEERING_SECTION= +if fm_brief_engineering_contract_present "$SCOUT_BRIEF"; then + ENGINEERING_BODY=$(fm_brief_heading_body "$SCOUT_BRIEF" "# Engineering method") + IFS= read -r -d '' ENGINEERING_SECTION < "$TMP" || { echo "error: could not render ship instructions for mode=$MODE" >&2; exit 1; } diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index d9867cca406..71082d5c45c 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -12,9 +12,9 @@ # "Delivery contract: mode=" line and REFUSES a mismatch, so the worker's # instructions and the recorded task delivery cannot drift apart; a brief # scaffolded before that line existed warns once and launches on the flag. A -# ship or scout spawn also refuses leftover `{TASK}` / `{FIRSTMATE_SPEC}` -# placeholders, an empty Task, an incomplete pair of Task subsections, or a -# `## Captain's intent` line opening with a Captain label or address. +# ship or scout spawn also refuses leftover Task or Engineering method +# placeholders, empty current task-specific subsections, or a `## Captain's +# intent` line opening with a Captain label or address. # Every ship or scout spawn renders `launch-brief.md`; for a no-mistakes ship # it also carries the current `--intent` contract and the extracted captain # intent. A legacy mixed Task is accepted there only under bin/fm-dod-lib.sh's @@ -2551,6 +2551,16 @@ if [ "$KIND" = ship ] || [ "$KIND" = scout ]; then echo "error: $BRIEF must contain nonempty ## Captain's intent and ## Firstmate spec subsections (or a nonempty legacy # Task body) before spawn" >&2 exit 1 fi + if fm_brief_engineering_contract_present "$BRIEF"; then + if fm_brief_engineering_placeholders_present "$BRIEF"; then + echo "error: $BRIEF still contains {REQUIRED_SKILLS} or {TESTING_SEAMS}; fill ## Required skills and ## Agreed testing seams before spawn" >&2 + exit 1 + fi + if ! fm_brief_engineering_content_valid "$BRIEF"; then + echo "error: $BRIEF must contain nonempty ## Required skills and ## Agreed testing seams under # Engineering method before spawn" >&2 + exit 1 + fi + fi if ADDRESS_LINE=$(fm_brief_intent_address_line "$BRIEF"); then echo "error: $BRIEF ## Captain's intent has an operator-address line: $ADDRESS_LINE; write the captain's actual words without a Captain label or address before spawn, since the heading already records provenance" >&2 exit 1 diff --git a/docs/architecture.md b/docs/architecture.md index 9e13ff818b7..351879cb8fa 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -313,6 +313,16 @@ The [`secondmate-provisioning` skill](../.agents/skills/secondmate-provisioning/ The `data/secondmates.md` line contract is owned by the [`secondmate-provisioning` skill](../.agents/skills/secondmate-provisioning/SKILL.md#routing-table), and the secondmate environment variables are documented in [configuration.md](configuration.md). +## Engineering methods stay inside the task lifecycle + +FirstMate owns captain interaction, task classification, worker allocation, isolation, supervision, delivery, and merge authority, while optional Matt Pocock skills contribute engineering methods inside that lifecycle. +The [`matt-pocock-integration` skill](../.agents/skills/matt-pocock-integration/SKILL.md) translates upstream planning and implementation workflows into FirstMate scouts, ships, and planning artifacts rather than allowing a nested agent fleet or second delivery path. +Every newly scaffolded ship or scout instruction file has task-specific `Required skills` and `Agreed testing seams` subsections, and `bin/fm-spawn.sh` refuses a current scaffold while either is unfilled or empty. +Saved instructions that predate the section remain launchable for recovery compatibility. +`bin/fm-promote.sh` validates the same contract and sends the current engineering method to the promoted worker, while preserving it in the instructions used by any later replacement. +The selected task delivery mode remains the sole final-review owner, so a no-mistakes task never stacks an upstream `code-review` or `implement` completion flow on top of its pipeline. +See [Matt Pocock engineering skills](matt-pocock-skills.md) for installation and operator-facing behavior. + ## Delivery modes are explicit per task `no-mistakes` tasks run the full validation pipeline, `direct-PR` tasks open PRs without that pipeline, and `local-only` tasks stay local until firstmate performs an approved fast-forward merge. diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index e459e95006a..aadbcebb7c1 100644 --- a/docs/documentation-audiences.json +++ b/docs/documentation-audiences.json @@ -25,6 +25,7 @@ ], "readmeSetupTargets": [ "docs/configuration.md", + "docs/matt-pocock-skills.md", "docs/wedge-alarm.md", "docs/tmux-backend.md", "docs/herdr-backend.md", @@ -152,6 +153,10 @@ "path": ".agents/skills/firstmate-orca/SKILL.md", "audience": "agent-runtime" }, + { + "path": ".agents/skills/matt-pocock-integration/SKILL.md", + "audience": "agent-runtime" + }, { "path": ".agents/skills/fmx-respond/SKILL.md", "audience": "agent-runtime" @@ -368,6 +373,10 @@ "path": "docs/herdr-backend.md", "audience": "operator-current" }, + { + "path": "docs/matt-pocock-skills.md", + "audience": "operator-current" + }, { "path": "docs/orca-backend.md", "audience": "operator-current" diff --git a/docs/matt-pocock-skills.md b/docs/matt-pocock-skills.md new file mode 100644 index 00000000000..bf6bebcda34 --- /dev/null +++ b/docs/matt-pocock-skills.md @@ -0,0 +1,50 @@ +# Matt Pocock engineering skills + +FirstMate can use the [mattpocock/skills](https://github.com/mattpocock/skills) pack as an engineering-method library while retaining its own orchestration and delivery lifecycle. +FirstMate remains responsible for user decisions, worker allocation, isolation, supervision, pull requests, CI, and merge authority. +The external skills contribute methods such as diagnosis, test-driven development, module design, and domain modeling. + +## Install + +Install the complete pack globally for every agent harness that may run either the firstmate or its workers. +The complete pack matters because user-facing workflow skills delegate to smaller method skills by ID. + +```sh +npx --yes skills@latest add mattpocock/skills \ + --skill '*' \ + --global \ + --agent claude-code \ + --agent codex \ + --agent opencode \ + --agent pi \ + --yes +``` + +Adjust the repeated `--agent` flags to the worker harnesses you actually use. +Do not install a second copy through another plugin system when that would expose duplicate skill definitions to the same harness. + +## How routing works + +Captain-facing workflows such as `grill-me` and `wayfinder` remain coordinated by FirstMate. +Project investigation, prototypes, implementation, and standalone reviews are dispatched through normal scout or ship work rather than through a nested agent fleet. +Publishing a plan, specification, ticket map, or investigation report never grants implementation authority. + +Every newly scaffolded ship or scout instruction file records two task-specific inputs: + +- Required skill IDs, or a reason no external method applies. +- Agreed public testing seams, or a reason no executable test applies. + +The launch is refused while either input is still a scaffold placeholder or has an empty body. +If a named skill is unavailable to the worker, the worker reports the missing skill instead of silently falling back. +Saved instructions created before this contract remain launchable for recovery compatibility. + +## Review ownership + +The task's selected FirstMate delivery path remains the only final review owner. +In particular, a worker does not invoke `implement` or `code-review` as an extra completion flow when no-mistakes already owns review, fixes, tests, documentation, the pull request, and CI. +A standalone code review remains available when explicitly requested as its own deliverable. + +## Project setup + +This integration does not modify individual projects. +Run `setup-matt-pocock-skills` separately for a project only when you want its issue-tracker and domain-document conventions configured, and ship those changes through the project's existing FirstMate delivery policy. diff --git a/docs/scripts.md b/docs/scripts.md index 5b8ceb56d3e..fbb5e02a579 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -32,8 +32,8 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-backlog-receive.sh` | Idempotently ingest one confined remote handoff outbox through tasks-axi | | `fm-captain-hold.sh` | Hold tasks for the captain, record the captain's answers, gate investigation completion, and report record divergence between the status log and the backlog | | `fm-decision-hold.sh` | One-release compatibility shim mapping the retired decision commands onto fm-captain-hold.sh | -| `fm-brief.sh` | Scaffold ship (explicit `--mode`), scout, secondmate-charter, and Herdr-lab briefs, with Captain's intent and Firstmate spec subsections on ship/scout | -| [`fm-dod-lib.sh`](../bin/fm-dod-lib.sh) | Own ship/scout worker role scope, ship definitions of done, and the no-mistakes `--intent` contract | +| `fm-brief.sh` | Scaffold ship, scout, secondmate-charter, and Herdr-lab instructions, including task-specific engineering methods for ships and scouts | +| [`fm-dod-lib.sh`](../bin/fm-dod-lib.sh) | Own worker role scope, instruction-section parsing, ship definitions of done, and the no-mistakes `--intent` contract | | `fm-herdr-lab.sh` | Provision and guardedly operate an isolated, never-default Herdr lab session | | `fm-herdr-lab-viewer.py` | The pty engine behind `fm-herdr-lab.sh viewer`: one real foreground Herdr client on a non-zero window grid | | `fm-install-herdr.sh` | Install CI's exact-version Herdr pin with official asset URL, SHA-256, and protocol checks | diff --git a/tests/fixtures.sh b/tests/fixtures.sh index 043d350012e..f138016c411 100755 --- a/tests/fixtures.sh +++ b/tests/fixtures.sh @@ -247,6 +247,13 @@ $intent ## Firstmate spec Exercise the spawn behavior under test. + +# Engineering method +## Required skills +- tdd + +## Agreed testing seams +- The task launch interface. EOF } diff --git a/tests/fm-afk-inject-herdr-e2e.test.sh b/tests/fm-afk-inject-herdr-e2e.test.sh index e761336e7b4..e67fd411298 100755 --- a/tests/fm-afk-inject-herdr-e2e.test.sh +++ b/tests/fm-afk-inject-herdr-e2e.test.sh @@ -59,17 +59,93 @@ SUPERVISOR_TARGET= PANE_ID= LOOP_SCRIPT= +# Hard bound for every herdr CLI call made through the shim below, and the +# join budget for a TERM'd supervise daemon. A wedged herdr server socket +# makes an unbounded CLI call (or the daemon blocked inside one) hang this +# script with no per-command evidence until its caller's whole-step tripwire +# fires (observed on CI run 37309436481: a 20-minute blind step timeout). 30s +# is two orders of magnitude over a healthy round trip against a real lab, so +# a bound hit names a genuine wedge, not scheduling slack. +HERDR_CMD_BOUND_SECS=30 + +# join_daemon: TERM the supervise daemon and wait for it to exit, escalating +# to KILL after HERDR_CMD_BOUND_SECS rather than waiting without bound - the +# daemon can outlive TERM indefinitely while it is blocked inside a wedged +# herdr call. Returns 1 when escalation was needed, so callers can name the +# timeout instead of dying blind inside `wait`. +join_daemon() { + local waited=0 + [ -n "${DAEMON_PID:-}" ] || return 0 + kill "$DAEMON_PID" 2>/dev/null || true + while kill -0 "$DAEMON_PID" 2>/dev/null && [ "$waited" -lt "$HERDR_CMD_BOUND_SECS" ]; do + sleep 1 + waited=$((waited + 1)) + done + if kill -0 "$DAEMON_PID" 2>/dev/null; then + kill -KILL "$DAEMON_PID" 2>/dev/null || true + wait "$DAEMON_PID" 2>/dev/null || true + DAEMON_PID= + return 1 + fi + wait "$DAEMON_PID" 2>/dev/null || true + DAEMON_PID= + return 0 +} + cleanup_all() { if [ -n "${DAEMON_PID:-}" ]; then afk_exit "${STATE_DIR:-}" 2>/dev/null || true - kill "$DAEMON_PID" 2>/dev/null || true - wait "$DAEMON_PID" 2>/dev/null || true + join_daemon || printf 'cleanup: supervise daemon did not exit within %ss of TERM; killed\n' "$HERDR_CMD_BOUND_SECS" >&2 fi herdr_safe_stop_and_delete "$SESSION" 2>/dev/null || true rm -rf "${HERDR_SHIM_DIR:-}" 2>/dev/null || true rm -rf "${STATE_DIR:-}" 2>/dev/null || true } trap cleanup_all EXIT + +STATE_DIR=$(mktemp -d "${TMPDIR:-/tmp}/fm-afk-herdr-e2e.XXXXXX") +mkdir -p "$STATE_DIR" +LOG_FILE="$STATE_DIR/submitted.log" +: > "$LOG_FILE" + +# --- herdr shim: forwards to the real binary, optionally swallows one Enter, +# and bounds every other call ------------------------------------------------- +# The swallow-half intercepts exactly one `pane send-keys enter` while +# Scenario B's .swallow-enter flag file exists (herdr's real CLI has no +# built-in way to drop a keystroke). The bound-half wraps every remaining call +# in fm-timeout-lib.sh's fm_run_timed so a wedged herdr server becomes a named +# per-command failure (exit 124, the exceeded bound named on stderr) instead +# of an unbounded suite hang. The long-lived `server` launch is exempt: the +# lab server under test must outlive any single call's bound. The shim sits +# first on PATH for the rest of the run, so lab provisioning (fm-herdr-lab), +# backend adapter operations, the supervise daemon and the supervisor pane's +# own herdr calls (both inherit this PATH), and teardown all sit under it. +REAL_HERDR=$(command -v herdr) +HERDR_SHIM_DIR=$(mktemp -d "${TMPDIR:-/tmp}/fm-herdr-shim.XXXXXX") +cat > "$HERDR_SHIM_DIR/herdr" <&2 +fi +exit "\$rc" +SHIM +chmod +x "$HERDR_SHIM_DIR/herdr" +export PATH="$HERDR_SHIM_DIR:$PATH" + fm_herdr_lab_prepare "$SESSION" || fail "could not prepare isolated Herdr lab session" # --- source the daemon (for afk_enter/afk_exit/FM_INJECT_MARK) + the backend - @@ -81,11 +157,6 @@ fm_backend_source herdr || fail "fm_backend_source herdr failed" fm_backend_herdr_version_check || fail "version_check failed against the real installed herdr" -STATE_DIR=$(mktemp -d "${TMPDIR:-/tmp}/fm-afk-herdr-e2e.XXXXXX") -mkdir -p "$STATE_DIR" -LOG_FILE="$STATE_DIR/submitted.log" -: > "$LOG_FILE" - CONTAINER_RAW=$(fm_backend_herdr_container_ensure /tmp) || fail "container_ensure failed" CONTAINER=${CONTAINER_RAW%%$'\t'*} SEEDED_TAB_ID=${CONTAINER_RAW#*$'\t'} @@ -233,23 +304,6 @@ fm_backend_herdr_send_text_line "$SUPERVISOR_TARGET" "bash '$LOOP_SCRIPT' '$LOG_ || fail "could not start the supervisor-loop script in the scratch herdr pane" sleep 1 # let the loop start and settle -# --- herdr shim: forwards to the real binary, optionally swallows one Enter -- -REAL_HERDR=$(command -v herdr) -HERDR_SHIM_DIR=$(mktemp -d "${TMPDIR:-/tmp}/fm-herdr-shim.XXXXXX") -cat > "$HERDR_SHIM_DIR/herdr" </dev/null || true - kill "$DAEMON_PID" 2>/dev/null || true - wait "$DAEMON_PID" 2>/dev/null || true - DAEMON_PID="" + join_daemon || fail "the supervise daemon did not exit within ${HERDR_CMD_BOUND_SECS}s of TERM (wedged inside a herdr call?); it was killed" sleep 1 } @@ -328,7 +379,7 @@ selfcheck_pane_input_pending() { fm_backend_herdr_send_literal "$SUPERVISOR_TARGET" "$check_text" \ || fail "selfcheck: could not send literal text to the scratch pane" sleep 0.5 - if PATH="$HERDR_SHIM_DIR:$PATH" pane_input_pending "$SUPERVISOR_TARGET" herdr; then + if pane_input_pending "$SUPERVISOR_TARGET" herdr; then fm_backend_herdr_send_key "$SUPERVISOR_TARGET" Enter sleep 0.5 return 0 @@ -484,7 +535,6 @@ test_scenario_d_max_defer() { fm_backend_herdr_send_literal "$SUPERVISOR_TARGET" "stuck-in-the-box" sleep 0.5 - PATH="$HERDR_SHIM_DIR:$PATH" \ HERDR_SESSION="$SESSION" \ FM_STATE_OVERRIDE="$STATE_DIR" \ FM_SUPERVISOR_BACKEND=herdr \ diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 88f4e566ff0..0d08f622f2a 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -213,6 +213,15 @@ test_ship_modes_generate_clean_briefs() { assert_grep "{FIRSTMATE_SPEC}" "$brief" "$id: brief missing the {FIRSTMATE_SPEC} placeholder" assert_grep "## Captain's intent" "$brief" "$id: brief missing Captain's intent subsection" assert_grep "## Firstmate spec" "$brief" "$id: brief missing Firstmate spec subsection" + assert_grep "# Engineering method" "$brief" "$id: brief missing Engineering method section" + assert_grep "## Required skills" "$brief" "$id: brief missing Required skills subsection" + assert_grep "{REQUIRED_SKILLS}" "$brief" "$id: brief missing the {REQUIRED_SKILLS} placeholder" + assert_grep "## Agreed testing seams" "$brief" "$id: brief missing Agreed testing seams subsection" + assert_grep "{TESTING_SEAMS}" "$brief" "$id: brief missing the {TESTING_SEAMS} placeholder" + assert_grep "FirstMate's selected delivery path is the only final review owner." "$brief" \ + "$id: brief did not keep final review with FirstMate's delivery path" + assert_grep "Do not invoke \`code-review\`, \`implement\`, \`research\`, \`wayfinder\`, or another workflow that delegates work or starts a second delivery path." "$brief" \ + "$id: brief did not prevent nested orchestration and duplicate review" assert_grep 'never a bare number such as "PR 108"' "$brief" "$id: brief missing the full-PR-URL rule" assert_grep "mid-task \`working:\` line (including setup complete) is nonterminal" "$brief" \ "$id: brief missing nonterminal working:/setup-complete gate protection" @@ -887,6 +896,9 @@ test_scout_and_secondmate_scaffold() { assert_grep "## Captain's intent" "$brief" "scout brief missing Captain's intent subsection" assert_grep "## Firstmate spec" "$brief" "scout brief missing Firstmate spec subsection" assert_grep "{FIRSTMATE_SPEC}" "$brief" "scout brief missing the spec placeholder" + assert_grep "# Engineering method" "$brief" "scout brief missing Engineering method section" + assert_grep "{REQUIRED_SKILLS}" "$brief" "scout brief missing the Required skills placeholder" + assert_grep "{TESTING_SEAMS}" "$brief" "scout brief missing the testing seams placeholder" FM_SECONDMATE_CHARTER='Supervise the alpha domain.' \ FM_HOME="$BRIEF_HOME" "$ROOT/bin/fm-brief.sh" brief-sm-q6 --secondmate alpha >/dev/null 2>&1 \ @@ -899,6 +911,8 @@ test_scout_and_secondmate_scaffold() { "secondmate charter must not grow ship/scout Task subsections" assert_no_grep "{FIRSTMATE_SPEC}" "$brief" \ "secondmate charter must not carry the Firstmate spec placeholder" + assert_no_grep "# Engineering method" "$brief" \ + "secondmate charter must not carry a task engineering method" pass "fm-brief: scout and secondmate code paths still scaffold well-formed briefs" } diff --git a/tests/fm-calm-pi-extension.test.sh b/tests/fm-calm-pi-extension.test.sh index 156136b01fe..53e286eaed5 100755 --- a/tests/fm-calm-pi-extension.test.sh +++ b/tests/fm-calm-pi-extension.test.sh @@ -1299,14 +1299,18 @@ for (const { name, actual } of rows) { throw new Error(`${name} was not hidden before export rendering`); } } +// Pi 1.0.1 renamed the createToolHtmlRenderer lookup dep from getToolDefinition +// to getToolRenderers (exportToHtml now resolves renderers through the extension +// runner). Older Pi destructures the old name and ignores the extra key, so the +// fixture passes both spellings of the same lookup to cover either era. +const exportToolHtmlDeps = () => { + const lookup = (name) => tools.find((tool) => tool.name === name); + return { getToolDefinition: lookup, getToolRenderers: lookup, theme, cwd: process.cwd() }; +}; async function assertStockHtmlRendering(command, submitData) { editorText = command; terminalInputHandler(submitData); - const htmlRenderer = createToolHtmlRenderer({ - getToolDefinition: (name) => tools.find((tool) => tool.name === name), - theme, - cwd: process.cwd(), - }); + const htmlRenderer = createToolHtmlRenderer(exportToolHtmlDeps()); const exportCases = [ ...cases.filter(([toolName]) => toolName === "grep" || toolName === "find"), ["fm_watch_arm_pi", watchArgs, watchResult], @@ -1333,11 +1337,7 @@ await assertStockHtmlRendering("/export calm.html", "\r"); getKeybindings().setUserBindings({ "tui.input.submit": "alt+s" }); editorText = "/export remapped.html"; terminalInputHandler("\r"); -const unmatchedRenderer = createToolHtmlRenderer({ - getToolDefinition: (name) => tools.find((tool) => tool.name === name), - theme, - cwd: process.cwd(), -}); +const unmatchedRenderer = createToolHtmlRenderer(exportToolHtmlDeps()); if (unmatchedRenderer.renderCall("unmatched-submit", "grep", { pattern: "alpha", path: "." })) { throw new Error("ordinary non-submit input activated HTML export rendering"); } @@ -3577,7 +3577,11 @@ TS {"type":"message","id":"a0000016","parentId":"a0000015","timestamp":"$now","message":{"role":"assistant","content":[{"type":"text","text":"The deterministic tool example is complete."}],"api":"anthropic-messages","provider":"anthropic","model":"claude-sonnet-4-5","usage":{"input":2,"output":1,"cacheRead":0,"cacheWrite":0,"totalTokens":3,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":16}} JSON - tmux -L "$TMUX_SOCKET" new-session -d -s "$TMUX_SESSION" -x 180 -y 44 \ + # The pane must be tall enough to hold the whole restored transcript plus every + # row this E2E appends later: since Pi 1.0's fullscreen TUI the transcript lives + # in Pi's own virtualized scrollback, so the tmux capture below (viewport plus + # recent history) only ever sees rows currently on screen. + tmux -L "$TMUX_SOCKET" new-session -d -s "$TMUX_SESSION" -x 180 -y 400 \ "cd '$project' && env FM_HOME='$home' PI_CODING_AGENT_DIR='$config' FM_OPERATIONAL_INPUT_SCRIPT='$OPERATIONAL_INPUT' PI_OFFLINE=1 pi --approve --no-skills --no-prompt-templates --no-context-files --session '$session_file'; rc=\$?; printf '\nPI_EXIT=%s\n' \"\$rc\"; sleep 30" wait_for_text "$default_snapshot" "The deterministic tool example is complete." \ || fail "Pi calm E2E did not reach the restored session transcript" @@ -3792,8 +3796,19 @@ const tree = dom.match(/]*id="tree-container"[^>]*>([\s\S]*?)]*id= if (!messages || !tree) process.exit(1); if (!/
]*>[\s\S]*Show a deterministic tool example\./.test(messages)) process.exit(1); if (!/
]*>[\s\S]*The deterministic tool example is complete\./.test(messages)) process.exit(1); -if (messages.includes('