diff --git a/skills/first-officer/references/first-officer-shared-core.md b/skills/first-officer/references/first-officer-shared-core.md index 88fd8852b..3083c0200 100644 --- a/skills/first-officer/references/first-officer-shared-core.md +++ b/skills/first-officer/references/first-officer-shared-core.md @@ -74,7 +74,7 @@ When a worker completes: If not gated: terminal → merge; else decide reuse-or-fresh. -**A completed non-gated, non-terminal stage is not a stopping point.** After verifying the report, the FO MUST advance the entity to the next stage and dispatch it (reuse-or-fresh per the dispatch module's reuse conditions) BEFORE ending its turn. The FO does not file a completion-only status and stop, waiting for the captain or a later turn to resume — advancing is the FO's next action, not the captain's. The only conditions that legitimately halt the turn here are: the next stage is `gate: true` (present the gate and wait), the entity is terminal (run the merge/cleanup ceremony), an explicit blocker (a `«halt.rebase-conflict»`, an unmet clarification), or a captain decision the contract requires. Absent one of those, stopping after a completion-only report is a contract violation. +**A completed non-gated, non-terminal stage is not a stopping point.** After verifying the report, the FO MUST advance the entity to the next stage and dispatch it (reuse-or-fresh per the dispatch module's reuse conditions) BEFORE ending its turn. The FO does not file a completion-only status and stop, waiting for the captain or a later turn to resume — advancing is the FO's next action, not the captain's. The only conditions that legitimately halt the turn here are: the next stage is `gate: true` (present the gate and wait), the entity is terminal (run the merge/cleanup ceremony), an explicit blocker (a `«halt.rebase-conflict»`, an unmet clarification), or a captain decision the contract requires. Absent one of those, stopping after a completion-only report is a contract violation. A blocker or captain decision halting the turn is rendered through `Skill(skill="spacedock:present-gate")` and its `## Decision Request` template — the FO's own recommendation, never the worker's options relayed as the option space. **Advancing a completed worker (reuse-or-fresh)** — the reuse conditions, the reuse/fresh-dispatch procedures, and supersede-shutdown live in the deferred dispatch module, already loaded by the time a completion reaches this point. Reuse only when the worker is addressable through a live runtime handle AND every reuse condition passes; otherwise dispatch fresh. diff --git a/skills/integration/decision_request_live_test.go b/skills/integration/decision_request_live_test.go new file mode 100644 index 000000000..42111e4c1 --- /dev/null +++ b/skills/integration/decision_request_live_test.go @@ -0,0 +1,83 @@ +//go:build live + +// ABOUTME: Live drive of the present-gate decision-request rendering — runs a real first +// ABOUTME: officer over the halted-worker fixture and grades its final message. +package integration + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func writeDecisionRequestFixture(t *testing.T) string { + t.Helper() + dir := t.TempDir() + for name, body := range map[string]string{ + "README.md": decisionRequestWorkflow(), + "reading.md": decisionRequestEntity(), + } { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + for _, args := range [][]string{ + {"init", "-q"}, + {"-c", "user.name=Spacedock", "-c", "user.email=test@example.invalid", "add", "README.md", "reading.md"}, + {"-c", "user.name=Spacedock", "-c", "user.email=test@example.invalid", "commit", "-qm", "fixture"}, + } { + cmd := exec.Command("git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + return dir +} + +// TestLiveDecisionRequest is the only check here that can establish the template +// works; the offline table can only establish that its graders were not +// loosened. Grading runs through gradeDecisionRequest, the same entry point that +// table pins, so a grader relaxed to turn this green breaks a case there. +func TestLiveDecisionRequest(t *testing.T) { + bin := os.Getenv("SPACEDOCK_BIN") + if bin == "" { + t.Fatal("set SPACEDOCK_BIN to the current spacedock binary") + } + repo := os.Getenv("SPACEDOCK_REPO_ROOT") + if repo == "" { + out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() + if err != nil { + t.Fatalf("resolve repo root: %v", err) + } + repo = strings.TrimSpace(string(out)) + } + + fixture := writeDecisionRequestFixture(t) + finalPath := filepath.Join(fixture, "final.txt") + + cmd := exec.Command(bin, "codex", "--plugin-dir", repo, "--skip-compat-check", + decisionRequestPrompt(fixture), + "--", "exec", "--json", "--dangerously-bypass-approvals-and-sandbox", + "--cd", fixture, "--output-last-message", finalPath) + cmd.Dir = fixture + stream, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("live run: %v\n%s", err, stream) + } + + final, err := os.ReadFile(finalPath) + if err != nil { + t.Fatalf("read final message: %v", err) + } + if dir := os.Getenv("SPACEDOCK_LIVE_ARTIFACT_DIR"); dir != "" { + _ = os.WriteFile(filepath.Join(dir, "decision-request-final.txt"), final, 0o644) + _ = os.WriteFile(filepath.Join(dir, "decision-request-stream.jsonl"), stream, 0o644) + } + + if failures := gradeDecisionRequest(string(final)); len(failures) > 0 { + t.Fatalf("decision request failed grading %v\n\n%s", failures, final) + } +} diff --git a/skills/integration/decision_request_test.go b/skills/integration/decision_request_test.go new file mode 100644 index 000000000..11c3c8f35 --- /dev/null +++ b/skills/integration/decision_request_test.go @@ -0,0 +1,414 @@ +// ABOUTME: Grading for the present-gate decision-request rendering, its live-fixture +// ABOUTME: builders, and the offline table that pins each grader against a failure it must reject. +package integration + +import ( + "regexp" + "sort" + "strings" + "testing" +) + +// The decision-request template exists because a first officer with no template +// for a mid-stage captain decision relays the halted worker's options. Grading +// reads the rendered final message: a decision request records nothing and moves +// no stage, so the message is its only observable. +// +// This table does NOT establish that the template works — only TestLiveDecisionRequest +// does, and it needs a model. It establishes that no grader can be loosened to +// turn a red live run green without breaking a case here. + +var ( + // A derivation a reader can open: a file, a line anchor, or a command. + reproducibleSourceRe = regexp.MustCompile(`\.md|\.go|\.sh|:[0-9]+|spacedock [a-z]`) + + // The bypass this field exists to stop: pointing at the worker's own + // summary, which is the input, in place of the evidence under it. + workerSummaryRe = regexp.MustCompile(`(?i)the (worker|ensign)'?s? (report|summary|options|list)( says| states)?[.,;]?$`) + + // A recommendation that reduces what gets delivered. Every option a worker + // can offer moves the budget or the structure; none moves the requirement, + // because "build less" is outside the remit of a worker told to build. + reducesSurfaceRe = regexp.MustCompile(`(?i)defer|drop|remove|cut|reduce|narrow|only|without`) + + // The three options the fixture's worker offered. A recommendation naming + // one of them is relayed, however well it is worded. + relayedOptionRe = regexp.MustCompile(`(?i)1,?400|raise the (stop|limit|number)|new (internal )?package|extract a package|in half|expiry`) + + // The surface the fixture puts beyond the worker's reach: it serves a user + // who does not exist yet, so only a re-derivation reaches it. + unrelayedSurfaceRe = regexp.MustCompile(`(?i)installed[- ]plugin|registration`) + + // The menu handed back rather than decided. + menuHandbackRe = regexp.MustCompile(`(?i)which of the three|pick one of the (three|3)|choose from the options above`) + + recommendLineRe = regexp.MustCompile(`(?im)^recommend.*$`) +) + +// derivedFromBlock returns the `Derived from` paragraph, or "" when the field is +// absent. The block ends at the first blank line, matching the template's shape. +func derivedFromBlock(final string) string { + lines := strings.Split(final, "\n") + for i, line := range lines { + if !strings.Contains(strings.ToLower(line), "derived from") { + continue + } + block := []string{line} + for _, next := range lines[i+1:] { + if strings.TrimSpace(next) == "" { + break + } + block = append(block, next) + } + return strings.Join(block, "\n") + } + return "" +} + +// gradeDecisionRequest returns every way the rendered message fails the +// decision-request contract, sorted so a table test can compare them directly. +// An empty result means the message satisfies every graded property. +func gradeDecisionRequest(final string) []string { + var failures []string + add := func(f string) { failures = append(failures, f) } + + // Presence is weak: a first officer with no template still writes + // "Decision request:" and a Recommend line. Graded first only so a missing + // field reports as itself rather than as the substantive failure below it. + for _, field := range []string{"decision request", "recommend", "derived from", "remit"} { + if !strings.Contains(strings.ToLower(final), field) { + add("missing-field:" + strings.ReplaceAll(field, " ", "-")) + } + } + + derived := derivedFromBlock(final) + if derived != "" { + if !reproducibleSourceRe.MatchString(derived) { + add("derived-from-cites-nothing-reproducible") + } + if workerSummaryRe.MatchString(derived) { + add("derived-from-names-the-worker-summary") + } + } + + // Exactly one recommendation: a list handed to the captain is the failure + // this template exists to catch. + recommends := recommendLineRe.FindAllString(final, -1) + switch len(recommends) { + case 1: + line := recommends[0] + // Two guards on the one line, because the failure produces a + // well-formed recommendation carrying a relayed option. The first alone + // passes on "cut slice 1 in half", which is the worker's own option 3. + if !reducesSurfaceRe.MatchString(line) { + add("recommendation-does-not-reduce-the-delivered-surface") + } + // Relaying is being CONFINED to what the worker could see, not naming one + // of its options: a recommendation whose substance is the un-relayed + // surface may carry a worker option beside it. Both directions are pinned + // below. + if relayedOptionRe.MatchString(line) && !unrelayedSurfaceRe.MatchString(line) { + add("recommendation-relays-a-worker-option") + } + case 0: + // Already reported as a missing field. + default: + add("more-than-one-recommendation") + } + + if !unrelayedSurfaceRe.MatchString(final) { + add("never-names-the-surface-with-no-user-today") + } + if menuHandbackRe.MatchString(final) { + add("hands-the-menu-back-to-the-captain") + } + + sort.Strings(failures) + return failures +} + +// decisionRequestCase is one rendered message and the exact set of graders it +// must trip. Each is written here rather than recorded: a recording of a run the +// graders were tuned against asserts only that the tuning happened. +type decisionRequestCase struct { + name string + msg string + want []string + why string +} + +var decisionRequestCases = []decisionRequestCase{ + { + name: "answered", + msg: `Decision request: Publish a document and hand out its link — implementation +Recommend shipping only the Go subcommand and deferring the installed-plugin entry points until such a user exists. +Raised by: the worker crossed its declared stop number and halted. + +Derived from: README.md:16 declares the limit; reading.md:11 records that the one waiting user has a checkout. + +Outside the worker's remit: reduce the requirement to the Go subcommand; an implementation worker could not remove its own scope. + +Alternatives: raising the limit keeps unused surface; a package adds structure without reducing scope. + +Decision: approve to narrow the slice. +`, + want: nil, + why: "guards a later tightening. It does not establish that a first officer writes this — " + + "only TestLiveDecisionRequest does, and it needs a model", + }, + { + name: "derivation names the worker's summary", + msg: `Decision request: x — implementation +Recommend deferring the installed-plugin registration surfaces. + +Derived from: the worker's report. + +Outside the worker's remit: none. +`, + want: []string{ + "derived-from-cites-nothing-reproducible", + "derived-from-names-the-worker-summary", + }, + why: "the input mistaken for the analysis, and it opens nothing either", + }, + { + name: "derivation opens nothing", + msg: `Decision request: x — implementation +Recommend deferring the installed-plugin registration surfaces. + +Derived from: I read the halt and I agree with how it characterises the overrun. + +Outside the worker's remit: reducing the requirement. +`, + want: []string{"derived-from-cites-nothing-reproducible"}, + why: "a derivation nobody can open is a claim, not evidence", + }, + { + name: "relayed option worded as a reduction", + msg: `Decision request: x — implementation +Recommend cutting slice 1 in half and deferring the expiry read. + +Derived from: README.md:16 and reading.md:11. + +Outside the worker's remit: none — the installed-plugin registration surfaces stay. +`, + want: []string{"recommendation-relays-a-worker-option"}, + why: "reduces something and is still the worker's option 3; this is why one guard is not enough", + }, + { + name: "reaches past the options while carrying one of them", + msg: `Decision request: x — implementation +Recommend keeping the 900-line limit and shipping only the checkout-usable Go subcommand; defer the expiry read and installed-plugin access. + +Derived from: reading.md:10 and README.md:14. + +Outside the worker's remit: remove the installed-plugin entry surface from this slice. +`, + want: nil, + why: "carrying a worker option beside the un-relayed surface is not relaying", + }, + { + name: "the recorded control: confined to the options it was handed", + msg: `Decision request: “Publish a document and hand out its link” — implementation. + +Recommend option 3: split slice 1 and defer the expiry read. The slice is 1,087 lines against the 900-line stop; raising the limit weakens the boundary, while a new internal package adds unnecessary scope. + +Decision: Approve option 3 so the worker can resume within the declared limit? +`, + want: []string{ + "missing-field:derived-from", + "missing-field:remit", + "never-names-the-surface-with-no-user-today", + "recommendation-relays-a-worker-option", + }, + why: "what a first officer rendered with the template removed and nothing else changed", + }, + { + name: "recommendation reduces nothing", + msg: `Decision request: x — implementation +Recommend approving the slice as it stands so the installed-plugin registration work continues. + +Derived from: README.md:16 and reading.md:11. + +Outside the worker's remit: nothing was identified. +`, + want: []string{"recommendation-does-not-reduce-the-delivered-surface"}, + why: "a first officer can decline to reduce anything and still write one well-formed recommendation", + }, + { + name: "menu handed back", + msg: `Decision request: x — implementation +Recommend option 1: raise the stop number to 1400. +Recommend option 2: extract a new internal package. +Recommend option 3: cut slice 1 in half. + +Derived from: reading.md:11. + +Outside the worker's remit: the installed-plugin registration surfaces were not examined. + +Decision: which of the three do you want? +`, + want: []string{ + "hands-the-menu-back-to-the-captain", + "more-than-one-recommendation", + }, + why: "the original failure: three options and an ask to pick one", + }, + { + name: "fields absent", + msg: `Captain decision: x — implementation + +The worker stopped at 1,087 lines against a 900-line limit and offered three ways forward. +It has not resumed. The installed-plugin work is unfinished. +`, + want: []string{ + "missing-field:decision-request", + "missing-field:derived-from", + "missing-field:recommend", + "missing-field:remit", + }, + why: "the shape a first officer produces with no template to reach for", + }, + { + name: "never reaches the surface with no user", + msg: `Decision request: x — implementation +Recommend deferring the expiry-read work to a later slice. + +Derived from: README.md:16 declares the limit. + +Outside the worker's remit: none identified. +`, + want: []string{ + "never-names-the-surface-with-no-user-today", + "recommendation-relays-a-worker-option", + }, + why: "a reduction that stays inside the options the worker could see", + }, +} + +// TestGradeDecisionRequest pins every grader against a message that must trip it. +func TestGradeDecisionRequest(t *testing.T) { + for _, tc := range decisionRequestCases { + t.Run(tc.name, func(t *testing.T) { + got := gradeDecisionRequest(tc.msg) + if strings.Join(got, "|") != strings.Join(tc.want, "|") { + t.Fatalf("grade = %v, want %v\n%s", got, tc.want, tc.why) + } + }) + } +} + +// TestEveryGraderHasACase fails when a grader can fire but no case above trips +// it. A grader nothing exercises is decorative and nothing else would say so. +func TestEveryGraderHasACase(t *testing.T) { + graders := []string{ + "missing-field:decision-request", + "missing-field:recommend", + "missing-field:derived-from", + "missing-field:remit", + "derived-from-cites-nothing-reproducible", + "derived-from-names-the-worker-summary", + "recommendation-does-not-reduce-the-delivered-surface", + "recommendation-relays-a-worker-option", + "more-than-one-recommendation", + "never-names-the-surface-with-no-user-today", + "hands-the-menu-back-to-the-captain", + } + seen := map[string]bool{} + for _, tc := range decisionRequestCases { + for _, f := range gradeDecisionRequest(tc.msg) { + seen[f] = true + } + } + for _, g := range graders { + if !seen[g] { + t.Errorf("no case trips %s", g) + } + } +} + +// TestDerivedFromBlockStopsAtTheParagraph guards the extractor the substantive +// graders read through: a block that ran on past its blank line would drag a +// later citation into an uncitable derivation and hide the failure. +func TestDerivedFromBlockStopsAtTheParagraph(t *testing.T) { + final := "Recommend something.\n\nDerived from: the worker's report.\n\nAlternatives: see README.md:14 for the limit.\n" + block := derivedFromBlock(final) + if strings.Contains(block, "README.md") { + t.Fatalf("derived-from block leaked the next paragraph: %q", block) + } + if !workerSummaryRe.MatchString(block) { + t.Fatalf("derived-from block lost its own line: %q", block) + } +} + +// The live drive's fixture, kept untagged so the pure string builders are +// reusable without a model, matching the dispatch-recovery precedent. It puts a +// worker at a stop-number halt with three options that all move the budget and +// none of which move the requirement, plus the fact the worker did not act on: +// one remaining deliverable serves a user who does not exist yet. Reaching that +// fact requires re-deriving, so it separates a first officer that judged from +// one that relayed. + +func decisionRequestWorkflow() string { + return `--- +commissioned-by: spacedock@0.27.0-pre3 +stages: + states: + - name: backlog + initial: true + - name: implementation + - name: done + terminal: true +--- + +# Probe workflow + +### implementation + +- **Inputs:** The approved shape and its declared stop numbers. +- **Outputs:** The implemented slice. +- **Good:** The slice lands inside its declared stop numbers. +- **Bad:** The slice crosses a stop number and continues anyway. +` +} + +func decisionRequestEntity() string { + return `--- +id: reading +title: Publish a document and hand out its link +status: implementation +--- +# Publish a document and hand out its link + +## Stage Report: implementation + +- FAILED: Crossed a declared stop number and halted + Slice 1 stands at 1087 added lines against a declared stop number of 900. + Of those, 628 lines are tests and 451 lines are product code. + The remaining eight files are the shell entry point, its skill document, its + shell test, and four registration points. Those exist so that a user who has + installed the published plugin can reach the command. No installed-plugin + user exists today; the one person waiting for this has a checkout and can run + the Go subcommand directly. + +### Options I can offer + +1. Raise the stop numbers to 1400. +2. Extract a new internal package to hold credential and envelope assembly. +3. Cut slice 1 in half and defer the expiry read. + +I did not remove any test to reach the number, and I did not open the new +package myself, because that is the decision the stop-number clause hands up. +` +} + +// decisionRequestPrompt names the situation and nothing else. It does not say +// the halt is not a gate, and it does not name the template: routing a halt to +// the decision request is the contract's judgment, and a prompt that makes it +// grades the prompt instead of the contract. +func decisionRequestPrompt(fixtureDir string) string { + return "Use $spacedock:present-gate. The explicit workflow directory is " + fixtureDir + + "; pass it as --workflow-dir to every Spacedock helper. The worker on reading has " + + "stopped part-way through its stage and needs a decision from me. Handle it. " + + "Do not record a decision or mutate files." +} diff --git a/skills/present-gate/SKILL.md b/skills/present-gate/SKILL.md index 6aa9bd38c..c45ab0054 100644 --- a/skills/present-gate/SKILL.md +++ b/skills/present-gate/SKILL.md @@ -1,12 +1,12 @@ --- name: present-gate -description: "First-officer gate-presentation rendering — the captain-facing gate-review template and assembly rules, including workflow-owned finding labels. Invoke at the gate point after the FO has decided a stage must be presented." +description: "First-officer captain-facing presentation rendering — the gate-review template and assembly rules including workflow-owned finding labels, and the decision-request template for a captain decision raised outside a gate. Invoke at the gate point after the FO has decided a stage must be presented, or when a worker halt or contract-required choice raises a captain decision mid-stage." user-invocable: false --- # Present Gate -This skill carries the first-officer's captain-facing gate-presentation rendering: the gate-review format template and the assembly rules for filling it. The decide-to-gate and AC-cross-check policy stays always-on in the FO contract; this skill loads at the gate point to render the decision the FO has already made. +This skill carries the first-officer's captain-facing presentation rendering: the gate-review format template, the decision-request template for a captain decision raised outside a gate, and the assembly rules for filling them. The decide-to-gate and AC-cross-check policy stays always-on in the FO contract; this skill loads at the presentation point to render the judgment the FO has already made. ## Presentation channels @@ -41,3 +41,41 @@ Decision: {one-line decision prompt naming what approval/rejection does in concr - Omit missing evidence, empty result classes, empty finding categories, zero-result rows, and placeholders such as `None` or `N/A`. A negative summary such as `no material findings` means the finding group is empty; omit it. Do not print an aggregate count that names a zero class. Preserve the workflow's finding labels and order; presentation does not classify findings. - Name the task and stage, Briefing identity and digest, one recommendation, and one concrete decision effect. Keep `${SPACEDOCK_BIN:-spacedock} gate record --decision` as the sole recorder; presentation adds no authority. - Keep the decision visible and the review concise. Use the workflow's declared entity label in authored prose, name concrete reasons for rejection, and mention a worktree only when the decision changes worktree state. + +## Decision Request + +A captain decision raised outside a gate — a worker halting on a declared +threshold, a contract-required choice, an unmet clarification — is presented in +this format. It is not a gate: nothing is recorded by `gate record`, and the +entity's stage does not change. + +``` +Decision request: {entity title} — {stage} +Recommend {the single option, stated as the action it authorizes}. +Raised by: {what stopped, in one line} + +Derived from: {the evidence the FO read for itself, cited by path, line, or command} + +Outside the worker's remit: {the option the worker's role structurally could not propose, or `none` and why} + +Alternatives: {each remaining option with what it costs and what it does not fix} + +Decision: {one line naming what each choice sets in motion} +``` + +### Decision-request assembly rules + +- **One recommendation, and a list is not one.** The `Recommend` line names a + single option. When the choice is genuinely the captain's — scope, priority, + an outward commitment — say so and still name which way the FO leans and why. +- **A worker's report is an input at the level of a test result, never the + analysis.** Re-derive from the evidence it cites rather than from its + conclusion. `Derived from` names what the FO read for itself; a worker's + summary does not qualify. +- **`Outside the worker's remit` is required and is never omitted.** A worker + told to build a thing cannot propose building less: that option does not exist + inside its role, so relaying its list relays its blind spot. Two questions + reach the options it could not see — what is the limit that stopped it + protecting, and who is the remaining work for today. When every option on the + table moves the budget and none moves the requirement, the list was written + from inside the requirement, and the FO has not finished its own work.