fix: harden forge-coordination skill against DCP compression

[jflowers]

Summary

Restructures the forge-coordination skill to survive DCP context compression. Critical access-control constraints were expressed as parenthetical asides and buried bullet items — exactly the constructs that context compression drops first.

Addresses four specific fragilities identified in #48:

1. (coordinator only) parenthetical on comms_release_all() — promoted to ## Coordinator-Only Operations section header
2. exclusive=true as optional bullet — made the documented default in comms_reserve call signature
3. Conflict resolution in a separate section — inlined at the "Reserve files" step in the Worker Protocol
4. Mixed coordinator/worker rules in a flat list — split into role-scoped sections with MUST/MUST NOT rules at section top

Closes #48

How to Test

# Build compiles with updated embedded skill
make build

# All tests pass (190+ tests, race detection)
make test

# Both skill copies are byte-identical
diff internal/agentkit/content/skills/forge-coordination/SKILL.md \
     .opencode/skills/forge-coordination/SKILL.md

# Verify the restructured document
cat internal/agentkit/content/skills/forge-coordination/SKILL.md

Verify the following structural properties in the output:

• ## Coordinator-Only Operations section exists with MUST NOT rules before protocol steps
• ## Worker-Only Operations section exists with MUST/MUST NOT rules before protocol steps
• comms_release_all() appears only under Coordinator-Only Operations
• Workers MUST NOT call comms_release_all() is explicit in Worker-Only Operations
• Conflict resolution steps are inlined after step 3 (Reserve files) in Worker Protocol
• comms_reserve(paths=[...], exclusive=true, reason="...") is the documented call

How to Demo

Review the restructured skill document side-by-side with the original (via git diff). The key observation is that safety constraints have moved from low-salience positions (parentheticals, buried bullets, separate sections) to high-salience positions (section headers, first-position MUST rules, inline at point of use).

Key Files Changed

Skill files (implementation):

• .opencode/skills/forge-coordination/SKILL.md — restructured with role-scoped sections
• internal/agentkit/content/skills/forge-coordination/SKILL.md — identical copy (embedded in binary)

OpenSpec artifacts (planning):

• openspec/changes/harden-forge-skill-compression/proposal.md — motivation, impact, constitution alignment
• openspec/changes/harden-forge-skill-compression/design.md — 5 design decisions with rationale
• openspec/changes/harden-forge-skill-compression/specs/forge-coordination-hardening.md — delta spec with GIVEN/WHEN/THEN scenarios
• openspec/changes/harden-forge-skill-compression/tasks.md — implementation checklist (all complete)

Known Issues

The following findings from the review council were acknowledged but not resolved (out of scope for this change):

• LOW: forge-global skill uses weaker phrasing for comms_release_all access control
• LOW: /handoff command calls comms_release_all() without role guard
• LOW: Worker agent checklist omits explicit comms_release step

This PR was generated by /uf.finale (AI-assisted).

[sonupreetam]

Review Council

Ran the review council (6 Divisor agents). The role-scoped restructuring is a good pattern but needs structural tests, drift detection, and a spec accuracy fix.

Cross-PR note: comms_release_all() has no caller identity check at the tool level — the "Workers MUST NOT call" constraint is prompt-only. Please file a tracked follow-up issue for caller identity validation.

[sonupreetam]

Review Council (6 Divisor agents)

Verdict: REQUEST CHANGES — one inline finding below, plus mechanical blockers.

Findings

MEDIUM — Proposal says "no behavioral changes" but design D5 adds Worker Protocol step 8: The proposal's Impact section states "No behavioral changes to MCP tools or Go code" and "No test changes required." However, design decision D5 explicitly adds a new step 7 ("Release files") to the Worker Protocol, expanding it from 7 to 8 steps. The design doc itself acknowledges this as "an intentional behavioral modification that strengthens the protocol." The proposal should be updated to reflect this (same fix as PR #54 did for its 7th rule codification).

LOW — findRepoRoot signature will conflict with PR #59: Both PRs define findRepoRoot in agentkit_test.go with different signatures. Whichever merges second must adopt the first's definition. Council recommends merging this PR (#56) 4th, after #55 and #57, so the findRepoRoot function is established here and PR #59 rebases onto it.

Mechanical blockers

• Needs rebase onto main (PR #54 merged, test file has shifted)
• findRepoRoot conflict with PR #59 must be coordinated via merge order

Merge order recommendation

Merge 4th (#54 already merged → #55 → #57 → #56 → #59). Rebase before merge.

[sonupreetam]

MEDIUM: This says "No behavioral changes to MCP tools or Go code" and "No test changes required," but design decision D5 adds a new step 7 ("Release files") to the Worker Protocol, expanding it from 7 to 8 steps. The design doc acknowledges this as "an intentional behavioral modification." Update this section to reflect the protocol step addition — same approach PR #54 used for its 7th rule codification.

[jflowers]

Good catch — the Impact section was stale after D5 was added. Updated in ff0f9e5:

• "No behavioral changes to MCP tools or Go code" → replaced with explicit acknowledgment of the Worker Protocol expansion (7→8 steps, design decision D5)
• "No test changes required" → replaced with reference to the structural test (TestForgeCoordinationSkill_StructuralHardening)

The Impact section now accurately reflects both the protocol step addition and the test addition.

[yvonnedevlinrh]

Review — APPROVE

CI passes. All four #48 fragilities are addressed. Prior CHANGES_REQUESTED findings from @sonupreetam are resolved (Decision #5 documents the 7→8 step expansion, #64 tracks the enforcement gap, structural test added in 65efd9f).

Three LOW findings, none blocking:

1. Stale claim in proposal.md

openspec/changes/harden-forge-skill-compression/proposal.md line 33:

No test changes required — this is a prompt/documentation hardening change

This is contradicted by the addition of TestForgeCoordinationSkill_StructuralHardening. The design.md Non-Goals were updated correctly; proposal.md was not.

2. Incomplete workerMustRules coverage

internal/agentkit/agentkit_test.go — the workerMustRules slice checks 2 of the 4 Worker MUST rules in the pre-protocol block. If either missing rule were deleted from both file copies, the test would still pass.

Suggested fix — add the missing rules:

workerMustRules := []string{
    "MUST reserve files before editing",
    "MUST use exclusive=true",
    "MUST NOT call `comms_release_all()`",
    "MUST release files when done",
}

3. Unbounded workerProtocolSection slice
   workerProtocolSection = text[workerProtocolIdx:] extends past ### Worker Protocol into ## Progress Reporting. The exclusive=true assertion would pass on a false match in any later section.
   Suggested fix — cap at the next ## boundary:

workerProtocolEnd := strings.Index(text[workerProtocolIdx+1:], "\n## ")
if workerProtocolEnd < 0 {
    workerProtocolEnd = len(text) - workerProtocolIdx - 1
}
workerProtocolSection := text[workerProtocolIdx : workerProtocolIdx+1+workerProtocolEnd]

[yvonnedevlinrh]

Manual Testing

Structural test — ran the new hardening test directly, all checks pass:

go test ./internal/agentkit/ -run TestForgeCoordinationSkill_StructuralHardening -v

File sync — confirmed both copies are byte-identical:

diff .opencode/skills/forge-coordination/SKILL.md \
     internal/agentkit/content/skills/forge-coordination/SKILL.md
# no output

End-to-end scaffold — built the binary and verified the embedded skill survives the compile-scaffold pipeline:

make build
tmpdir=$(mktemp -d)
./bin/replicator init --path "$tmpdir"
grep "## Coordinator-Only Operations" "$tmpdir/.opencode/skills/forge-coordination/SKILL.md"  # found
grep "## Worker-Only Operations" "$tmpdir/.opencode/skills/forge-coordination/SKILL.md"        # found
grep "exclusive=true" "$tmpdir/.opencode/skills/forge-coordination/SKILL.md"                   # found
grep "## File Reservation Rules" "$tmpdir/.opencode/skills/forge-coordination/SKILL.md"        # not found (PASS)
grep "## Conflict Resolution" "$tmpdir/.opencode/skills/forge-coordination/SKILL.md"           # not found (PASS)

Finding #2 validation — confirmed the test gap by deleting a MUST rule from both copies and observing the test still passes:

sed -i '/^- Workers MUST use `exclusive=true`/d' \
    internal/agentkit/content/skills/forge-coordination/SKILL.md \
    .opencode/skills/forge-coordination/SKILL.md
go test ./internal/agentkit/ -run TestForgeCoordinationSkill_StructuralHardening -v  # PASS (gap confirmed)
git checkout internal/agentkit/content/skills/forge-coordination/SKILL.md \
             .opencode/skills/forge-coordination/SKILL.md

CI gate — make check passes.

[sonupreetam]

Review Council — Re-review

MEDIUM finding addressed:

1. Proposal accuracy — Impact section now acknowledges Worker Protocol expansion from 7 to 8 steps as "an intentional behavioral modification" and documents the structural test addition. Matches the design doc's D5 framing.

Remaining mechanical blockers

• Needs rebase onto main (PR #54 merged)
• findRepoRoot signature conflict with PR #59 — merge this PR 4th, before #59

Verdict: APPROVE (pending rebase and merge ordering per #68)