-
Notifications
You must be signed in to change notification settings - Fork 7
fix(ci): make chunk state atomic with merge #319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Chunk Map: WS-CI-003 Atomic Chunk State | ||
|
|
||
| | Chunk | Goal | Risk | State represented by this change | | ||
| |---|---|---:|---| | ||
| | `WS-CI-003-01` | Require atomic chunk completion state in the implementation PR | L1 | Complete | | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # Status: WS-CI-003 Atomic Chunk State | ||
|
|
||
| - Initiative state: active | ||
| - Current chunk: `WS-CI-003-01` | ||
| - Outcome on merge: `WS-CI-003-01` is complete and Agent Gates requires every | ||
| chunk PR to carry its contract, chunk-map, initiative-status, and current-state | ||
| outcome atomically. | ||
| - Product behavior changed: no | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| # Chunk Contract: WS-CI-003-01 Atomic Chunk State | ||
|
|
||
| ## Goal | ||
|
|
||
| Ensure a human merge atomically lands both the bounded change and its durable | ||
| chunk/initiative state, without a pre-merge memory PR or post-merge repair PR. | ||
|
|
||
| ## Why this chunk exists | ||
|
|
||
| PR #318 had to reconcile state after earlier chunks merged, and its own | ||
| `WS-ARCH-001-HK1` row still landed as `In review`. The repository had no gate | ||
| requiring changed chunk contracts and their projections to describe the state | ||
| that would exist after merge. | ||
|
|
||
| ## Risk class | ||
|
|
||
| L1 CI and contributor workflow. | ||
|
|
||
| ## Allowed files | ||
|
|
||
| ```text | ||
| .github/workflows/agent-gates.yml | ||
| .github/pull_request_template.md | ||
| AGENTS.md | ||
| CONTRIBUTING.md | ||
| scripts/check_chunk_state_sync.py | ||
| scripts/test_chunk_state_sync.py | ||
| scripts/test_lightweight_agent_gates.py | ||
| .agent-loop/CURRENT_STATE.md | ||
| .agent-loop/templates/PR_TRUST_BUNDLE.md | ||
| .agent-loop/initiatives/WS-CI-002-deterministic-agent-gates/STATUS.md | ||
| .agent-loop/initiatives/WS-CI-003-atomic-chunk-state/** | ||
| ``` | ||
|
|
||
| ## Not allowed | ||
|
|
||
| - Post-merge commits, automated merge PRs, direct pushes, or write tokens. | ||
| - Product, schema, authorization, dependency, test-selection, coverage, or | ||
| branch-protection changes. | ||
| - Inferring completion from historical review files or chat. | ||
| - More than one implementation chunk in one PR. | ||
|
|
||
| ## Acceptance criteria | ||
|
|
||
| - [x] Implementation-surface changes require exactly one changed chunk contract. | ||
| - [x] Every changed chunk contract declares one final outcome on merge. | ||
| - [x] The same PR changes its initiative `CHUNK_MAP.md`, initiative `STATUS.md`, | ||
| and `.agent-loop/CURRENT_STATE.md`. | ||
| - [x] All three projections name the exact chunk and final outcome. | ||
| - [x] A completed chunk cannot remain `in review`, `pending review`, or | ||
| `ready for review` in its chunk-map row. | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| - [x] Planning, completion, cancellation, and supersession are supported. | ||
| - [x] GitHub review and human merge remain the only approval and merge steps. | ||
| - [x] No post-merge automation is introduced. | ||
|
|
||
| ## Merge state | ||
|
|
||
| - Outcome on merge: `complete` | ||
|
|
||
| ## Verification | ||
|
|
||
| ```bash | ||
| python3 -m unittest -v scripts.test_chunk_state_sync scripts.test_lightweight_agent_gates | ||
| python3 scripts/check_chunk_state_sync.py --base-ref origin/main | ||
| python3 scripts/check_markdown_links.py | ||
| python3 scripts/check_stale_workstream_wording.py | ||
| git diff --check origin/main...HEAD | ||
| ``` | ||
|
|
||
| ## Required review | ||
|
|
||
| CI integrity and documentation review. Human review should confirm the rule is | ||
| atomic, deterministic, and does not introduce a second merge workflow. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,177 @@ | ||
| #!/usr/bin/env python3 | ||
| """Require one chunk PR to land its durable state projections atomically.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import re | ||
| import subprocess | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[1] | ||
| CHUNK_PATH = re.compile(r"^\.agent-loop/initiatives/([^/]+)/chunks/([^/]+)\.md$") | ||
| OUTCOME = re.compile(r"^- Outcome on merge: `(planned|complete|cancelled|superseded)`\s*$") | ||
| MERGE_STATE_SECTION = re.compile( | ||
| r"^## Merge state\s*$\n(?P<body>.*?)(?=^##\s|\Z)", | ||
| re.MULTILINE | re.DOTALL, | ||
| ) | ||
| CHUNK_ID = re.compile(r"^([A-Z]+-[A-Z]+-[0-9]+-[A-Z0-9]+)(?:-|$)") | ||
| IMPLEMENTATION_PREFIXES = ( | ||
| ".ci/", | ||
| ".github/workflows/", | ||
| "backend/", | ||
| "frontend/src/", | ||
| "scripts/", | ||
| ) | ||
| OUTCOME_WORDS = { | ||
| "planned": ("planned", "proposed"), | ||
| "complete": ("complete", "merged"), | ||
| "cancelled": ("cancelled",), | ||
| "superseded": ("superseded",), | ||
| } | ||
| REVIEW_ONLY_WORDS = ("in review", "pending review", "ready for review") | ||
|
|
||
|
|
||
| class ChunkStateError(RuntimeError): | ||
| """Raised when a PR would merge stale or incomplete chunk state.""" | ||
|
|
||
|
|
||
| def changed_paths(base_ref: str, head_ref: str = "HEAD") -> list[str]: | ||
| """Return paths changed by the prospective merge.""" | ||
| result = subprocess.run( | ||
| ["git", "diff", "--name-only", f"{base_ref}...{head_ref}"], | ||
| cwd=ROOT, | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| return [line for line in result.stdout.splitlines() if line] | ||
|
Comment on lines
+40
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Use async subprocess execution.
Replace it with As per coding guidelines: “Execution is async-first; do not document or implement synchronous-first checkers or jobs.” 🧰 Tools🪛 ast-grep (0.45.1)[error] 37-43: Command coming from incoming request (subprocess-from-request) 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
|
|
||
| def _read(relative_path: str) -> str: | ||
| try: | ||
| return (ROOT / relative_path).read_text(encoding="utf-8") | ||
| except (OSError, UnicodeError) as exc: | ||
| raise ChunkStateError(f"CHUNK_STATE_UNREADABLE: {relative_path}") from exc | ||
|
|
||
|
|
||
| def _chunk_row(chunk_map: str, chunk_id: str) -> str: | ||
| rows = [line for line in chunk_map.splitlines() if f"`{chunk_id}`" in line] | ||
| if len(rows) != 1: | ||
| raise ChunkStateError(f"CHUNK_STATE_MAP_ROW_INVALID: {chunk_id}") | ||
| return rows[0] | ||
|
|
||
|
|
||
| def _projection_lines(projection: str, chunk_id: str) -> list[str]: | ||
| identifier = re.compile(rf"(?<![A-Z0-9-]){re.escape(chunk_id)}(?![A-Z0-9-])") | ||
| return [line for line in projection.splitlines() if identifier.search(line)] | ||
|
|
||
|
|
||
| def _declared_outcome(contract: str, chunk_id: str) -> str: | ||
| sections = list(MERGE_STATE_SECTION.finditer(contract)) | ||
| declarations = [ | ||
| match | ||
| for line in contract.splitlines() | ||
| if (match := OUTCOME.fullmatch(line)) is not None | ||
| ] | ||
| if len(sections) != 1 or len(declarations) != 1: | ||
| raise ChunkStateError(f"CHUNK_STATE_OUTCOME_INVALID: {chunk_id}") | ||
| section_declarations = [ | ||
| match | ||
| for line in sections[0].group("body").splitlines() | ||
| if (match := OUTCOME.fullmatch(line)) is not None | ||
| ] | ||
| if len(section_declarations) != 1: | ||
| raise ChunkStateError(f"CHUNK_STATE_OUTCOME_INVALID: {chunk_id}") | ||
| return section_declarations[0].group(1) | ||
|
|
||
|
|
||
| def _has_outcome(line: str, outcome: str) -> bool: | ||
| """Return whether a line asserts, rather than merely contains, an outcome.""" | ||
| folded = line.casefold() | ||
| for word in OUTCOME_WORDS[outcome]: | ||
| token = re.compile(rf"(?<![a-z0-9_]){re.escape(word)}(?![a-z0-9_])") | ||
| for match in token.finditer(folded): | ||
| prefix = folded[max(0, match.start() - 32) : match.start()] | ||
| if re.search(r"\b(?:not(?:\s+yet)?|never)\s+$", prefix): | ||
| continue | ||
| return True | ||
| return False | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| def _requires_contract(paths: set[str]) -> bool: | ||
| return any(path.startswith(IMPLEMENTATION_PREFIXES) for path in paths) | ||
|
|
||
|
|
||
| def _validate_chunk(chunk_path: str, changed: set[str]) -> str: | ||
| """Validate one changed contract and return its declared merge outcome.""" | ||
| match = CHUNK_PATH.fullmatch(chunk_path) | ||
| assert match is not None | ||
| initiative_directory, chunk_filename = match.groups() | ||
| chunk_id_match = CHUNK_ID.match(chunk_filename) | ||
| if chunk_id_match is None: | ||
| raise ChunkStateError(f"CHUNK_STATE_ID_INVALID: {chunk_filename}") | ||
| chunk_id = chunk_id_match.group(1) | ||
| contract = _read(chunk_path) | ||
| outcome = _declared_outcome(contract, chunk_id) | ||
|
|
||
| initiative_root = f".agent-loop/initiatives/{initiative_directory}" | ||
| chunk_map_path = f"{initiative_root}/CHUNK_MAP.md" | ||
| status_path = f"{initiative_root}/STATUS.md" | ||
| current_state_path = ".agent-loop/CURRENT_STATE.md" | ||
| required = {chunk_map_path, status_path, current_state_path} | ||
| missing = sorted(required - changed) | ||
| if missing: | ||
| raise ChunkStateError("CHUNK_STATE_PROJECTION_MISSING: " + ", ".join(missing)) | ||
|
|
||
| chunk_map = _read(chunk_map_path) | ||
| status = _read(status_path) | ||
| current_state = _read(current_state_path) | ||
| row = _chunk_row(chunk_map, chunk_id) | ||
| if not _has_outcome(row, outcome): | ||
| raise ChunkStateError(f"CHUNK_STATE_MAP_OUTCOME_MISMATCH: {chunk_id}") | ||
| if outcome == "complete" and any(word in row.casefold() for word in REVIEW_ONLY_WORDS): | ||
| raise ChunkStateError(f"CHUNK_STATE_REVIEW_WORDING: {chunk_id}") | ||
| for projection_path, projection in ( | ||
| (status_path, status), | ||
| (current_state_path, current_state), | ||
| ): | ||
| lines = _projection_lines(projection, chunk_id) | ||
| if not lines: | ||
| raise ChunkStateError(f"CHUNK_STATE_ID_MISSING: {projection_path}: {chunk_id}") | ||
| if not any(_has_outcome(line, outcome) for line in lines): | ||
| raise ChunkStateError(f"CHUNK_STATE_OUTCOME_MISMATCH: {projection_path}: {chunk_id}") | ||
| return outcome | ||
|
|
||
|
|
||
| def validate(paths: list[str]) -> None: | ||
| """Validate atomic state for planning contracts or one implementation chunk.""" | ||
| changed = set(paths) | ||
| chunk_paths = sorted(path for path in changed if CHUNK_PATH.fullmatch(path)) | ||
| implementation = _requires_contract(changed) | ||
| if implementation and not chunk_paths: | ||
| raise ChunkStateError("CHUNK_STATE_CONTRACT_MISSING") | ||
| if implementation and len(chunk_paths) > 1: | ||
| raise ChunkStateError("CHUNK_STATE_MULTIPLE_CONTRACTS") | ||
| outcomes = [_validate_chunk(chunk_path, changed) for chunk_path in chunk_paths] | ||
| if len(outcomes) > 1 and any(outcome != "planned" for outcome in outcomes): | ||
| raise ChunkStateError("CHUNK_STATE_MULTIPLE_FINAL_OUTCOMES") | ||
|
|
||
|
|
||
| def main() -> int: | ||
| parser = argparse.ArgumentParser() | ||
| parser.add_argument("--base-ref", required=True) | ||
| parser.add_argument("--head-ref", default="HEAD") | ||
| args = parser.parse_args() | ||
| try: | ||
| validate(changed_paths(args.base_ref, args.head_ref)) | ||
| except (ChunkStateError, subprocess.CalledProcessError) as exc: | ||
| print(str(exc), file=sys.stderr) | ||
| return 1 | ||
| print("Atomic chunk state check passed.") | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main()) | ||
Uh oh!
There was an error while loading. Please reload this page.