Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions skills/commission/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: <prior_stage>` 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
Expand Down
57 changes: 57 additions & 0 deletions skills/commission/bin/status
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions skills/ensign/references/ensign-shared-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions skills/first-officer/references/first-officer-shared-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <slug> status=<next>` unless the entity body contains a `## Stage Report: <prior>` 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:
Expand Down
204 changes: 204 additions & 0 deletions tests/test_status_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
Loading