fix: harden forge command prompt against DCP context compression

[jflowers]

Summary

Restructures the /forge command prompt (internal/agentkit/content/commands/forge.md) to survive DCP context compression, fixing the quality gate vulnerability where the review-before-complete ordering constraint was lost under compression.

Addresses all four fragilities identified in #47:

1. Review-before-complete ordering embedded with explicit constraint language
2. Critical review rule moved from 5th to 1st position
3. Error Recovery section inlined at point of use
4. Strategy Selection section inlined at point of use

Additionally adds a mandatory review gate constraint to the Critical Invariants section using positive constraint language (avoids naming bypass parameters).

How to Test

# Build succeeds (forge.md is embedded content)
make build

# All tests pass (no behavioral changes)
make test

# Verify all 17 MCP tool references preserved
grep -c 'forge_review\|forge_complete\|comms_inbox\|forge_status' \
  internal/agentkit/content/commands/forge.md

# Verify review constraint in 3 redundant locations
grep -n 'review.*before.*complete\|MUST finish before step 8\|NEVER skip' \
  internal/agentkit/content/commands/forge.md

How to Demo

Read internal/agentkit/content/commands/forge.md and verify the restructured prompt:

• Critical Invariants section appears before Workflow
• Step 7 includes "MUST finish before step 8"
• First rule in Rules section is the review-before-complete rule
• No standalone Strategy Selection, Error Recovery, or Completion sections remain

Key Files Changed

File	Change
internal/agentkit/content/commands/forge.md	Restructured for DCP compression resilience (+22/-39 lines)
openspec/changes/forge-dcp-hardening/proposal.md	Change proposal with constitution alignment
openspec/changes/forge-dcp-hardening/design.md	6 design decisions with risk analysis
openspec/changes/forge-dcp-hardening/specs/forge-prompt-structure.md	Delta spec with Given/When/Then scenarios
openspec/changes/forge-dcp-hardening/tasks.md	Implementation tasks (all complete)

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

Closes #47

[sonupreetam]

Review Council

Ran the review council (6 Divisor agents). The restructuring approach is solid but needs structural tests (like PRs #54/#57/#58 have) and a couple of spec accuracy fixes.

[yvonnedevlinrh]

Reviewed using /uf.review-pr 55 with a custom-built UF binary (latest changes merged into main) after running /uf.init to scaffold the agent kit.

---

MEDIUM

A1: Inconsistent casing between Critical Invariants and Rules

Critical Invariants (line 15) reads ALWAYS create a forge, even for small tasks. while Rules (line 46) reads Always create a forge, even for small tasks. Per the PR's own FR-004: "MUST use explicit RFC 2119 language." The Rules instance predates this PR but the inconsistency was introduced by adding the ALWAYS variant in Critical Invariants without updating Rules to match.

C5: No Review Council APPROVE on revised code

Per AGENTS.md: "there must be minimal to no code changes between the council's APPROVE verdict and the PR submission." The original council review returned CHANGES_REQUESTED. The subsequent revisions added 169 lines of new test code and modified forge.md. No follow-up APPROVE verdict appears in the review history — only COMMENTED reviews with empty bodies. A fresh council APPROVE on the revised code is required before merge.

LOW

A3: No Closes #47 keyword

Issue #47 (all four fragilities addressed by this PR) has no Closes #47 / Fixes #47 keyword in the PR body or commits. It won't auto-close on merge.

[yvonnedevlinrh]

Manual Testing Summary

Tested by checking out opsx/forge-dcp-hardening, building the binary (make build), and running /forge with the scaffolded agent kit to exercise the hardened prompt.

Tests Performed

Test 1 — Small task via /forge: "Rename the hivemind_store tag parameter from tags to tag in all GoDoc comments and error messages across the codebase"

• Agent bypassed the forge workflow entirely. Made 3 edits directly as coordinator — no forge_decompose, forge_spawn_subtask, forge_review, or forge_complete called.
• Context compression did not trigger (session too short).
• The "ALWAYS create a forge, even for small tasks" invariant was ignored.

Test 2 — Larger task via /forge: "Add a --json flag to every CLI command that currently uses table output"

• Agent again bypassed the forge workflow. Edited 7 files, added 22 tests, built and verified — all as a single agent. No forge tools called.
• Context compression triggered twice (-7.4K and -31.3K removed).
• The "ALWAYS create a forge" invariant was ignored a second time, even on a multi-file, multi-package task.

Test 3 — Post-compression context probe: After compression triggered in test 2, asked the agent to describe the forge rules and skip_review constraints.

• Agent read .opencode/commands/forge.md from disk and recited the rules accurately, rather than answering from what survived in compressed context.
• This made the test inconclusive — we couldn't determine whether the constraints survived compression because the agent bypassed its context and went to the source file. Asking about the rules prompted a file lookup rather than a recall from context.

Findings

1. Forge workflow was never exercised — in both tests, the agent chose to do the work directly rather than decomposing into subtasks and spawning workers. The review-before-complete ordering (the core concern of this PR) was never reached.

2. "ALWAYS create a forge" invariant ineffective — the Critical Invariants section states this as a non-negotiable rule with RFC 2119 language, but the agent overrode it in both tests. Prompt-level ALWAYS/MUST/NEVER did not prevent the agent from exercising its own judgment.

3. Compression survival could not be directly tested — the forge workflow was never exercised, so we could not observe whether the review-before-complete constraint survives compression during actual execution. When we attempted to probe the compressed context directly (test 3), the agent re-read the source file instead of relying on context. However, during a real forge session, the command content is loaded into context once when /forge is invoked, and the agent acts on what's in context from that point — it does not re-read the command file before each decision. If compression drops the review constraint mid-session, the agent would proceed without it. This is the real risk the PR aims to mitigate, but we were unable to reach the scenario where it matters.

4. Automated tests are the primary verification — TestForgeMD_StructuralHardening (7/7 subtests passing) confirms the structural properties (section ordering, redundant placement, explicit RFC 2119 keywords) that the design doc argues correlate with compression resilience.

Conclusion

The structural changes and automated tests are solid. The prompt hardening addresses a real risk — during long forge sessions, the agent relies on what survives in compressed context, not the original file. However, the hardening may be necessary but not sufficient. In both manual tests, agents skipped the forge workflow entirely regardless of prompt language, suggesting tool-level enforcement (e.g., requiring forge_decompose when invoked via /forge) is needed alongside prompt-level hardening.

[jflowers]

Addressed all feedback from @yvonnedevlinrh's review:

A1 [MEDIUM]: Applied — normalized Always to ALWAYS in Rules section (forge.md:46) for consistent keyword casing with Critical Invariants section.

A2/E [LOW]: Applied — updated proposal.md: Impact/Testing section now references TestForgeMD_StructuralHardening with 7 subtests. Testability assessment changed from N/A to PASS.

A3 [LOW]: Applied — added Closes #47 to PR body.

T1/F [LOW]: Applied — fixed comment to say 'same-level heading' which matches the implementation.

T2/G [LOW]: Applied — changed Step7OrderingConstraint from OR to AND logic. Test now independently asserts both MUST (RFC 2119 language) AND an ordering signal (first or before step 8).

C5: Acknowledged — will run review council on the revised code before requesting re-review.

Re: manual testing observation — agreed that tool-level enforcement is the right next step. That's tracked in #63 for skip_review: true code enforcement.

All changes in commit e2dece1. make check passes (vet + test with race detection).

[jflowers]

Review Council Report — PR #55

Branch: opsx/forge-dcp-hardening → main
Scope: Prompt restructuring + structural tests + OpenSpec artifacts
Pre-flight: PASS (go vet, go test -race, govulncheck, go build, coverage ratchets 58.4%)
Gaze Quality: Healthy (Scaffold CRAP 7.14, Q1 Safe, 100% contract coverage with AI mapping)

Council Members

Reviewer	Verdict	Findings
Adversary	APPROVE (with caveats)	2 HIGH, 3 MEDIUM, 3 LOW
Architect	APPROVE (9/10)	1 MEDIUM, 6 LOW
Guard	APPROVE	2 MEDIUM, 6 LOW
Testing	REQUEST CHANGES	4 HIGH, 4 MEDIUM, 2 LOW
Scribe	APPROVE	1 MEDIUM, 3 LOW, 2 INFO

Consolidated Verdict: REQUEST CHANGES

The prompt restructuring is well-executed and all 4 fragilities from #47 are addressed. The spec artifacts are exemplary. However, the structural hardening tests have a systematic gap: they verify that standalone sections were removed but do not verify that content was actually inlined at the correct location. For a restructuring PR, this is the core regression risk — content can be silently dropped while tests pass.

---

Blocking Findings (must fix)

B1 [HIGH] Tests verify absence without verifying presence of inlined content

Source: Testing
Files: internal/agentkit/agentkit_test.go:374-386
Spec refs: specs/forge-prompt-structure.md scenarios 7-9

The NoStandaloneSections subtest checks that ## Strategy Selection, ## Error Recovery, and ## Completion do not exist as standalone sections. However, no test verifies that:

• Step 3 contains strategy selection guidance (e.g., forge_get_strategy_insights)
• Step 6 contains error recovery guidance (e.g., "blocked" or "reassign")
• Step 8 contains completion tool references (forge_complete, forge_record_outcome, hivemind_store, org_sync)

Fix: Add 3 subtests — one per inlined section — verifying the content is present at its new location.

B2 [HIGH] No test for MCP tool reference completeness

Source: Testing
Files: internal/agentkit/agentkit_test.go
Task ref: tasks.md line 30, task 2.3

Task 2.3 requires verifying all MCP tool references are preserved after restructuring. This is the strongest regression guard for a restructuring change. Currently not tested.

Fix: Add a subtest listing all expected tool references and verifying each appears in forge.md.

B3 [MEDIUM] RedundantReviewConstraint allows 2-location coverage; spec requires 3

Source: Testing
Files: internal/agentkit/agentkit_test.go:306-325

The test requires inInvariants && (inWorkflow || inRules) — minimum 2 locations. The spec and design require redundancy across all 3 locations (Critical Invariants, step 7, and first rule). Change to inInvariants && inWorkflow && inRules.

---

Non-blocking Findings (address or acknowledge)

N1 [HIGH] Worker prompt contradicts new Critical Invariant

Source: Adversary, Guard, Scribe
Files: internal/forge/spawn.go:41

spawn.go:41 generates worker prompts containing "5. Complete with forge_complete when done" — directly contradicting the new invariant "workers MUST NOT call forge_complete." The design doc acknowledges this (lines 90-94) and scopes it as a follow-up.

Action: File a tracking issue for worker prompt hardening. Reference from design.md.

N2 [HIGH] skip_review prohibition is prompt-only; schema advertises the bypass

Source: Adversary
Files: internal/agentkit/content/commands/forge.md:14

The forge_complete tool schema advertises skip_review: true. The prompt prohibition is defense-in-depth, not a hard gate. Issue #63 already tracks code-level enforcement.

Action: Already tracked. No additional action needed.

N3 [MEDIUM] Assertion matching too loose — "review" + "before" matches unrelated content

Source: Adversary, Testing
Files: internal/agentkit/agentkit_test.go:282-283

ReviewBeforeCompleteInvariant checks for "review" and "before" independently. Content like "review the strategy before decomposing" would satisfy the check. Consider requiring "complete" or a more specific phrase.

N4 [MEDIUM] skip_verification: true not addressed alongside skip_review

Source: Adversary, Scribe
Files: internal/agentkit/content/commands/forge.md:14

forge_complete accepts both skip_review and skip_verification. Only skip_review is prohibited. Document why skip_verification is excluded or add it to the prohibition.

N5 [MEDIUM] Proposal says "9-step workflow"; forge.md has 8 steps

Source: Scribe
Files: openspec/changes/forge-dcp-hardening/proposal.md:3

Minor inaccuracy in spec artifact. Verify against git history and correct if needed.

N6 [LOW] .gitignore change unrelated to forge DCP hardening scope

Source: Adversary, Architect

N7 [LOW] NoStandaloneSections does not include ## Monitoring

Source: Adversary

N8 [LOW] Test helpers (findHeading/sectionContent) could be extracted for reuse

Source: Architect, Testing

---

Walkthrough

File	Change	Focus
.gitignore	Add .uf/feedback/	standard
internal/agentkit/agentkit_test.go	7 structural subtests for forge.md invariants	test-quality
internal/agentkit/content/commands/forge.md	Restructure: Critical Invariants → Workflow → Rules	documentation
openspec/changes/forge-dcp-hardening/* (5)	Full OpenSpec change artifacts	documentation

---

Review Council — 5 Divisor agents, consolidated by coordinator. Pre-flight PASS, Gaze Q1 Safe.

[jflowers]

Review Council — Re-review Verdict: APPROVE

All blocking findings from the previous council have been resolved in commit 3d491e4.

Blocking Findings Resolution

Finding	Status	Verification
B1 [HIGH]: Tests verify absence but not inlined content	RESOLVED	3 new subtests: StrategyInlinedInStep3, ErrorRecoveryInlinedInStep6, CompletionInlinedInStep8
B2 [HIGH]: No MCP tool reference completeness test	RESOLVED	AllToolReferencesPresent checks all 17 tool refs
B3 [MEDIUM]: RedundantReviewConstraint OR→AND	RESOLVED	Now requires all 3 locations (Critical Invariants, Workflow, Rules)

Divisor Results (Re-review)

Divisor	Verdict	Key Finding
Testing	APPROVE	All 3 blocking findings verified resolved. 11 subtests cover all spec scenarios.
Adversary	APPROVE	No new bypass vectors. AND logic correctly enforced.
Guard	APPROVE	No scope creep. All new subtests map directly to spec THEN clauses.
Architect	APPROVE (prior round)	9/10 structural alignment
Scribe	APPROVE (prior round)	Documentation complete

Pre-flight: PASS

• go vet: PASS
• go test -race -count=1: PASS (all packages)
• govulncheck: PASS
• Coverage ratchets: PASS (58.4% global)

Non-blocking items (tracked separately)

• N1: Worker prompt contradiction in spawn.go:41 (design doc acknowledges)
• N2: skip_review prompt-only defense (tracked as feat: enforce skip_review prohibition in forge_complete tool handler #63)
• N3-N5: Assertion tightening, skip_verification, step count (LOW/MEDIUM)

Verdict: All 5 divisors APPROVE. Ready for merge.

[sonupreetam]

Review Council (6 Divisor agents)

Verdict: REQUEST CHANGES — two inline findings below, plus mechanical blockers.

Findings

MEDIUM — skip_review:true naming risk: Explicitly prohibiting skip_review: true in the prompt teaches agents that this bypass parameter exists. Consider rephrasing as a positive constraint or filing a follow-up for tool-level enforcement.

MEDIUM — Inconsistent RFC 2119 casing: The Rules line "Coordinator orchestrates, workers execute" has no RFC 2119 keyword, while the Critical Invariants version states "workers MUST NOT call forge_complete". Consider adding a MUST keyword for consistency with FR-004.

LOW — Missing Closes #47: PR description should include Closes #47 for auto-closing the tracked issue on merge.

Mechanical blockers

• Needs rebase onto main (PR #54 merged, test file has shifted)
• GitHub review status from @yvonnedevlinrh still shows REQUEST_CHANGES on older commit

Merge order recommendation

Merge 2nd (after #54 which is already merged). Rebase before merge.

[jflowers]

Addressed all findings from sonupreetam's 2nd review (commit 634c791):

MEDIUM — skip_review naming risk: Rephrased as positive constraint. Line now reads: "The review gate is mandatory — forge_complete MUST NOT be called until forge_review has passed for every worker. NEVER bypass the review gate." Test renamed SkipReviewProhibition → ReviewGateMandatory to verify positive language.

MEDIUM — Inconsistent RFC 2119 casing: Added workers MUST NOT call forge_complete`` to the Rules section coordinator/worker rule for consistency with Critical Invariants.

LOW — Missing Closes #47: Already present at bottom of PR body (may have been added after review). Verified still present.

Rebase: Branch is already up to date with upstream/main (merge base is c1ec4ef, no divergence).

Pre-flight: make check passes (vet + test with race detection).

[yvonnedevlinrh]

LGTM, only findings are nitpicky items.

[sonupreetam]

Review Council — Re-review

Both MEDIUM findings from the prior council review are addressed:

1. skip_review naming — Rephrased as positive constraint ("The review gate is mandatory"). No longer names the bypass parameter. Test renamed to ReviewGateMandatory with updated assertions. Proposal updated.

2. RFC 2119 casing — Rules section now uses MUST NOT keyword ("workers MUST NOT call forge_complete"), consistent with Critical Invariants.

Remaining mechanical blockers

• Needs rebase onto main (PR #54 merged)
• Closes #47 still missing from PR description

Verdict: APPROVE (pending rebase)