From 50ae279dd77e07ad61fe9d3ad51e2aa2f9c0aeea Mon Sep 17 00:00:00 2001 From: Jay Flowers Date: Sun, 2 Aug 2026 15:05:40 -0400 Subject: [PATCH 1/5] fix: harden forge command prompt against DCP context compression Restructure forge.md to prevent the review-before-complete quality gate from being lost under context compression: - Add Critical Invariants section at top of file - Embed ordering constraint directly in step 7 text - Move review rule to 1st position in Rules section - Inline strategy selection into decompose step - Inline error recovery into monitoring step - Merge Completion section into step 8 sub-items - Add skip_review prohibition to invariants All original content preserved (17 MCP tool references verified). No behavioral changes to the forge workflow. Closes #47 Assisted-by: claude-opus-4-6 Generated with AI assistance (claude-opus-4-6) --- internal/agentkit/content/commands/forge.md | 61 ++++------ .../forge-dcp-hardening/.openspec.yaml | 2 + .../changes/forge-dcp-hardening/design.md | 98 ++++++++++++++++ .../changes/forge-dcp-hardening/proposal.md | 66 +++++++++++ .../specs/forge-prompt-structure.md | 108 ++++++++++++++++++ openspec/changes/forge-dcp-hardening/tasks.md | 33 ++++++ 6 files changed, 329 insertions(+), 39 deletions(-) create mode 100644 openspec/changes/forge-dcp-hardening/.openspec.yaml create mode 100644 openspec/changes/forge-dcp-hardening/design.md create mode 100644 openspec/changes/forge-dcp-hardening/proposal.md create mode 100644 openspec/changes/forge-dcp-hardening/specs/forge-prompt-structure.md create mode 100644 openspec/changes/forge-dcp-hardening/tasks.md diff --git a/internal/agentkit/content/commands/forge.md b/internal/agentkit/content/commands/forge.md index a786f04..88f40a6 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. +- **NEVER pass `skip_review: true` to `forge_complete`** — the review gate exists to catch broken work before it is marked done. +- 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 +- 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 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..b827845 --- /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 + +- None -- this is a hardening change, not a feature addition + +### 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 (forge.md is embedded content, not tested directly). Manual verification that the restructured prompt reads correctly and maintains all original 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**: N/A + +No testable components are added or modified. The change is to embedded prompt content. Existing parity tests and build verification continue to apply. 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 + + From e0ea358121e1c80a174b432bc2d0d36722b89940 Mon Sep 17 00:00:00 2001 From: Jay Flowers Date: Fri, 7 Aug 2026 14:42:28 -0400 Subject: [PATCH 2/5] fix: address PR #55 review feedback - Fix keyword casing: Always -> ALWAYS (RFC 2119 consistency) - Add TestForgeMD_StructuralHardening with 7 subtests covering all spec scenarios from forge-prompt-structure.md - Fix intent drift in proposal.md: document skip_review prohibition as a new capability instead of claiming 'None' - File issue #63 for code-level skip_review enforcement - Add .uf/feedback/ to .gitignore --- .gitignore | 1 + internal/agentkit/agentkit_test.go | 169 ++++++++++++++++++ internal/agentkit/content/commands/forge.md | 2 +- .../changes/forge-dcp-hardening/proposal.md | 2 +- 4 files changed, 172 insertions(+), 2 deletions(-) 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..bbb4d00 100644 --- a/internal/agentkit/agentkit_test.go +++ b/internal/agentkit/agentkit_test.go @@ -491,6 +491,175 @@ 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: skip_review prohibition is present in Critical Invariants. + t.Run("SkipReviewProhibition", func(t *testing.T) { + section := sectionContent("## Critical Invariants") + if section == "" { + t.Fatal("Critical Invariants section not found") + } + if !strings.Contains(section, "skip_review") { + t.Error("Critical Invariants must contain skip_review prohibition") + } + if !strings.Contains(section, "NEVER") { + t.Error("Critical Invariants must use NEVER for skip_review prohibition") + } + }) + + // Scenario 4: Review-before-complete constraint has redundant placement + // (present in both Critical Invariants AND at least one other section). + 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 && !inRules { + t.Error("review-before-complete must appear in at least one section beyond Critical Invariants") + } + }) + + // 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. + hasStep7Constraint := false + for _, line := range strings.Split(workflow, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "7.") { + lower := strings.ToLower(line) + if strings.Contains(lower, "first") || strings.Contains(lower, "before step 8") || + strings.Contains(line, "MUST") { + hasStep7Constraint = true + } + break + } + } + if !hasStep7Constraint { + t.Error("Step 7 must contain explicit ordering constraint (FIRST/before step 8/MUST)") + } + }) + + // 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) + } + } + }) +} + 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 88f40a6..a01da46 100644 --- a/internal/agentkit/content/commands/forge.md +++ b/internal/agentkit/content/commands/forge.md @@ -12,7 +12,7 @@ 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. - **NEVER pass `skip_review: true` to `forge_complete`** — the review gate exists to catch broken work before it is marked done. -- Always create a forge, even for small tasks. +- ALWAYS create a forge, even for small tasks. - Coordinator orchestrates, workers execute — workers MUST NOT call `forge_complete`. ## Task diff --git a/openspec/changes/forge-dcp-hardening/proposal.md b/openspec/changes/forge-dcp-hardening/proposal.md index b827845..048e300 100644 --- a/openspec/changes/forge-dcp-hardening/proposal.md +++ b/openspec/changes/forge-dcp-hardening/proposal.md @@ -20,7 +20,7 @@ Restructure `forge.md` to survive DCP context compression by applying prompt har ### New Capabilities -- None -- this is a hardening change, not a feature addition +- `skip_review: true` prohibition added to Critical Invariants — prevents agents from bypassing the review gate via `forge_complete` parameter ### Modified Capabilities From 73a19c7e48258332c80d542d5b54d9504b903032 Mon Sep 17 00:00:00 2001 From: Jay Flowers Date: Mon, 10 Aug 2026 11:41:23 -0400 Subject: [PATCH 3/5] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20casing=20consistency,=20stale=20docs,=20test=20prec?= =?UTF-8?q?ision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A1: Normalize 'Always' to 'ALWAYS' in Rules section (forge.md:46) - E/A2: Update proposal.md testing section to reflect TestForgeMD_StructuralHardening - F/T1: Fix comment to match implementation ('same-level' not 'same/higher level') - G/T2: Strengthen Step7OrderingConstraint to require MUST AND ordering signal - A3: Add Closes #47 to PR body --- internal/agentkit/agentkit_test.go | 19 ++++++++++++------- internal/agentkit/content/commands/forge.md | 2 +- .../changes/forge-dcp-hardening/proposal.md | 6 +++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/internal/agentkit/agentkit_test.go b/internal/agentkit/agentkit_test.go index bbb4d00..985b8d5 100644 --- a/internal/agentkit/agentkit_test.go +++ b/internal/agentkit/agentkit_test.go @@ -609,20 +609,25 @@ func TestForgeMD_StructuralHardening(t *testing.T) { if workflow == "" { t.Fatal("Workflow section not found") } - // Find step 7 line. - hasStep7Constraint := false + // 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) - if strings.Contains(lower, "first") || strings.Contains(lower, "before step 8") || - strings.Contains(line, "MUST") { - hasStep7Constraint = true + 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 !hasStep7Constraint { - t.Error("Step 7 must contain explicit ordering constraint (FIRST/before step 8/MUST)") + if !foundStep7 { + t.Error("Step 7 not found in Workflow section") } }) diff --git a/internal/agentkit/content/commands/forge.md b/internal/agentkit/content/commands/forge.md index a01da46..4adb8d0 100644 --- a/internal/agentkit/content/commands/forge.md +++ b/internal/agentkit/content/commands/forge.md @@ -43,7 +43,7 @@ $ARGUMENTS ## Rules - Review every worker's output before marking complete — NEVER skip this step -- Always create a forge, even for small tasks +- ALWAYS create a forge, even for small tasks - Coordinator orchestrates, workers execute - Workers reserve their own files via `comms_reserve` - Check inbox regularly for blocked workers diff --git a/openspec/changes/forge-dcp-hardening/proposal.md b/openspec/changes/forge-dcp-hardening/proposal.md index 048e300..7f50c1d 100644 --- a/openspec/changes/forge-dcp-hardening/proposal.md +++ b/openspec/changes/forge-dcp-hardening/proposal.md @@ -35,7 +35,7 @@ Restructure `forge.md` to survive DCP context compression by applying prompt har - **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 (forge.md is embedded content, not tested directly). Manual verification that the restructured prompt reads correctly and maintains all original content. +- **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 @@ -61,6 +61,6 @@ The change strengthens observable quality by ensuring the review step (which val ### IV. Testability -**Assessment**: N/A +**Assessment**: PASS -No testable components are added or modified. The change is to embedded prompt content. Existing parity tests and build verification continue to apply. +`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. From 806d9fa5e7284a909325b1111df713607dab7e95 Mon Sep 17 00:00:00 2001 From: Jay Flowers Date: Mon, 10 Aug 2026 12:04:54 -0400 Subject: [PATCH 4/5] fix: address review council blocking findings in structural tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - B1: Add subtests verifying inlined content at correct steps: StrategyInlinedInStep3, ErrorRecoveryInlinedInStep6, CompletionInlinedInStep8 - B2: Add AllToolReferencesPresent subtest verifying all 17 MCP tool references are present in forge.md - B3: Fix RedundantReviewConstraint to require AND logic — all 3 locations (Critical Invariants, Workflow, Rules) must contain the review-before-complete constraint --- internal/agentkit/agentkit_test.go | 134 ++++++++++++++++++++++++++++- 1 file changed, 131 insertions(+), 3 deletions(-) diff --git a/internal/agentkit/agentkit_test.go b/internal/agentkit/agentkit_test.go index 985b8d5..373f865 100644 --- a/internal/agentkit/agentkit_test.go +++ b/internal/agentkit/agentkit_test.go @@ -581,7 +581,7 @@ func TestForgeMD_StructuralHardening(t *testing.T) { }) // Scenario 4: Review-before-complete constraint has redundant placement - // (present in both Critical Invariants AND at least one other section). + // (present in ALL THREE sections: Critical Invariants, Workflow, and Rules). t.Run("RedundantReviewConstraint", func(t *testing.T) { invariants := sectionContent("## Critical Invariants") workflow := sectionContent("## Workflow") @@ -598,8 +598,11 @@ func TestForgeMD_StructuralHardening(t *testing.T) { if !inInvariants { t.Error("review-before-complete not found in Critical Invariants") } - if !inWorkflow && !inRules { - t.Error("review-before-complete must appear in at least one section beyond 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") } }) @@ -663,6 +666,131 @@ func TestForgeMD_StructuralHardening(t *testing.T) { } } }) + + // 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) { From e8dbf37140692ecae420eac7cc8a9eed53a1576f Mon Sep 17 00:00:00 2001 From: Jay Flowers Date: Mon, 10 Aug 2026 14:26:37 -0400 Subject: [PATCH 5/5] fix: rephrase skip_review prohibition as positive constraint, add RFC 2119 keyword to Rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address sonupreetam's 2nd review findings: - MEDIUM: Rephrase skip_review prohibition to avoid teaching agents the bypass parameter name. Now uses positive constraint language: 'The review gate is mandatory — forge_complete MUST NOT be called until forge_review has passed for every worker. NEVER bypass the review gate.' - MEDIUM: Add 'workers MUST NOT call forge_complete' to the Rules section coordinator/worker rule for RFC 2119 consistency with Critical Invariants. - Update test: Rename SkipReviewProhibition → ReviewGateMandatory, verify positive constraint language (MUST NOT, NEVER, 'review gate'). - Update proposal.md: Reflect new capability description. --- internal/agentkit/agentkit_test.go | 15 ++++++++++----- internal/agentkit/content/commands/forge.md | 4 ++-- openspec/changes/forge-dcp-hardening/proposal.md | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/internal/agentkit/agentkit_test.go b/internal/agentkit/agentkit_test.go index 373f865..164c8dd 100644 --- a/internal/agentkit/agentkit_test.go +++ b/internal/agentkit/agentkit_test.go @@ -566,17 +566,22 @@ func TestForgeMD_StructuralHardening(t *testing.T) { } }) - // Scenario 3: skip_review prohibition is present in Critical Invariants. - t.Run("SkipReviewProhibition", func(t *testing.T) { + // 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") } - if !strings.Contains(section, "skip_review") { - t.Error("Critical Invariants must contain skip_review prohibition") + // 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 skip_review prohibition") + 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") } }) diff --git a/internal/agentkit/content/commands/forge.md b/internal/agentkit/content/commands/forge.md index 4adb8d0..727958a 100644 --- a/internal/agentkit/content/commands/forge.md +++ b/internal/agentkit/content/commands/forge.md @@ -11,7 +11,7 @@ Decompose a task and spawn parallel workers. 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. -- **NEVER pass `skip_review: true` to `forge_complete`** — the review gate exists to catch broken work before it is marked done. +- **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`. @@ -44,7 +44,7 @@ $ARGUMENTS - Review every worker's output before marking complete — NEVER skip this step - ALWAYS create a forge, even for small tasks -- Coordinator orchestrates, workers execute +- Coordinator orchestrates, workers execute — workers MUST NOT call `forge_complete` - Workers reserve their own files via `comms_reserve` - Check inbox regularly for blocked workers - Store learnings after completion diff --git a/openspec/changes/forge-dcp-hardening/proposal.md b/openspec/changes/forge-dcp-hardening/proposal.md index 7f50c1d..8673e97 100644 --- a/openspec/changes/forge-dcp-hardening/proposal.md +++ b/openspec/changes/forge-dcp-hardening/proposal.md @@ -20,7 +20,7 @@ Restructure `forge.md` to survive DCP context compression by applying prompt har ### New Capabilities -- `skip_review: true` prohibition added to Critical Invariants — prevents agents from bypassing the review gate via `forge_complete` parameter +- 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