Fix #18: workflow-gate and emit-shape regressions in v1.2.0

skills/pharaoh-change/SKILL.md

@@ -3,6 +3,16 @@ name: pharaoh-change
 description: "Use when analyzing the impact of changing a requirement, specification, or any sphinx-needs item, including traceability to code via codelinks"
 ---
 
+## Output invariant
+
+This skill's visible output is the full **Change Document** as defined in Step 4. The document is mandatory. Every invocation MUST emit the complete document with all sections. The session-state update in Step 5 is internal bookkeeping and MUST NOT replace the document.
+
+Failure modes:
+- Returning only "Acknowledge this change analysis?" -> REGRESSION. Emit the full document.
+- Returning only "Session state written" -> REGRESSION. Emit the full document.
+
+Non-interactive callers cannot reach the document past a follow-up prompt. The skill MUST be self-terminating. Acknowledgment is checked by downstream enforcing-mode authoring skills (`pharaoh-author`, `pharaoh-req-regenerate`, etc.) against the session-state flag. They FAIL with a message naming the file path and the field to flip.
+
 # pharaoh-change: Change Impact Analysis
 
 Analyze the full impact of a proposed change to any sphinx-needs item. Trace through ALL link types -- standard `links`, `extra_links` (implements, tests, etc.), and sphinx-codelinks -- to produce a structured Change Document listing every affected need and code file with a recommended action.
@@ -296,7 +306,7 @@ For each target need ID, add or update an entry in the `changes` dictionary:
 
 Key points:
 - Set `change_analysis` to the current timestamp.
-- Set `acknowledged` to `false`. The user must explicitly acknowledge before this gate is satisfied.
+- Set `acknowledged` to `false`. Downstream enforcing-mode authoring skills check this flag and the user flips it by editing the session-state file directly.
 - Do not overwrite `authored` or `verified` if the entry already exists -- preserve those values.
 - Update the top-level `updated` timestamp.
 
@@ -306,35 +316,16 @@ Write the updated JSON to `.pharaoh/session.json`. Ensure the JSON is properly f
 
 ---
 
-## 6. Ask for Acknowledgment
-
-After presenting the Change Document and updating session state, ask the user to acknowledge the analysis.
-
-Present exactly this prompt:
-
-```
-Acknowledge this change analysis? Acknowledging allows proceeding to the authoring skill for the affected needs.
-```
-
-### If the user acknowledges
-
-Update `.pharaoh/session.json`: set `acknowledged` to `true` for each target need ID analyzed in this invocation. Update the `updated` timestamp.
-
-Respond with:
-
-```
-Change analysis for <TARGET_ID(s)> acknowledged. You may now proceed with the appropriate authoring skill.
-```
-
-### If the user does not acknowledge
+## 6. End the turn
 
-Do not update the session state. The `acknowledged` field remains `false`.
+After emitting the Change Document and writing session state, end the turn. Do not ask the user any follow-up question.
 
-If the user asks questions about the Change Document, answer them. If the user requests modifications to the analysis (e.g., "also check the impact on module X"), re-run the relevant parts of the analysis and present an updated Change Document. Then ask for acknowledgment again.
+Acknowledgment is a separate concern handled by downstream authoring skills:
 
-### If the user ignores the acknowledgment prompt
+- In **advisory** strictness, no skill checks `acknowledged`. The user proceeds freely.
+- In **enforcing** strictness, downstream authoring skills check `.pharaoh/session.json[changes][<id>].acknowledged` and FAIL with a message naming the file path and the field to flip. The user edits the session-state file directly to acknowledge.
 
-Do not force the issue. The session state remains with `acknowledged: false`. In advisory mode this has no effect. In enforcing mode, any authoring skill will check and block if acknowledgment is missing.
+This split keeps `pharaoh-change` non-interactive and CI-safe.
 
 ---
 
@@ -346,13 +337,13 @@ Follow the instructions in `skills/shared/strictness.md` for strictness handling
 
 - Always produce the full Change Document regardless of workflow state.
 - No gating -- this skill has no prerequisites.
-- After completing the analysis, the acknowledgment step is optional. If the user skips it, other skills will show a tip but will not block.
+- The `acknowledged` flag in session state remains `false`. Other skills will show a tip but will not block.
 
 ### Enforcing mode
 
 - This skill itself has no prerequisites (it is gate-free per `skills/shared/strictness.md` Section 3, "Skills with no gates").
 - However, its output gates any authoring skill. In enforcing mode, authoring skills check `.pharaoh/session.json` for `acknowledged: true` on the relevant need IDs.
-- Always perform the full analysis. Always update session state. Always ask for acknowledgment.
+- Always perform the full analysis. Always update session state. Always end the turn after emitting the document.
 
 ### Strictness has no effect on analysis depth
 
@@ -494,4 +485,4 @@ Code impact: Not applicable (codelinks not configured).
 
 **Step 5** -- Session state written: `REQ_001` entry with `acknowledged: false`.
 
-**Step 6** -- User asked to acknowledge. User says "yes". Session updated: `acknowledged: true`.
+**Step 6** -- Turn ends after the Change Document and session-state write. Session state holds `acknowledged: false`. The user edits `.pharaoh/session.json` directly to flip the flag before invoking an enforcing-mode authoring skill.

skills/pharaoh-decide/SKILL.md

@@ -3,6 +3,17 @@ name: pharaoh-decide
 description: "Use when recording a design decision as a traceable sphinx-needs object with alternatives, rationale, and links to affected requirements"
 ---
 
+## Output invariant
+
+This skill's visible output MUST contain, in order:
+
+1. Confirmation line: `Decision <DEC_ID> written to <file_path>`
+2. Optional follow-up suggestion (Step 7 content)
+
+The confirmation is mandatory. The suggestion is optional and is suppressed when the skill is called by `pharaoh:spec` (per existing Step 7 logic). Returning only the suggestion without the confirmation line is a REGRESSION.
+
+Session-state update is internal bookkeeping and runs before the visible output.
+
 # pharaoh-decide
 
 Record design decisions as traceable sphinx-needs `decision` directives. Each decision captures the chosen option, rejected alternatives, rationale, and explicit links to the requirements or specifications it affects. This skill ensures every decision has proper `decided_by`, `alternatives`, and `rationale` fields.
@@ -175,7 +186,9 @@ Decisions
 
 ---
 
-### Step 6: Update Session State
+### Step 6: Update Session State (internal)
+
+This step is internal bookkeeping. It MUST run before any visible output is emitted in Step 7. Do not print anything in this step.
 
 After successfully writing the decision:
 
@@ -203,15 +216,19 @@ After successfully writing the decision:
 
 #### Standalone invocation
 
-After writing the decision, suggest the next step:
+Emit the visible turn output. The FIRST line MUST be the written-confirmation. The follow-up suggestion is appended after a blank line:
 
 ```
+Decision <DEC_ID> written to <file_path>
+
 Next step: Run pharaoh:req-review to validate the decision against its linked requirements.
 ```
 
+Substitute `<DEC_ID>` with the generated ID from Step 3 and `<file_path>` with the absolute or repo-relative path to the file written in Step 4 / Step 5. The confirmation line MUST appear even if the follow-up suggestion is suppressed for any reason.
+
 #### Called by `pharaoh:spec`
 
-Return the decision ID silently. Do not print follow-up suggestions. The calling skill manages the workflow.
+Return the decision ID silently. Do not print follow-up suggestions. Do not print the written-confirmation line. The calling skill manages the workflow.
 
 ---
 
@@ -302,9 +319,11 @@ Why was PostgreSQL chosen over the alternatives?
 
 **Step 6** -- Session state updated: `DEC_003.authored = true`.
 
-**Step 7** -- Follow-up:
+**Step 7** -- Visible output:
 
 ```
+Decision DEC_003 written to docs/decisions.rst
+
 Next step: Run pharaoh:req-review to validate the decision against its linked requirements.
 ```
 

skills/pharaoh-mece/SKILL.md

@@ -3,6 +3,17 @@ name: pharaoh-mece
 description: "Use when checking for gaps, redundancies, and inconsistencies in sphinx-needs requirements, or validating traceability completeness"
 ---
 
+## Output invariant
+
+This skill's visible output is the full **MECE Analysis Report** as defined in Step 9. The report is mandatory -- every invocation MUST emit the complete report with all sections (omit only sections with no findings, but always emit the Summary section). The session-state update in Step 8 is internal bookkeeping and MUST NOT replace the report in the output.
+
+Failure modes:
+- Returning a one-paragraph executive summary instead of the table-formatted report -> REGRESSION. Emit the full report.
+- Returning only the session-state confirmation -> REGRESSION. Emit the full report.
+- Returning the report and asking the user a follow-up question instead of finishing -> REGRESSION. End the turn after the report.
+
+Non-interactive callers (`claude -p`, CI, batch) cannot reach the report past a follow-up prompt. The skill MUST be self-terminating.
+
 # pharaoh:mece -- MECE Analysis
 
 Analyze a sphinx-needs project for structural completeness and consistency.
@@ -300,11 +311,33 @@ Schema validation: Skipped (ubc CLI not available)
 
 ---
 
-### Step 8: Present MECE report
+### Step 8: Update session state (internal)
+
+This step is internal bookkeeping. Perform it silently before emitting the
+report in Step 9 -- do not narrate it in the visible output.
+
+Update the session state file (`.pharaoh/session.json`) as described in
+`skills/shared/strictness.md`:
+
+1. Read the current `.pharaoh/session.json` (or create the default structure
+   if it does not exist).
+2. Set `global.mece_checked` to `true`.
+3. Set `global.mece_timestamp` to the current ISO 8601 timestamp.
+4. Set `updated` to the current ISO 8601 timestamp.
+5. Write the file back.
+
+The interaction with `require_mece_on_release` is described in Section 3
+(Strictness Behavior).
+
+---
+
+### Step 9: Emit the MECE report (visible output)
 
-Compile all findings into a single structured report. Use the format below
-exactly. Omit sections that have no findings (but mention "None found" in the
-summary counts).
+This is the skill's visible output. Compile all findings into a single
+structured report. Use the format below exactly. Omit sections that have no
+findings (but mention "None found" in the summary counts). Always emit the
+Summary section. After emitting the report, end the turn -- do not ask the
+user follow-up questions.
 
 ```
 ## MECE Analysis Report
@@ -376,22 +409,7 @@ summary counts).
 - **critical**: More than 5 errors, or any category has more errors than valid
   needs of that type. The traceability structure has significant problems.
 
----
-
-### Step 9: Update session state
-
-After presenting the report, update the session state file
-(`.pharaoh/session.json`) as described in `skills/shared/strictness.md`:
-
-1. Read the current `.pharaoh/session.json` (or create the default structure
-   if it does not exist).
-2. Set `global.mece_checked` to `true`.
-3. Set `global.mece_timestamp` to the current ISO 8601 timestamp.
-4. Set `updated` to the current ISO 8601 timestamp.
-5. Write the file back.
-
-This records that MECE analysis was performed, which satisfies the
-`require_mece_on_release` gate if `pharaoh.toml` has it enabled.
+Emit the report and end the turn.
 
 ---
 

skills/pharaoh-release/SKILL.md

@@ -3,6 +3,16 @@ name: pharaoh-release
 description: "Use when preparing a release, generating changelogs from requirements, or summarizing requirement changes for version management"
 ---
 
+## Output invariant
+
+This skill's visible output is the full release report (changelog plus release summary) as defined in Step 7. The report is mandatory. Every invocation MUST emit the complete report. The session-state update is internal bookkeeping. The skill is self-terminating. No interactive prompt at the end of the turn.
+
+Failure modes:
+- Ending the turn with a "save these release artifacts?" prompt instead of the report -> REGRESSION. Emit the full report and end the turn.
+- Returning a brief summary instead of the full changelog and release summary -> REGRESSION.
+
+File-save behaviour is controlled by an input flag (`save_artifacts`), not an interactive prompt. See Step 7.2 below.
+
 # pharaoh-release
 
 Generate release artifacts from sphinx-needs changes. This skill identifies which
@@ -19,6 +29,10 @@ analysis).
 - Producing traceability coverage metrics for compliance or audit documentation.
 - Generating release notes that include requirement impact chains.
 
+## Input
+
+- **`save_artifacts`** (optional, default `none`): one of `none` | `changelog` | `release_notes` | `both`. Controls whether the skill writes the rendered changelog/release-notes artifacts to disk in addition to emitting them in the turn output. Pass explicitly when calling non-interactively. Defaults to `none` so a bare invocation never silently writes files.
+
 ## Prerequisites
 
 This skill has workflow gates. Follow the strictness check in Step 1 before
@@ -438,87 +452,83 @@ Code files referencing needs: <count>
 
 ### Step 7: Output and Next Steps
 
-#### 7a. Present changelog to user
+The sub-steps below run in this fixed order: session-state update first
+(internal bookkeeping), then artifact writes per the `save_artifacts` input
+flag (silent side effects), and finally the visible release report. The
+report is the LAST instruction in the skill so it is the last visible turn
+content. The skill MUST end the turn after the report. No interactive
+prompts.
 
-Display the complete changelog (from Step 5) and release summary (from Step 6)
-to the user in a single output.
+#### 7.1. Update session state (internal)
 
-#### 7b. Offer to write to file
+Run this BEFORE emitting any visible output. Update `.pharaoh/session.json`:
 
-After presenting the output, ask the user:
+1. Read the current session state (or create the initial structure).
+2. Set `global.last_release` to the current ISO 8601 timestamp.
+3. Set `updated` to the current ISO 8601 timestamp.
+4. Write the updated JSON back to `.pharaoh/session.json`.
 
-```
-Would you like to save these release artifacts?
+This is internal bookkeeping. It MUST NOT produce any visible turn output.
 
-  1. Write changelog to CHANGELOG.md (append at top)
-  2. Write full release notes to docs/releases/<version>.md
-  3. Write both
-  4. Do not write any files
+#### 7.2. Write artifacts per `save_artifacts` input flag
 
-Choose an option: [1/2/3/4]
-```
+Branch on the `save_artifacts` input value (default `none`):
 
-**Option 1: Append to CHANGELOG.md**
+- `none`: do nothing. The output was already presented.
+- `changelog`: append the changelog entry to `CHANGELOG.md` (Option 1 logic below).
+- `release_notes`: write `docs/releases/<version>.md` (Option 2 logic below).
+- `both`: execute both branches in order.
+
+Do NOT ask the user. Callers that want to choose interactively pass the flag explicitly.
+
+**`changelog` branch (append to CHANGELOG.md):**
 
 1. Check if `CHANGELOG.md` exists in the workspace root.
-2. If it exists, read its current content. Insert the new changelog entry at the
-   top of the file, after any existing header (e.g., after a `# Changelog` line).
-3. If it does not exist, create it with a `# Changelog` header followed by the
-   new entry.
-4. Show the user what will be written and confirm before writing.
+2. If it exists, read its current content. Insert the new changelog entry at the top, after any existing header (e.g. after a `# Changelog` line).
+3. If it does not exist, create it with a `# Changelog` header followed by the new entry.
 
-**Option 2: Write to docs/releases/**
+**`release_notes` branch (write `docs/releases/<version>.md`):**
 
 1. Create the `docs/releases/` directory if it does not exist.
-2. Write the full release notes (changelog + release summary) to
-   `docs/releases/<version>.md`.
-3. Show the user what will be written and confirm before writing.
-
-**Option 3: Both**
+2. Write the full release notes (changelog plus release summary) to `docs/releases/<version>.md`.
 
-Execute both Option 1 and Option 2.
+**`both` branch:** execute both branches in order.
 
-**Option 4: No files**
+#### 7.3. Emit the release report (final visible output)
 
-Do nothing. The output was already presented on screen.
+This sub-step is the LAST instruction in the skill and produces the LAST
+visible turn content. After emitting the report, end the turn. Do not ask
+follow-up questions.
 
-#### 7c. Suggest git tag
+Display the complete changelog (from Step 5) and release summary (from
+Step 6) to the user in a single output. Append the tag suggestion below as
+the final footer line of the report.
 
-After file output is handled, suggest tagging:
+**Tag suggestion footer (append at the end of the report):**
 
 ```
 Suggested next step:
   git tag -a <version> -m "Release <version>"
 
-Create this tag now? [yes/no]
+(Run the tag command manually if desired. The skill does not run git tag
+automatically.)
 ```
 
-If the user confirms, run the `git tag` command. Do **not** push the tag. If the
-user wants to push, they must explicitly request it.
-
-If the user declines, do nothing.
-
-#### 7d. Update session state
-
-After the release process completes successfully (regardless of whether files were
-written), update `.pharaoh/session.json`:
-
-1. Read the current session state (or create the initial structure).
-2. Set `global.last_release` to the current ISO 8601 timestamp.
-3. Set `updated` to the current ISO 8601 timestamp.
-4. Write the updated JSON back to `.pharaoh/session.json`.
+The skill MUST NOT execute `git tag` itself. The skill MUST NOT ask the
+user whether to create the tag. The footer is informational only.
 
 ---
 
 ## Key Constraints
 
-1. **Never auto-tag or auto-push without user confirmation.** The `git tag` and
-   `git push` commands must always be explicitly confirmed by the user. Never run
-   them silently.
+1. **Never run `git tag` or `git push`.** The skill MUST NOT execute either
+   command. The release report includes a tag suggestion as a footer line so the
+   user can run it manually. The skill MUST NOT prompt the user about tagging.
 
-2. **Never overwrite files without asking.** Before writing to `CHANGELOG.md` or
-   any release notes file, show the user what will be written and get explicit
-   confirmation. If the file already exists, show how it will be modified.
+2. **File writes are gated by the `save_artifacts` input flag.** The skill MUST
+   NOT prompt the user before writing `CHANGELOG.md` or `docs/releases/<version>.md`.
+   The flag is the contract. Default `none` means no files are written. When the
+   flag selects a write branch, perform the write without asking.
 
 3. **Include traceability metrics for safety-critical audit trails.** The release
    summary must always include the needs inventory and traceability coverage