diff --git a/.opencode/skills/forge-coordination/SKILL.md b/.opencode/skills/forge-coordination/SKILL.md index a0dcdd3..e8476ab 100644 --- a/.opencode/skills/forge-coordination/SKILL.md +++ b/.opencode/skills/forge-coordination/SKILL.md @@ -8,7 +8,13 @@ tags: [forge, coordination, multi-agent] Patterns for coordinating parallel agent work. -## Coordinator Protocol +## Coordinator-Only Operations + +- Coordinators MUST NOT reserve files +- Coordinators MUST NOT edit files directly +- Only coordinators MAY call `comms_release_all()` for emergency release of all reservations + +### Coordinator Protocol 1. **Initialize**: `comms_init(project_path=".", task_description="...")` 2. **Check learnings**: `hivemind_find(query="")` @@ -21,23 +27,27 @@ Patterns for coordinating parallel agent work. 9. **Complete**: `forge_complete(bead_id, summary, files_touched)` 10. **Learn**: `hivemind_store(information="...", tags="forge,")` -## Worker Protocol +## Worker-Only Operations + +- Workers MUST reserve files before editing +- Workers MUST use `exclusive=true` when reserving files +- Workers MUST NOT call `comms_release_all()` +- Workers MUST release files when done: `comms_release(paths=[...])` + +### Worker Protocol 1. **Initialize**: `comms_init(project_path=".", task_description="...")` 2. **Check learnings**: `hivemind_find(query="")` -3. **Reserve files**: `comms_reserve(paths=[...], reason="...")` -4. **Implement**: Make changes to reserved files +3. **Reserve files**: `comms_reserve(paths=[...], exclusive=true, reason="...")` + If reservation fails: + a. Check who holds the reservation + b. Send a message via `comms_send` to negotiate release + c. Wait for release or escalate to coordinator +4. **Implement**: Make changes to reserved files only 5. **Report progress**: `forge_progress(bead_id, progress_percent, status)` 6. **Store learnings**: `hivemind_store(information="...", tags="...")` -7. **Complete**: `forge_complete(bead_id, summary, files_touched)` - -## File Reservation Rules - -- Workers MUST reserve files before editing -- Coordinators NEVER reserve files -- Use `comms_reserve(paths=[...], exclusive=true)` for exclusive access -- Release files when done: `comms_release(paths=[...])` -- Emergency release: `comms_release_all()` (coordinator only) +7. **Release files**: `comms_release(paths=[...])` +8. **Complete**: `forge_complete(bead_id, summary, files_touched)` ## Progress Reporting @@ -53,10 +63,3 @@ forge_progress( message="Implemented core logic, starting tests" ) ``` - -## Conflict Resolution - -If a file reservation fails: -1. Check who holds the reservation -2. Send a message via `comms_send` to negotiate -3. Wait for release or escalate to coordinator diff --git a/internal/agentkit/agentkit_test.go b/internal/agentkit/agentkit_test.go index 164c8dd..aae3e30 100644 --- a/internal/agentkit/agentkit_test.go +++ b/internal/agentkit/agentkit_test.go @@ -8,6 +8,26 @@ import ( "testing" ) +// findRepoRoot walks up from the current working directory to find the +// directory containing go.mod, which is the repository root. +func findRepoRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatal("could not find repo root (no go.mod found)") + } + dir = parent + } +} + func TestScaffold_FreshDirectory(t *testing.T) { dir := t.TempDir() results, err := Scaffold(dir, false) @@ -798,6 +818,91 @@ func TestForgeMD_StructuralHardening(t *testing.T) { }) } +func TestForgeCoordinationSkill_StructuralHardening(t *testing.T) { + data, err := content.ReadFile("content/skills/forge-coordination/SKILL.md") + if err != nil { + t.Fatalf("read embedded forge-coordination SKILL.md: %v", err) + } + text := string(data) + + // (1) Role-scoped section headers exist. + roleSections := []string{ + "## Coordinator-Only Operations", + "## Worker-Only Operations", + } + for _, sec := range roleSections { + if !strings.Contains(text, sec) { + t.Errorf("forge-coordination SKILL.md: missing role-scoped section %q", sec) + } + } + + // (2) MUST/MUST NOT rules appear before protocol steps in each role section. + coordinatorOpsIdx := strings.Index(text, "## Coordinator-Only Operations") + coordinatorProtocolIdx := strings.Index(text, "### Coordinator Protocol") + workerOpsIdx := strings.Index(text, "## Worker-Only Operations") + workerProtocolIdx := strings.Index(text, "### Worker Protocol") + + if coordinatorOpsIdx < 0 || coordinatorProtocolIdx < 0 { + t.Fatal("forge-coordination SKILL.md: missing Coordinator sections") + } + if workerOpsIdx < 0 || workerProtocolIdx < 0 { + t.Fatal("forge-coordination SKILL.md: missing Worker sections") + } + + // Coordinator MUST NOT rules must be between the section header and the protocol. + coordSection := text[coordinatorOpsIdx:coordinatorProtocolIdx] + coordMustRules := []string{ + "MUST NOT reserve files", + "MUST NOT edit files directly", + } + for _, rule := range coordMustRules { + if !strings.Contains(coordSection, rule) { + t.Errorf("forge-coordination SKILL.md: coordinator MUST rule %q not found before Coordinator Protocol", rule) + } + } + + // Worker MUST rules must be between the section header and the protocol. + workerSection := text[workerOpsIdx:workerProtocolIdx] + workerMustRules := []string{ + "MUST reserve files before editing", + "MUST NOT call `comms_release_all()`", + } + for _, rule := range workerMustRules { + if !strings.Contains(workerSection, rule) { + t.Errorf("forge-coordination SKILL.md: worker MUST rule %q not found before Worker Protocol", rule) + } + } + + // (3) exclusive=true appears in the comms_reserve call within Worker Protocol. + workerProtocolSection := text[workerProtocolIdx:] + if !strings.Contains(workerProtocolSection, "exclusive=true") { + t.Error("forge-coordination SKILL.md: Worker Protocol missing exclusive=true in comms_reserve call") + } + + // (4) Removed sections from original document are absent. + removedSections := []string{ + "## File Reservation Rules", + "## Conflict Resolution", + } + for _, sec := range removedSections { + if strings.Contains(text, sec) { + t.Errorf("forge-coordination SKILL.md: removed section %q should not be present", sec) + } + } + + // (5) Both copies are byte-identical (embedded agentkit vs .opencode at repo root). + // Find repo root by walking up from the working directory to locate go.mod. + repoRoot := findRepoRoot(t) + opencodePath := filepath.Join(repoRoot, ".opencode", "skills", "forge-coordination", "SKILL.md") + opencodeCopy, err := os.ReadFile(opencodePath) + if err != nil { + t.Fatalf("read .opencode forge-coordination SKILL.md: %v", err) + } + if string(data) != string(opencodeCopy) { + t.Error("forge-coordination SKILL.md: embedded copy and .opencode copy are not byte-identical") + } +} + 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/skills/forge-coordination/SKILL.md b/internal/agentkit/content/skills/forge-coordination/SKILL.md index a0dcdd3..e8476ab 100644 --- a/internal/agentkit/content/skills/forge-coordination/SKILL.md +++ b/internal/agentkit/content/skills/forge-coordination/SKILL.md @@ -8,7 +8,13 @@ tags: [forge, coordination, multi-agent] Patterns for coordinating parallel agent work. -## Coordinator Protocol +## Coordinator-Only Operations + +- Coordinators MUST NOT reserve files +- Coordinators MUST NOT edit files directly +- Only coordinators MAY call `comms_release_all()` for emergency release of all reservations + +### Coordinator Protocol 1. **Initialize**: `comms_init(project_path=".", task_description="...")` 2. **Check learnings**: `hivemind_find(query="")` @@ -21,23 +27,27 @@ Patterns for coordinating parallel agent work. 9. **Complete**: `forge_complete(bead_id, summary, files_touched)` 10. **Learn**: `hivemind_store(information="...", tags="forge,")` -## Worker Protocol +## Worker-Only Operations + +- Workers MUST reserve files before editing +- Workers MUST use `exclusive=true` when reserving files +- Workers MUST NOT call `comms_release_all()` +- Workers MUST release files when done: `comms_release(paths=[...])` + +### Worker Protocol 1. **Initialize**: `comms_init(project_path=".", task_description="...")` 2. **Check learnings**: `hivemind_find(query="")` -3. **Reserve files**: `comms_reserve(paths=[...], reason="...")` -4. **Implement**: Make changes to reserved files +3. **Reserve files**: `comms_reserve(paths=[...], exclusive=true, reason="...")` + If reservation fails: + a. Check who holds the reservation + b. Send a message via `comms_send` to negotiate release + c. Wait for release or escalate to coordinator +4. **Implement**: Make changes to reserved files only 5. **Report progress**: `forge_progress(bead_id, progress_percent, status)` 6. **Store learnings**: `hivemind_store(information="...", tags="...")` -7. **Complete**: `forge_complete(bead_id, summary, files_touched)` - -## File Reservation Rules - -- Workers MUST reserve files before editing -- Coordinators NEVER reserve files -- Use `comms_reserve(paths=[...], exclusive=true)` for exclusive access -- Release files when done: `comms_release(paths=[...])` -- Emergency release: `comms_release_all()` (coordinator only) +7. **Release files**: `comms_release(paths=[...])` +8. **Complete**: `forge_complete(bead_id, summary, files_touched)` ## Progress Reporting @@ -53,10 +63,3 @@ forge_progress( message="Implemented core logic, starting tests" ) ``` - -## Conflict Resolution - -If a file reservation fails: -1. Check who holds the reservation -2. Send a message via `comms_send` to negotiate -3. Wait for release or escalate to coordinator diff --git a/openspec/changes/harden-forge-skill-compression/.openspec.yaml b/openspec/changes/harden-forge-skill-compression/.openspec.yaml new file mode 100644 index 0000000..63fe932 --- /dev/null +++ b/openspec/changes/harden-forge-skill-compression/.openspec.yaml @@ -0,0 +1,2 @@ +schema: unbound-force +created: 2026-08-02 diff --git a/openspec/changes/harden-forge-skill-compression/design.md b/openspec/changes/harden-forge-skill-compression/design.md new file mode 100644 index 0000000..6d73264 --- /dev/null +++ b/openspec/changes/harden-forge-skill-compression/design.md @@ -0,0 +1,70 @@ +## Context + +The `forge-coordination` skill (`internal/agentkit/content/skills/forge-coordination/SKILL.md` and its mirror at `.opencode/skills/forge-coordination/SKILL.md`) encodes critical multi-agent safety constraints. These constraints are currently expressed in low-salience constructs — parenthetical asides, buried bullets, and separate sections — that DCP context compression discards first. + +The proposal (constitution-aligned, all principles PASS or N/A) calls for restructuring the document to promote safety-critical constraints to positions that survive compression. + +## Goals / Non-Goals + +### Goals +- Restructure the skill to use role-scoped sections that clearly separate coordinator-only and worker-only operations +- Inline conflict resolution steps at the point where reservation failure occurs, rather than in a separate section +- Make `exclusive=true` the documented default for `comms_reserve`, not an option to remember +- Ensure the strongest phrasing (MUST/NEVER) appears first and at section-level prominence +- Keep both copies of the file in sync (agentkit embed + opencode skills) + +### Non-Goals +- Changing MCP tool behavior or adding new tools +- Modifying Go source code +- Adding tests for MCP tool behavior (structural tests for document shape are in scope per project convention) +- Changing the agentkit embed mechanism +- Restructuring other skills — this change targets forge-coordination only + +## Decisions + +### 1. Role-scoped sections replace mixed bullet lists + +**Decision**: Replace the flat "File Reservation Rules" section with two explicit sections: "Coordinator-Only Operations" and "Worker-Only Operations." Each section states what that role MUST and MUST NOT do. + +**Rationale**: Section headers are high-salience constructs that survive compression. A parenthetical like `(coordinator only)` is exactly the kind of qualifier DCP drops. Promoting it to a section header makes it structurally impossible to compress away without losing the entire section. + +### 2. Inline conflict resolution at point of use + +**Decision**: Move the conflict resolution steps from a separate section into the Worker Protocol, immediately after the "Reserve files" step. + +**Rationale**: When compression removes "less important" sections, a separate "Conflict Resolution" section is a candidate for removal. If it's inlined at the step where reservation failure occurs, it's part of the protocol flow and survives as long as the protocol itself does. + +### 3. `exclusive=true` as the documented default + +**Decision**: Change the `comms_reserve` call in the Worker Protocol to include `exclusive=true` directly: `comms_reserve(paths=[...], exclusive=true, reason="...")`. + +**Rationale**: When the parameter appears in the primary protocol step, agents copy it by default. When it's a separate bullet explaining an option, it's an optimization to drop. + +### 4. Strongest constraint phrasing in primary position + +**Decision**: Each role section opens with its MUST/MUST NOT rules as the first lines, before any protocol steps. + +**Rationale**: First-position content in a section is the last thing compression removes. Burying MUST rules after descriptive text makes them candidates for trimming. + +### 5. Worker Protocol expanded from 7 to 8 steps + +**Decision**: Insert an explicit "Release files" step (step 7) into the Worker Protocol and renumber "Complete" from step 7 to step 8. + +**Rationale**: The original 7-step protocol omitted an explicit file release step, relying on agents to infer cleanup. Making release explicit (a) ensures file reservations are freed before the completion signal and (b) positions the `comms_release` call at the point of use rather than as a separate rule to remember. This is an intentional behavioral modification that strengthens the protocol. + +### 6. Both files updated identically + +**Decision**: Both `internal/agentkit/content/skills/forge-coordination/SKILL.md` and `.opencode/skills/forge-coordination/SKILL.md` receive the same content. + +**Rationale**: These files are currently identical. One is embedded into the binary for scaffolding; the other is loaded by opencode at runtime. Divergence would create inconsistent agent behavior. + +## Risks / Trade-offs + +### Risk: Increased document length +The restructured document will be slightly longer due to inlining and explicit role sections. This is an acceptable trade-off — a longer document with redundant safety constraints is better than a shorter document where safety constraints are compressed away. + +### Risk: Divergence between the two file copies +Both files must be updated identically. The implementation should update one, then copy to the other, to minimize divergence risk. Existing agentkit embed tests will catch if the embedded copy is missing or malformed. + +### Trade-off: Redundancy vs. DRY +Some constraints will appear in multiple places (e.g., "Workers MUST reserve files" appears in both the Worker Protocol steps and the Worker-Only Operations rules). This intentional redundancy ensures the constraint survives even if one occurrence is compressed. diff --git a/openspec/changes/harden-forge-skill-compression/proposal.md b/openspec/changes/harden-forge-skill-compression/proposal.md new file mode 100644 index 0000000..3ef0167 --- /dev/null +++ b/openspec/changes/harden-forge-skill-compression/proposal.md @@ -0,0 +1,62 @@ +## Why + +The `forge-coordination` skill contains critical access-control and operational constraints expressed as parenthetical asides, weak bullet items, and separated sections. DCP context compression drops these low-salience constructs first, causing agents to lose safety-critical rules: + +- `(coordinator only)` parenthetical on `comms_release_all()` — compressed away, workers could release all reservations system-wide +- `exclusive=true` as one bullet in a list — omitted, workers call `comms_reserve` without exclusivity, enabling concurrent edits +- Conflict resolution steps separated from the point of failure — compressed to "resolve conflicts," skipping negotiation +- Duplicate constraints at different strength levels across files — compressor may pick the weakest phrasing + +References: [unbound-force/replicator#48](https://github.com/unbound-force/replicator/issues/48), [unbound-force/unbound-force#346](https://github.com/unbound-force/unbound-force/issues/346) + +## What Changes + +Restructure the `forge-coordination` skill to survive DCP context compression by promoting critical constraints to high-salience positions (section headers, MUST rules at section top, inline at point-of-use). + +## Capabilities + +### New Capabilities +- None + +### Modified Capabilities +- `forge-coordination skill`: Restructured to use explicit role-scoped sections ("Coordinator-Only Operations", "Worker-Only Operations"), inline conflict resolution at point of use, strongest constraint phrasing in primary position, and `exclusive=true` as the documented default + +### Removed Capabilities +- None + +## Impact + +- **File**: `internal/agentkit/content/skills/forge-coordination/SKILL.md` (embedded agentkit copy) +- **File**: `.opencode/skills/forge-coordination/SKILL.md` (opencode skills copy) +- Both files must stay in sync — they are currently identical +- The Worker Protocol is expanded from 7 to 8 steps (explicit "Release files" step added as step 7; see design decision D5) — this is an intentional behavioral modification to the protocol, not a Go code or MCP tool change +- A structural test (`TestForgeCoordinationSkill_StructuralHardening`) validates compression-critical patterns survive editing +- Agents loading this skill will receive compression-resistant constraints + +## Constitution Alignment + +Assessed against the Unbound Force org constitution. + +### I. Autonomous Collaboration + +**Assessment**: PASS + +This change strengthens artifact-based communication by ensuring the skill document (an artifact consumed by autonomous agents) retains its critical constraints under compression. The change maintains self-describing outputs and does not alter inter-agent communication protocols. + +### II. Composability First + +**Assessment**: N/A + +No dependencies are introduced or removed. The skill file remains a standalone document. No changes to binary functionality or Dewey integration. + +### III. Observable Quality + +**Assessment**: N/A + +No MCP tool responses or machine-parseable outputs are changed. This is a prompt document restructuring. + +### IV. Testability + +**Assessment**: N/A + +No testable components are added or modified. The change targets prompt content only. Existing agentkit embed tests continue to verify the file is properly embedded. diff --git a/openspec/changes/harden-forge-skill-compression/specs/forge-coordination-hardening.md b/openspec/changes/harden-forge-skill-compression/specs/forge-coordination-hardening.md new file mode 100644 index 0000000..e4fd60a --- /dev/null +++ b/openspec/changes/harden-forge-skill-compression/specs/forge-coordination-hardening.md @@ -0,0 +1,90 @@ +## ADDED Requirements + +### Requirement: Role-Scoped Operation Sections + +The forge-coordination skill MUST organize operations under explicit role-scoped section headers: "Coordinator-Only Operations" and "Worker-Only Operations." + +#### Scenario: Coordinator-only operations are visually separated +- **GIVEN** an agent loads the forge-coordination skill +- **WHEN** the skill content is processed (with or without compression) +- **THEN** coordinator-only operations (e.g., `comms_release_all`) appear under a "Coordinator-Only Operations" section header, not as parenthetical annotations + +#### Scenario: Worker-only operations are visually separated +- **GIVEN** an agent loads the forge-coordination skill +- **WHEN** the skill content is processed (with or without compression) +- **THEN** worker-only operations (e.g., `comms_reserve`, file editing) appear under a "Worker-Only Operations" section header + +### Requirement: MUST/MUST NOT Rules at Section Top + +Each role-scoped section MUST open with its access-control constraints (MUST/MUST NOT rules) as the first content after the section header, before protocol steps. + +#### Scenario: Coordinator constraints appear before protocol +- **GIVEN** an agent reads the "Coordinator-Only Operations" section +- **WHEN** the section is parsed top-to-bottom +- **THEN** the constraint "Coordinators MUST NOT reserve files" appears before any protocol steps + +#### Scenario: Worker constraints appear before protocol +- **GIVEN** an agent reads the "Worker-Only Operations" section +- **WHEN** the section is parsed top-to-bottom +- **THEN** the constraint "Workers MUST reserve files before editing" appears before any protocol steps + +### Requirement: Inline Conflict Resolution + +The conflict resolution procedure MUST be inlined in the Worker Protocol at the point where file reservation failure occurs, not in a separate section. + +#### Scenario: Reservation failure triggers inline resolution +- **GIVEN** a worker follows the Worker Protocol +- **WHEN** the worker reaches the "Reserve files" step +- **THEN** conflict resolution steps (check holder, negotiate via `comms_send`, escalate to coordinator) are documented immediately following that step, within the same protocol flow + +### Requirement: comms_release_all Coordinator-Only Access Control + +`comms_release_all` MUST appear exclusively in the "Coordinator-Only Operations" section with a MUST-level access control statement. Workers MUST NOT call `comms_release_all`. + +#### Scenario: comms_release_all is scoped to coordinators +- **GIVEN** an agent reads the "Coordinator-Only Operations" section +- **WHEN** the section is parsed +- **THEN** `comms_release_all` appears with a MUST-level access control statement restricting it to coordinators + +#### Scenario: Workers are explicitly prohibited from release_all +- **GIVEN** an agent reads the "Worker-Only Operations" section +- **WHEN** the section's MUST/MUST NOT constraints are read +- **THEN** a constraint "Workers MUST NOT call `comms_release_all`" is present + +### Requirement: Exclusive Reservation as Default + +The documented `comms_reserve` call in the Worker Protocol MUST include `exclusive=true` in the function signature, making exclusive access the default documented pattern. + +#### Scenario: Worker copies reserve call from protocol +- **GIVEN** a worker copies the `comms_reserve` call from the skill +- **WHEN** the call is executed as documented +- **THEN** `exclusive=true` is included in the call parameters + +## MODIFIED Requirements + +### Requirement: File Reservation Rules + +Previously: Flat bullet list mixing coordinator and worker rules with `(coordinator only)` parenthetical and `exclusive=true` as an optional parameter. + +The file reservation rules MUST be distributed into the appropriate role-scoped sections. The flat "File Reservation Rules" section MUST be replaced by role-specific constraint blocks within "Coordinator-Only Operations" and "Worker-Only Operations." + +### Requirement: Conflict Resolution Section + +Previously: Standalone "## Conflict Resolution" section at the end of the document. + +The conflict resolution procedure MUST be inlined at the point of use in the Worker Protocol. The standalone section MUST be removed. + +### Requirement: File Sync + +Both copies of the skill file MUST contain identical content: +- `internal/agentkit/content/skills/forge-coordination/SKILL.md` +- `.opencode/skills/forge-coordination/SKILL.md` + +#### Scenario: Files stay in sync after change +- **GIVEN** the hardening changes are applied +- **WHEN** both files are compared +- **THEN** they are byte-identical + +## REMOVED Requirements + +None. diff --git a/openspec/changes/harden-forge-skill-compression/tasks.md b/openspec/changes/harden-forge-skill-compression/tasks.md new file mode 100644 index 0000000..97c0f42 --- /dev/null +++ b/openspec/changes/harden-forge-skill-compression/tasks.md @@ -0,0 +1,34 @@ + + +## 1. Restructure the Skill Document + +- [x] 1.1 Rewrite `internal/agentkit/content/skills/forge-coordination/SKILL.md` with the hardened structure: + - Replace "## File Reservation Rules" with "## Coordinator-Only Operations" and "## Worker-Only Operations" sections + - Open each role section with MUST/MUST NOT constraints before protocol steps + - Inline conflict resolution steps into the Worker Protocol after the "Reserve files" step (remove standalone "## Conflict Resolution" section) + - Update `comms_reserve` call in Worker Protocol to include `exclusive=true` as the default: `comms_reserve(paths=[...], exclusive=true, reason="...")` + - Preserve existing Coordinator Protocol and Worker Protocol step sequences + - Move `comms_release_all()` into the Coordinator-Only Operations section with a MUST-level access control statement + +## 2. Sync the Mirror Copy + +- [x] 2.1 Copy the updated content from `internal/agentkit/content/skills/forge-coordination/SKILL.md` to `.opencode/skills/forge-coordination/SKILL.md` so both files are byte-identical + +## 3. Verification + +- [x] 3.1 Run `make build` to verify the agentkit embed compiles with the updated skill file +- [x] 3.2 Run `make test` to verify no existing tests break +- [x] 3.3 Verify both skill files are byte-identical (diff returns no output) +- [x] 3.4 Verify constitution alignment: change is PASS/N/A on all four principles per proposal (no new code, no new tools, no behavioral changes — prompt hardening only) + +