diff --git a/.gitignore b/.gitignore index 93d633d..3524088 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,4 @@ coverage.out .unbound-force/ .muti-mind/ .mx-f/ +.uf/feedback/ diff --git a/internal/agentkit/agentkit_test.go b/internal/agentkit/agentkit_test.go index 9784bf7..164c8dd 100644 --- a/internal/agentkit/agentkit_test.go +++ b/internal/agentkit/agentkit_test.go @@ -491,6 +491,313 @@ func TestWorkerPrompt_HardenedStructure(t *testing.T) { } } +func TestForgeMD_StructuralHardening(t *testing.T) { + // Read forge.md from embedded content. + data, err := content.ReadFile("content/commands/forge.md") + if err != nil { + t.Fatalf("read forge.md: %v", err) + } + text := string(data) + lines := strings.Split(text, "\n") + + // Helper: find the line index of a heading (e.g., "## Critical Invariants"). + findHeading := func(heading string) int { + for i, line := range lines { + if strings.TrimSpace(line) == heading { + return i + } + } + return -1 + } + + // Helper: extract the section between a heading and the next same-level heading. + sectionContent := func(heading string) string { + start := findHeading(heading) + if start < 0 { + return "" + } + level := 0 + for _, ch := range heading { + if ch == '#' { + level++ + } else { + break + } + } + var sb strings.Builder + for i := start + 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if strings.HasPrefix(trimmed, strings.Repeat("#", level)+" ") && !strings.HasPrefix(trimmed, strings.Repeat("#", level+1)) { + break + } + sb.WriteString(lines[i]) + sb.WriteString("\n") + } + return sb.String() + } + + // Scenario 1: Critical Invariants section appears before Workflow section. + t.Run("InvariantsBeforeWorkflow", func(t *testing.T) { + invIdx := findHeading("## Critical Invariants") + wfIdx := findHeading("## Workflow") + if invIdx < 0 { + t.Fatal("Critical Invariants section not found") + } + if wfIdx < 0 { + t.Fatal("Workflow section not found") + } + if invIdx >= wfIdx { + t.Errorf("Critical Invariants (line %d) must appear before Workflow (line %d)", invIdx, wfIdx) + } + }) + + // Scenario 2: Review-before-complete invariant is present in Critical Invariants. + t.Run("ReviewBeforeCompleteInvariant", func(t *testing.T) { + section := sectionContent("## Critical Invariants") + if section == "" { + t.Fatal("Critical Invariants section not found") + } + lower := strings.ToLower(section) + if !strings.Contains(lower, "review") || !strings.Contains(lower, "before") { + t.Error("Critical Invariants must contain review-before-complete constraint") + } + if !strings.Contains(section, "MUST") { + t.Error("Critical Invariants must use RFC 2119 MUST language for review constraint") + } + }) + + // Scenario 3: Review gate mandatory constraint is present in Critical Invariants. + t.Run("ReviewGateMandatory", func(t *testing.T) { + section := sectionContent("## Critical Invariants") + if section == "" { + t.Fatal("Critical Invariants section not found") + } + // The review gate must be stated as a positive constraint (not naming bypass parameters). + if !strings.Contains(section, "MUST NOT") { + t.Error("Critical Invariants must use MUST NOT for review gate constraint") + } + if !strings.Contains(section, "NEVER") { + t.Error("Critical Invariants must use NEVER for review gate constraint") + } + lower := strings.ToLower(section) + if !strings.Contains(lower, "review gate") { + t.Error("Critical Invariants must reference 'review gate' as mandatory") + } + }) + + // Scenario 4: Review-before-complete constraint has redundant placement + // (present in ALL THREE sections: Critical Invariants, Workflow, and Rules). + t.Run("RedundantReviewConstraint", func(t *testing.T) { + invariants := sectionContent("## Critical Invariants") + workflow := sectionContent("## Workflow") + rules := sectionContent("## Rules") + + inInvariants := strings.Contains(strings.ToLower(invariants), "review") && + strings.Contains(strings.ToLower(invariants), "before") + inWorkflow := strings.Contains(strings.ToLower(workflow), "review") && + strings.Contains(strings.ToLower(workflow), "before") + inRules := strings.Contains(strings.ToLower(rules), "review") && + strings.Contains(strings.ToLower(rules), "before") && + strings.Contains(strings.ToLower(rules), "complete") + + if !inInvariants { + t.Error("review-before-complete not found in Critical Invariants") + } + if !inWorkflow { + t.Error("review-before-complete not found in Workflow section") + } + if !inRules { + t.Error("review-before-complete not found in Rules section") + } + }) + + // Scenario 5: Step 7 text includes explicit ordering constraint. + t.Run("Step7OrderingConstraint", func(t *testing.T) { + workflow := sectionContent("## Workflow") + if workflow == "" { + t.Fatal("Workflow section not found") + } + // Find step 7 line — require MUST AND an ordering signal. + var foundStep7 bool + for _, line := range strings.Split(workflow, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "7.") { + foundStep7 = true + lower := strings.ToLower(line) + hasMust := strings.Contains(line, "MUST") + hasOrdering := strings.Contains(lower, "first") || strings.Contains(lower, "before step 8") + if !hasMust { + t.Error("Step 7 must use RFC 2119 MUST language") + } + if !hasOrdering { + t.Error("Step 7 must contain ordering signal (FIRST or 'before step 8')") + } + break + } + } + if !foundStep7 { + t.Error("Step 7 not found in Workflow section") + } + }) + + // Scenario 6: Review rule is first item in Rules section. + t.Run("ReviewRuleFirstInRules", func(t *testing.T) { + rules := sectionContent("## Rules") + if rules == "" { + t.Fatal("Rules section not found") + } + // Find first bullet in Rules section. + for _, line := range strings.Split(rules, "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "- ") { + lower := strings.ToLower(trimmed) + if !strings.Contains(lower, "review") { + t.Errorf("first Rules bullet must be about review, got: %s", trimmed) + } + break + } + } + }) + + // Scenario 7: No standalone Strategy Selection, Error Recovery, or Completion sections. + t.Run("NoStandaloneSections", func(t *testing.T) { + prohibited := []string{ + "## Strategy Selection", + "## Error Recovery", + "## Completion", + } + for _, heading := range prohibited { + if findHeading(heading) >= 0 { + t.Errorf("found prohibited standalone section: %s", heading) + } + } + }) + + // Scenario 8: Strategy selection content is inlined within step 3 (Decompose). + t.Run("StrategyInlinedInStep3", func(t *testing.T) { + workflow := sectionContent("## Workflow") + if workflow == "" { + t.Fatal("Workflow section not found") + } + // Find step 3 and its sub-items (lines between "3." and the next step "4."). + wfLines := strings.Split(workflow, "\n") + var step3Content strings.Builder + inStep3 := false + for _, line := range wfLines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "3.") { + inStep3 = true + } else if inStep3 && len(trimmed) > 0 && trimmed[0] >= '1' && trimmed[0] <= '9' && len(trimmed) > 1 && trimmed[1] == '.' { + break + } + if inStep3 { + step3Content.WriteString(line) + step3Content.WriteString("\n") + } + } + s3 := step3Content.String() + if !strings.Contains(s3, "forge_get_strategy_insights") { + t.Error("Step 3 must contain forge_get_strategy_insights (strategy selection inlined)") + } + if !strings.Contains(s3, "forge_decompose") { + t.Error("Step 3 must contain forge_decompose") + } + }) + + // Scenario 9: Error recovery content is inlined within step 6 (Monitor). + t.Run("ErrorRecoveryInlinedInStep6", func(t *testing.T) { + workflow := sectionContent("## Workflow") + if workflow == "" { + t.Fatal("Workflow section not found") + } + wfLines := strings.Split(workflow, "\n") + var step6Content strings.Builder + inStep6 := false + for _, line := range wfLines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "6.") { + inStep6 = true + } else if inStep6 && len(trimmed) > 0 && trimmed[0] >= '1' && trimmed[0] <= '9' && len(trimmed) > 1 && trimmed[1] == '.' { + break + } + if inStep6 { + step6Content.WriteString(line) + step6Content.WriteString("\n") + } + } + s6 := step6Content.String() + lower := strings.ToLower(s6) + if !strings.Contains(lower, "blocked") { + t.Error("Step 6 must contain blocked-worker recovery guidance") + } + if !strings.Contains(lower, "unblock") && !strings.Contains(lower, "reassign") { + t.Error("Step 6 must contain recovery action (unblock or reassign)") + } + }) + + // Scenario 10: Completion sub-steps are inlined within step 8. + t.Run("CompletionInlinedInStep8", func(t *testing.T) { + workflow := sectionContent("## Workflow") + if workflow == "" { + t.Fatal("Workflow section not found") + } + wfLines := strings.Split(workflow, "\n") + var step8Content strings.Builder + inStep8 := false + for _, line := range wfLines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "8.") { + inStep8 = true + } else if inStep8 && len(trimmed) > 0 && trimmed[0] >= '1' && trimmed[0] <= '9' && len(trimmed) > 1 && trimmed[1] == '.' { + break + } + if inStep8 { + step8Content.WriteString(line) + step8Content.WriteString("\n") + } + } + s8 := step8Content.String() + required := []string{ + "forge_complete", + "forge_record_outcome", + "hivemind_store", + "org_sync", + } + for _, tool := range required { + if !strings.Contains(s8, tool) { + t.Errorf("Step 8 must contain %s (completion sub-step)", tool) + } + } + }) + + // Scenario 11: All MCP tool references from the forge workflow are present. + t.Run("AllToolReferencesPresent", func(t *testing.T) { + allTools := []string{ + "comms_init", + "hivemind_find", + "forge_decompose", + "forge_get_strategy_insights", + "org_create_epic", + "forge_spawn_subtask", + "comms_inbox", + "forge_status", + "org_cells", + "comms_read_message", + "comms_ack", + "forge_review", + "forge_complete", + "forge_record_outcome", + "hivemind_store", + "org_sync", + "comms_reserve", + } + for _, tool := range allTools { + if !strings.Contains(text, tool) { + t.Errorf("forge.md missing MCP tool reference: %s", tool) + } + } + }) +} + func TestSkillTemplates_HaveNameField(t *testing.T) { // Walk the embedded content filesystem and verify every SKILL.md // has a "name: " field in its YAML frontmatter. diff --git a/internal/agentkit/content/commands/forge.md b/internal/agentkit/content/commands/forge.md index a786f04..727958a 100644 --- a/internal/agentkit/content/commands/forge.md +++ b/internal/agentkit/content/commands/forge.md @@ -6,6 +6,15 @@ description: Decompose task into subtasks and coordinate parallel agents Decompose a task and spawn parallel workers. +## Critical Invariants + +These rules are non-negotiable and MUST NOT be skipped: + +- **Review MUST complete before marking work done** — step 7 (review) MUST finish before step 8 (complete). NEVER skip review. +- **The review gate is mandatory** — `forge_complete` MUST NOT be called until `forge_review` has passed for every worker. NEVER bypass the review gate. +- ALWAYS create a forge, even for small tasks. +- Coordinator orchestrates, workers execute — workers MUST NOT call `forge_complete`. + ## Task $ARGUMENTS @@ -15,53 +24,27 @@ $ARGUMENTS 1. Initialize comms: `comms_init(project_path=".", task_description="Forge: ")` 2. Check prior learnings: `hivemind_find(query="")` 3. Decompose: `forge_decompose(task="", context="")` + - Before decomposing, check historical success rates: `forge_get_strategy_insights(task="")` + - Choose from: `file-based`, `feature-based`, `risk-based`, or `auto` 4. Create epic: `org_create_epic(epic_title="", subtasks=[...])` 5. For each subtask: `forge_spawn_subtask(bead_id, epic_id, subtask_title, files)` 6. Monitor: check `comms_inbox()` every few minutes -7. Review: `forge_review(task_id, files_touched)` for each completed worker -8. Complete: `forge_complete(bead_id, summary, files_touched)` -9. Store learnings: `hivemind_store(information="...", tags="forge,")` + - `comms_inbox()` — check for messages from workers + - `forge_status(epic_id, project_key)` — check worker progress + - `org_cells(status="in_progress")` — see active cells + - If a worker is blocked: read the message with `comms_read_message(message_id)`, acknowledge with `comms_ack(message_id)`, then either unblock or reassign the subtask +7. Review FIRST (before complete): `forge_review(task_id, files_touched)` for each completed worker — MUST finish before step 8 +8. Complete (after ALL reviews pass): + - `forge_complete(bead_id, summary, files_touched)` — mark epic done + - `forge_record_outcome(bead_id, duration_ms, success)` — record for learning + - `hivemind_store(information="...", tags="forge,")` — store learnings + - `org_sync()` — persist state to git ## Rules -- Always create a forge, even for small tasks -- Coordinator orchestrates, workers execute +- Review every worker's output before marking complete — NEVER skip this step +- ALWAYS create a forge, even for small tasks +- Coordinator orchestrates, workers execute — workers MUST NOT call `forge_complete` - Workers reserve their own files via `comms_reserve` - Check inbox regularly for blocked workers -- Review every worker's output before marking complete - Store learnings after completion - -## Strategy Selection - -Before decomposing, check historical success rates: - -``` -forge_get_strategy_insights(task="") -``` - -Choose from: `file-based`, `feature-based`, `risk-based`, or `auto`. - -## Monitoring - -While workers are active: - -1. `comms_inbox()` — check for messages from workers -2. `forge_status(epic_id, project_key)` — check worker progress -3. `org_cells(status="in_progress")` — see active cells - -## Completion - -After all workers finish: - -1. `forge_complete(bead_id, summary, files_touched)` — mark epic done -2. `forge_record_outcome(bead_id, duration_ms, success)` — record for learning -3. `hivemind_store(information="...", tags="forge,")` — store learnings -4. `org_sync()` — persist state to git - -## Error Recovery - -If a worker is blocked: - -1. Read the worker's message: `comms_read_message(message_id)` -2. Acknowledge: `comms_ack(message_id)` -3. Either unblock or reassign the subtask diff --git a/openspec/changes/forge-dcp-hardening/.openspec.yaml b/openspec/changes/forge-dcp-hardening/.openspec.yaml new file mode 100644 index 0000000..63fe932 --- /dev/null +++ b/openspec/changes/forge-dcp-hardening/.openspec.yaml @@ -0,0 +1,2 @@ +schema: unbound-force +created: 2026-08-02 diff --git a/openspec/changes/forge-dcp-hardening/design.md b/openspec/changes/forge-dcp-hardening/design.md new file mode 100644 index 0000000..d102d3e --- /dev/null +++ b/openspec/changes/forge-dcp-hardening/design.md @@ -0,0 +1,98 @@ +## Context + +The `/forge` command prompt at `internal/agentkit/content/commands/forge.md` is a 67-line markdown file embedded in the replicator binary. It instructs the coordinator agent to decompose tasks, spawn workers, review their output, and mark work complete. The file currently structures its content as: workflow steps, rules, strategy selection, monitoring, completion, error recovery -- in that order. + +Under DCP (Dynamic Context Protocol) context compression, long prompts get summarized. Research on prompt compression behavior shows: + +- **Opening content** is preserved with highest fidelity +- **Numbered sequences** retain better than bullet lists, but adjacent steps with similar semantics get merged +- **Middle items** in lists are most likely to be dropped +- **Trailing sections** (especially "error handling" or "edge cases") are first to be entirely removed +- **Explicit constraint language** (MUST, NEVER, FIRST) survives better than implicit ordering + +The current forge.md has all four fragilities identified in issue #47. + +## Goals / Non-Goals + +### Goals + +- Restructure forge.md so the review-before-complete ordering constraint survives context compression +- Move critical invariants to the top of the file where compressors preserve them +- Inline error recovery and strategy selection at their point of use rather than as droppable trailing sections +- Preserve all existing workflow semantics -- same steps, same tools, same order + +### Non-Goals + +- Changing the forge workflow behavior or adding new steps +- Adding new MCP tool calls or modifying tool signatures +- Restructuring other command prompts (those are separate changes) +- Implementing programmatic enforcement of review-before-complete (that would be a code change, not a prompt change) + +## Decisions + +### D1: Critical Invariants section at top of file + +Place a "Critical Invariants" section immediately after the title and before the Workflow section. This section states the non-negotiable rules in explicit constraint language. Compressors prioritize opening content, so these rules have the highest survival rate. + +Content: The review-before-complete ordering, the "always create a forge" rule, the coordinator-orchestrates/workers-execute boundary, and the prohibition on `skip_review: true` in `forge_complete` calls. + +### D2: Inline ordering constraint in step text + +Change step 7 from: +``` +7. Review: forge_review(task_id, files_touched) for each completed worker +``` +To: +``` +7. Review FIRST (before complete): forge_review(task_id, files_touched) for each completed worker — MUST finish before step 8 +``` + +This embeds the ordering constraint directly in the step text so it cannot be separated from the step by compression. + +### D3: Reorder Rules section by survival priority + +Move "Review every worker's output before marking complete" from 5th bullet (lowest survival position) to 1st bullet. First and last items in lists survive compression best. + +### D4: Inline strategy selection with decompose step + +Instead of a separate "Strategy Selection" section (lines 34-42) that can be dropped entirely, inline the `forge_get_strategy_insights` call as a sub-step of step 3 (Decompose). This ensures strategy selection is never separated from decomposition. + +### D5: Inline error recovery with monitoring step + +Instead of a trailing "Error Recovery" section (lines 61-67), inline recovery guidance as sub-items of step 6 (Monitor). Trailing sections are first to be dropped; inlined content at the point of use survives with the parent step. + +### D6: Consolidate Completion section into step 8 + +The separate "Completion" section (lines 52-59) repeats step 8 with more detail. Merge its content into step 8's text to eliminate the redundancy that compressors exploit (they drop the "duplicate" section). + +## Risks / Trade-offs + +### Risk: Longer step descriptions reduce readability + +Inlining strategy selection, error recovery, and completion details into workflow steps makes each step longer. This trades readability for compression resilience. + +**Mitigation**: Use sub-items (indented bullets) under steps rather than paragraph text. This preserves scanability while keeping content co-located. + +### Risk: Over-engineering for a theoretical problem + +DCP compression behavior is based on observed patterns, not guaranteed specifications. The fragilities may never manifest in practice. + +**Mitigation**: The restructuring preserves all content and semantics. Even without compression, the new structure is arguably better organized (invariants first, related content co-located). No downside risk. + +### Risk: `forge_complete` API has `skip_review` parameter + +The `forge_complete` MCP tool accepts `skip_review: true` and `skip_verification: true` parameters (see `internal/tools/forge/tools.go`). Under DCP compression, a compressed agent could discover or hallucinate this parameter and bypass the review gate entirely, regardless of prompt hardening. + +**Mitigation**: Add "NEVER pass `skip_review: true` to `forge_complete`" to the Critical Invariants section. This is a prompt-level defense; programmatic enforcement (removing or guarding the parameter) is out of scope for this change but should be tracked as a follow-up. The redundancy of the constraint (invariants + step 7 text + first rule) means even partial compression still leaves the prohibition visible. + +**Follow-up**: Consider removing or access-gating the `skip_review` parameter in a separate change. + +### Risk: Worker prompts also contain completion instructions + +The `forge_spawn_subtask` tool generates worker prompts that include "Complete with `forge_complete` when done" — with no mention of the review gate. If a worker calls `forge_complete` directly, it bypasses the coordinator's review step. + +**Mitigation**: The Critical Invariants section should state "Workers MUST NOT call `forge_complete` — only the coordinator completes work after review." This is a follow-up hardening target for the worker prompt (`internal/forge/spawn.go`), tracked separately from this change. + +### Trade-off: File grows slightly from explicit constraint language + +Adding "MUST", "FIRST", "before step 8" makes the file marginally longer. This is acceptable because explicit constraint language has higher compression survival than implicit ordering. diff --git a/openspec/changes/forge-dcp-hardening/proposal.md b/openspec/changes/forge-dcp-hardening/proposal.md new file mode 100644 index 0000000..8673e97 --- /dev/null +++ b/openspec/changes/forge-dcp-hardening/proposal.md @@ -0,0 +1,66 @@ +## Why + +The `/forge` command prompt (`internal/agentkit/content/commands/forge.md`) defines a 9-step workflow where Step 7 (review) must happen before Step 8 (complete). Under DCP context compression, this ordering dependency is the most likely constraint to be lost -- the two steps get compressed into a single "review and complete workers" action, allowing the agent to skip reviews or complete before reviewing. + +This is a quality gate vulnerability. The review step is the only point where the coordinator validates worker output before marking it done. If skipped, broken or incomplete work gets marked as complete with no human-visible signal that review was bypassed. + +Related: [unbound-force/replicator#47](https://github.com/unbound-force/replicator/issues/47), [unbound-force/unbound-force#346](https://github.com/unbound-force/unbound-force/issues/346). + +## What Changes + +Restructure `forge.md` to survive DCP context compression by applying prompt hardening techniques: + +1. Add a "Critical Invariants" section at the top of the file (before Workflow) stating non-negotiable rules -- compressors prioritize opening content +2. Embed ordering constraints directly in step text: "7. Review FIRST: ... -- MUST complete before step 8" +3. Move "Review every worker before marking complete" from 5th bullet to 1st position in Rules +4. Inline error recovery guidance at the monitoring step rather than as a separate droppable section at the end +5. Add strategy selection reminder inline with decompose step + +## Capabilities + +### New Capabilities + +- Review gate mandatory constraint added to Critical Invariants — states the review gate is non-negotiable using positive constraint language (avoids naming bypass parameters) + +### Modified Capabilities + +- `/forge` command: Same workflow behavior, restructured prompt for compression resilience + +### Removed Capabilities + +- None + +## Impact + +- **File**: `internal/agentkit/content/commands/forge.md` (single file change) +- **Behavioral**: No change to forge workflow semantics -- agents follow the same steps in the same order +- **Risk**: Low -- restructuring prompt text only, no code changes +- **Testing**: Existing parity tests continue to pass. `TestForgeMD_StructuralHardening` verifies structural invariants (section ordering, redundant constraint placement, prohibited standalone sections) via 7 subtests against the embedded forge.md content. + +## Constitution Alignment + +Assessed against the Replicator constitution (`.specify/memory/constitution.md`), which extends the Unbound Force org constitution v1.1.0. + +### I. Autonomous Collaboration + +**Assessment**: PASS + +This change improves the reliability of artifact-based coordination. The forge command orchestrates autonomous workers through well-defined tool calls. Hardening the prompt ensures the review gate -- which validates worker artifacts before marking them complete -- survives context compression. Self-describing outputs are unaffected. + +### II. Composability First + +**Assessment**: N/A + +No new dependencies introduced. The forge command continues to work standalone. This is a prompt text restructuring within an existing embedded file. + +### III. Observable Quality + +**Assessment**: PASS + +The change strengthens observable quality by ensuring the review step (which validates machine-parseable worker output) is not skipped under compression. No changes to tool response shapes or JSON output. + +### IV. Testability + +**Assessment**: PASS + +`TestForgeMD_StructuralHardening` (7 subtests) validates structural invariants of the embedded forge.md content: section ordering, constraint presence, redundant placement, and absence of prohibited standalone sections. Tests use in-memory embedded filesystem — no external services required. diff --git a/openspec/changes/forge-dcp-hardening/specs/forge-prompt-structure.md b/openspec/changes/forge-dcp-hardening/specs/forge-prompt-structure.md new file mode 100644 index 0000000..13b7ea8 --- /dev/null +++ b/openspec/changes/forge-dcp-hardening/specs/forge-prompt-structure.md @@ -0,0 +1,108 @@ +## ADDED Requirements + +### Requirement: Critical Invariants Section + +The forge command prompt MUST include a "Critical Invariants" section positioned immediately after the title and before the Workflow section. This section MUST state the non-negotiable ordering constraints and behavioral rules in explicit RFC 2119 language. + +#### Scenario: Invariants appear before workflow + +- **GIVEN** the forge.md file is loaded by an agent +- **WHEN** the content is parsed or compressed +- **THEN** the Critical Invariants section MUST appear before the Workflow section in document order + +#### Scenario: Review-before-complete invariant present + +- **GIVEN** the Critical Invariants section exists +- **WHEN** its content is read +- **THEN** it MUST contain a statement that review (step 7) MUST complete before marking work done (step 8) + +#### Scenario: skip_review prohibition present + +- **GIVEN** the Critical Invariants section exists +- **WHEN** its content is read +- **THEN** it MUST contain a statement that `skip_review: true` MUST NEVER be passed to `forge_complete` + +#### Scenario: Review-before-complete constraint has redundant placement + +- **GIVEN** the forge.md file +- **WHEN** any single section is removed entirely +- **THEN** the review-before-complete constraint MUST still be present in at least one other location + +## MODIFIED Requirements + +### Requirement: Workflow step 7 ordering constraint + +Step 7 (Review) MUST embed an explicit ordering constraint in its text indicating it MUST be completed before step 8 (Complete). The constraint MUST use explicit language ("MUST", "FIRST", "before step 8") rather than relying on sequential numbering alone. + +Previously: "7. Review: `forge_review(task_id, files_touched)` for each completed worker" + +#### Scenario: Step 7 text includes ordering constraint + +- **GIVEN** the Workflow section of forge.md +- **WHEN** step 7 text is read +- **THEN** it MUST contain explicit language indicating it must be completed before step 8 + +### Requirement: Rules section ordering + +The rule "Review every worker's output before marking complete" MUST be the first item in the Rules section. Rules SHOULD be ordered by criticality (most critical first). + +Previously: "Review every worker's output before marking complete" appeared as the 5th of 6 bullets. + +#### Scenario: Review rule is first in list + +- **GIVEN** the Rules section of forge.md +- **WHEN** the bullet items are read in order +- **THEN** the review-before-complete rule MUST be the first item + +### Requirement: Strategy selection inlined with decompose + +Strategy selection guidance (including `forge_get_strategy_insights`) MUST be inlined as a sub-item of the decompose step rather than appearing as a separate trailing section. + +Previously: Strategy Selection was a standalone section at lines 34-42. + +#### Scenario: Strategy selection is part of decompose step + +- **GIVEN** the Workflow section of forge.md +- **WHEN** the decompose step is read +- **THEN** it MUST include strategy selection guidance as a sub-item +- **AND** there MUST NOT be a separate "Strategy Selection" section + +### Requirement: Error recovery inlined with monitoring + +Error recovery guidance MUST be inlined as sub-items of the monitoring step rather than appearing as a separate trailing section. + +Previously: Error Recovery was a standalone section at lines 61-67. + +#### Scenario: Error recovery is part of monitoring step + +- **GIVEN** the Workflow section of forge.md +- **WHEN** the monitoring step is read +- **THEN** it MUST include error recovery guidance as sub-items +- **AND** there MUST NOT be a separate "Error Recovery" section at the end of the file + +### Requirement: Completion details inlined with complete step + +Completion sub-steps (`forge_complete`, `forge_record_outcome`, `hivemind_store`, `org_sync`) MUST be inlined as sub-items of step 8 (Complete) rather than appearing as a separate "Completion" section. + +Previously: Completion was a standalone section at lines 52-59 repeating step 8 with more detail. + +#### Scenario: Completion details are part of step 8 + +- **GIVEN** the Workflow section of forge.md +- **WHEN** step 8 is read +- **THEN** it MUST include completion sub-steps as sub-items +- **AND** there MUST NOT be a separate "Completion" section + +## REMOVED Requirements + +### Requirement: Standalone Strategy Selection section + +Removed as a standalone section. Content is preserved but moved inline with the decompose step (see MODIFIED: Strategy selection inlined with decompose). + +### Requirement: Standalone Error Recovery section + +Removed as a standalone section. Content is preserved but moved inline with the monitoring step (see MODIFIED: Error recovery inlined with monitoring). + +### Requirement: Standalone Completion section + +Removed as a standalone section. Content is preserved but moved inline with step 8 (see MODIFIED: Completion details inlined with complete step). diff --git a/openspec/changes/forge-dcp-hardening/tasks.md b/openspec/changes/forge-dcp-hardening/tasks.md new file mode 100644 index 0000000..fe57d63 --- /dev/null +++ b/openspec/changes/forge-dcp-hardening/tasks.md @@ -0,0 +1,33 @@ + + +## 1. Restructure forge.md for DCP compression resilience + +- [x] 1.1 Add Critical Invariants section after title, before Workflow. Include: (a) review MUST complete before marking work done, (b) NEVER pass `skip_review: true` to `forge_complete`, (c) always create a forge even for small tasks, (d) coordinator orchestrates, workers execute +- [x] 1.2 Rewrite step 7 text to embed explicit ordering constraint: "Review FIRST (before complete): `forge_review(task_id, files_touched)` for each completed worker — MUST finish before step 8" +- [x] 1.3 Reorder Rules section: move "Review every worker's output before marking complete" to 1st bullet position +- [x] 1.4 Inline strategy selection (`forge_get_strategy_insights`) as sub-item of step 3 (Decompose); remove standalone Strategy Selection section +- [x] 1.5 Inline error recovery guidance as sub-items of step 6 (Monitor); remove standalone Error Recovery section +- [x] 1.6 Merge Completion section content into step 8 as sub-items (`forge_complete`, `forge_record_outcome`, `hivemind_store`, `org_sync`); remove standalone Completion section + +## 2. Verification + +- [x] 2.1 Verify `make build` succeeds (forge.md is embedded content) +- [x] 2.2 Verify `make test` passes (no behavioral changes, parity tests unaffected) +- [x] 2.3 Verify all MCP tool call references from the original forge.md are present in the restructured version (grep for: `forge_review`, `forge_complete`, `forge_record_outcome`, `hivemind_store`, `org_sync`, `comms_inbox`, `forge_status`, `org_cells`, `comms_read_message`, `comms_ack`, `forge_get_strategy_insights`, `forge_decompose`, `org_create_epic`, `forge_spawn_subtask`, `comms_init`, `hivemind_find`, `comms_reserve`) +- [x] 2.4 Verify review-before-complete constraint appears in at least 3 locations (Critical Invariants section, step 7 text, first rule in Rules section) for compression redundancy + +