From 58fcc6d5071db6b54c6a0d7856e83a11c78dca0f Mon Sep 17 00:00:00 2001 From: Jared Scott Date: Tue, 26 May 2026 11:56:44 +0800 Subject: [PATCH] fix(status): opt-in stage-report-presence guard on status advance (#235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #235 reports an intermittent failure mode in the spacedock-answer-questions workflow: the FO advances entity status past a stage that emits stage-side YAML approval evidence (e.g. a semantic-mapping.yml with `review.status: approved`) without appending the corresponding `## Stage Report: ` section to the entity body. A downstream consumer that gates artifact publishing on the approval- evidence pair then either rejects valid runs or has to backfill the log section manually. Add an opt-in mechanical guard in `status --set`. When the workflow declares `stages.defaults.require-stage-report: true`, the status binary refuses `--set status=` unless the entity body contains `## Stage Report: ` (with optional `(cycle N)` suffix for feedback rounds). The initial stage is exempt because some workflows use a pure-state intake; only transitions out of stages that ran a worker need the audit log to exist. `--force` bypasses the guard for explicit manual recovery. Default OFF preserves backward compatibility — all 643 existing tests pass without change. Workflows opt in by adding the flag to their README's stages.defaults block. Docs updated in three places: - skills/first-officer/references/first-officer-shared-core.md (Completion and Gates — new "Stage-report-presence enforcement" para) - skills/ensign/references/ensign-shared-core.md (Stage Report Protocol — note about the FO's advancement refusal) - skills/commission/SKILL.md (commission template — optional defaults flag documented) Tests: 8 new test cases in TestStageReportGuard cover refusal, allowed transitions with the report, initial-stage exemption, idempotent --set, --force bypass, opt-in absence (backward compat), `(cycle N)` suffix matching, and the error message contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/commission/SKILL.md | 1 + skills/commission/bin/status | 57 +++++ .../ensign/references/ensign-shared-core.md | 1 + .../references/first-officer-shared-core.md | 2 + tests/test_status_script.py | 204 ++++++++++++++++++ 5 files changed, 265 insertions(+) diff --git a/skills/commission/SKILL.md b/skills/commission/SKILL.md index f677a6975..c1ae72909 100644 --- a/skills/commission/SKILL.md +++ b/skills/commission/SKILL.md @@ -288,6 +288,7 @@ stages: defaults: worktree: false concurrency: 2 + {require-stage-report: true — opt-in (issue #235). When true, `status --set status=` refuses to advance unless the entity body contains a `## Stage Report: ` section (`(cycle N)`-suffixed reports also count). The initial stage is exempt. Reserves `--force` for explicit manual recovery. Recommended for workflows where every stage runs a worker that produces an audit log; skip for workflows whose initial-or-non-worker stages are pure state tracking.} states: - name: {first_stage} initial: true diff --git a/skills/commission/bin/status b/skills/commission/bin/status index 8ca021c65..f64255c7c 100755 --- a/skills/commission/bin/status +++ b/skills/commission/bin/status @@ -2446,6 +2446,63 @@ def main(): print(f'Warning: --force overriding mod-block ({mod_block}) on entity {slug}', file=sys.stderr) + # Stage-report-presence guard (issue #235). When the workflow opts in + # via `stages.defaults.require-stage-report: true`, refuse to advance + # `status: X -> Y` unless the entity body contains a + # `## Stage Report: X` section (optionally `(cycle N)`-suffixed for + # feedback rounds). Catches the intermittent failure mode where an + # FO emits stage-side YAML approvals (e.g. semantic-mapping.yml's + # `review.status: approved`) and advances `status` without writing + # the prose audit section, leaving a "ghost-approved" entity that + # downstream consumers cannot reconstruct from the file alone. + new_status_value = next( + (value.strip() for field, value in updates if field == 'status'), + None, + ) + current_status = current_fields.get('status', '').strip() + if ( + not force + and new_status_value is not None + and new_status_value + and new_status_value != current_status + and current_status + and os.path.exists(readme_path) + ): + _stages_for_defaults, defaults = parse_stages_with_defaults(readme_path) + require_stage_report = ( + str(defaults.get('require-stage-report', 'false')).strip().lower() + == 'true' + ) + if require_stage_report: + # Initial-stage exemption: in some workflows the initial stage + # is a pure intake state without a worker, so the very first + # advancement has no prior report to require. + initial_names = { + s['name'] for s in (stages or []) if s.get('initial') + } + if current_status not in initial_names: + with open(entity_path, 'r') as _body_handle: + body = _body_handle.read() + stage_report_pattern = re.compile( + r'^## Stage Report:\s*' + + re.escape(current_status) + + r'(\s*\(cycle\s+\d+\))?\s*$', + re.MULTILINE, + ) + if not stage_report_pattern.search(body): + print( + f"Error: entity {slug} cannot advance status " + f"{current_status!r} -> {new_status_value!r} — " + f"required audit section " + f"'## Stage Report: {current_status}' is missing " + f"from the entity body. Append the section per the " + f"Stage Report Protocol " + f"(skills/ensign/references/ensign-shared-core.md), " + f"or use --force to bypass.", + file=sys.stderr, + ) + sys.exit(1) + try: resolved = update_frontmatter(entity_path, updates) # Mirror PR metadata back to main for discovery, but keep all diff --git a/skills/ensign/references/ensign-shared-core.md b/skills/ensign/references/ensign-shared-core.md index 63d55d5cd..c2e973f7a 100644 --- a/skills/ensign/references/ensign-shared-core.md +++ b/skills/ensign/references/ensign-shared-core.md @@ -75,6 +75,7 @@ Rules: - do not use markdown checkbox markers in stage reports - append the report at the end of the entity file — do not read the entire entity body to find an insertion point - if redoing a stage after rejection, append a new `## Stage Report: {stage_name} (cycle N)` section at the end rather than locating and overwriting the prior report — the latest report is always the last one in the file +- commit the stage-report section in the same commit that completes your stage work — the first officer's call to `status --set status={next_stage}` will refuse to advance if the section is missing when the workflow opted in to `stages.defaults.require-stage-report: true`. Repairing a missing report after the fact requires either (a) appending the section and re-running the FO's advancement, or (b) the FO passing `--force` (reserved for explicit manual recovery). ## Completion diff --git a/skills/first-officer/references/first-officer-shared-core.md b/skills/first-officer/references/first-officer-shared-core.md index 7b4c7c357..f6bec17af 100644 --- a/skills/first-officer/references/first-officer-shared-core.md +++ b/skills/first-officer/references/first-officer-shared-core.md @@ -129,6 +129,8 @@ The checklist review produces an explicit count summary: **AC coverage cross-check.** Additionally, at every gate, scan the entity body's `## Acceptance criteria` section and confirm each `**AC-N**` item has at least one evidence citation from this stage's report or a prior stage report. Name any AC without evidence; REJECT if this stage was the natural place to address it. This cross-check is independent of checklist DONE/SKIPPED/FAILED accounting — checklist items are dispatch signals, AC items are entity properties. +**Stage-report-presence enforcement.** Workflows can opt in to mechanical enforcement of stage-report presence by setting `stages.defaults.require-stage-report: true` in the workflow README. When opted in, the status binary refuses `status --set status=` unless the entity body contains a `## Stage Report: ` section (optionally `(cycle N)`-suffixed for feedback rounds). The initial stage is exempt because some workflows use a pure-state intake. The FO must ensure the section is appended before calling `--set status=`; if it is missing, fix the report rather than passing `--force` (the bypass exists for manual repairs, not for routine advancement). + If the stage is not gated: if terminal, proceed to merge. Otherwise, decide reuse-or-fresh for the next stage. A completed worker is reusable only when both hold: diff --git a/tests/test_status_script.py b/tests/test_status_script.py index b043b5274..466d9e59e 100644 --- a/tests/test_status_script.py +++ b/tests/test_status_script.py @@ -3522,6 +3522,210 @@ def _read_frontmatter(filepath): return fields +README_WITH_STAGE_REPORT_REQUIRED = textwrap.dedent("""\ + --- + entity-type: task + entity-label: task + stages: + defaults: + worktree: false + concurrency: 2 + require-stage-report: true + states: + - name: backlog + initial: true + - name: ideation + gate: true + - name: implementation + worktree: true + - name: validation + worktree: true + fresh: true + gate: true + - name: done + terminal: true + --- + + # Test Pipeline (stage-report enforced) + """) + + +def _entity_with_body(id, title, status, body, score='', worktree='', pr=''): + """Generate an entity file with a custom body section (after frontmatter). + + Built as frontmatter + body concatenation so the body's own indentation is + not flattened by an outer `textwrap.dedent` (a multi-line `{body}` + interpolation produces inconsistent leading whitespace that defeats the + common-leading-whitespace calculation). + """ + frontmatter = textwrap.dedent(f"""\ + --- + id: {id} + title: {title} + status: {status} + source: + started: + completed: + verdict: + score: {score} + worktree: {worktree} + pr: {pr} + --- + """) + return frontmatter + "\n" + body + + +class TestStageReportGuard(unittest.TestCase): + """Test stage-report-presence enforcement on status transitions. + + Issue #235: in workflows where every stage produces an audit log section + (`## Stage Report: {stage}`), the FO can intermittently advance status + without writing the section. When the workflow opts in via + `stages.defaults.require-stage-report: true`, the status binary refuses + to advance `status: X -> Y` unless the entity body contains + `## Stage Report: X` (or `## Stage Report: X (cycle N)`). + + Opt-in preserves backward compatibility with workflows that use pure + state-tracking stages (e.g. a `backlog` initial stage with no worker). + """ + + def setUp(self): + self._script_dir = tempfile.mkdtemp() + self.script_path = build_status_script(self._script_dir) + + def tearDown(self): + os.unlink(self.script_path) + os.rmdir(self._script_dir) + + def test_advance_refused_when_prior_stage_report_missing(self): + """Opt-in workflow: --set status=Y refused when body lacks '## Stage Report: X'.""" + with tempfile.TemporaryDirectory() as tmpdir: + make_pipeline(tmpdir, README_WITH_STAGE_REPORT_REQUIRED, { + 'task-a.md': _entity_with_body('001', 'Task A', 'ideation', + body='Description with no stage report.'), + }) + result = run_status(tmpdir, '--set', 'task-a', 'status=implementation', + script_path=self.script_path) + self.assertNotEqual(result.returncode, 0) + self.assertIn('Stage Report: ideation', result.stderr) + # Body unchanged, status field unchanged. + fields = _read_frontmatter(os.path.join(tmpdir, 'task-a.md')) + self.assertEqual(fields['status'], 'ideation') + + def test_advance_allowed_when_prior_stage_report_present(self): + """Opt-in workflow: --set status=Y succeeds when body has '## Stage Report: X'.""" + with tempfile.TemporaryDirectory() as tmpdir: + body = textwrap.dedent("""\ + Description. + + ## Stage Report: ideation + + - DONE: chose approach + Evidence: see commit abc123. + + ### Summary + + Approach selected. + """) + make_pipeline(tmpdir, README_WITH_STAGE_REPORT_REQUIRED, { + 'task-a.md': _entity_with_body('001', 'Task A', 'ideation', body=body), + }) + result = run_status(tmpdir, '--set', 'task-a', 'status=implementation', + script_path=self.script_path) + self.assertEqual(result.returncode, 0, result.stderr) + fields = _read_frontmatter(os.path.join(tmpdir, 'task-a.md')) + self.assertEqual(fields['status'], 'implementation') + + def test_advance_allowed_from_initial_stage_without_report(self): + """Opt-in workflow: advancing OUT of the initial stage is exempt. + + The initial stage is often a pure intake state without a worker. + Requiring a stage report for its exit would break the very first + advancement (entity creation -> first work stage). + """ + with tempfile.TemporaryDirectory() as tmpdir: + make_pipeline(tmpdir, README_WITH_STAGE_REPORT_REQUIRED, { + 'task-a.md': _entity_with_body('001', 'Task A', 'backlog', + body='Description with no stage report.'), + }) + result = run_status(tmpdir, '--set', 'task-a', 'status=ideation', + script_path=self.script_path) + self.assertEqual(result.returncode, 0, result.stderr) + fields = _read_frontmatter(os.path.join(tmpdir, 'task-a.md')) + self.assertEqual(fields['status'], 'ideation') + + def test_idempotent_status_update_allowed_without_report(self): + """Setting status to its current value is a no-op and bypasses the guard.""" + with tempfile.TemporaryDirectory() as tmpdir: + make_pipeline(tmpdir, README_WITH_STAGE_REPORT_REQUIRED, { + 'task-a.md': _entity_with_body('001', 'Task A', 'ideation', + body='Description with no stage report.'), + }) + result = run_status(tmpdir, '--set', 'task-a', 'status=ideation', + script_path=self.script_path) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_advance_allowed_with_force_bypass(self): + """--force bypasses the guard (escape hatch for manual repairs).""" + with tempfile.TemporaryDirectory() as tmpdir: + make_pipeline(tmpdir, README_WITH_STAGE_REPORT_REQUIRED, { + 'task-a.md': _entity_with_body('001', 'Task A', 'ideation', + body='Description with no stage report.'), + }) + result = run_status(tmpdir, '--set', 'task-a', + 'status=implementation', '--force', + script_path=self.script_path) + self.assertEqual(result.returncode, 0, result.stderr) + fields = _read_frontmatter(os.path.join(tmpdir, 'task-a.md')) + self.assertEqual(fields['status'], 'implementation') + + def test_advance_allowed_when_opt_in_absent(self): + """Default workflow (no require-stage-report) preserves prior behavior.""" + with tempfile.TemporaryDirectory() as tmpdir: + make_pipeline(tmpdir, README_WITH_STAGES, { + 'task-a.md': _entity_with_body('001', 'Task A', 'ideation', + body='Description with no stage report.'), + }) + result = run_status(tmpdir, '--set', 'task-a', 'status=implementation', + script_path=self.script_path) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_advance_matches_cycle_suffix(self): + """A report with the '(cycle N)' suffix from feedback rounds counts.""" + with tempfile.TemporaryDirectory() as tmpdir: + body = textwrap.dedent("""\ + Description. + + ## Stage Report: ideation + + - DONE: original cycle. + + ## Stage Report: ideation (cycle 2) + + - DONE: redone after feedback. + """) + make_pipeline(tmpdir, README_WITH_STAGE_REPORT_REQUIRED, { + 'task-a.md': _entity_with_body('001', 'Task A', 'ideation', body=body), + }) + result = run_status(tmpdir, '--set', 'task-a', 'status=implementation', + script_path=self.script_path) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_error_message_names_the_missing_section(self): + """Error message is specific enough for an FO to fix without context.""" + with tempfile.TemporaryDirectory() as tmpdir: + make_pipeline(tmpdir, README_WITH_STAGE_REPORT_REQUIRED, { + 'task-a.md': _entity_with_body('001', 'Task A', 'implementation', + body='Description with no stage report.'), + }) + result = run_status(tmpdir, '--set', 'task-a', 'status=validation', + script_path=self.script_path) + self.assertNotEqual(result.returncode, 0) + self.assertIn('## Stage Report: implementation', result.stderr) + # Mentions --force as the documented escape hatch. + self.assertIn('--force', result.stderr) + + class TestEntityAsFolder(unittest.TestCase): """Test first-class entity-as-folder support (issue #99).