fix(ci): make chunk state atomic with merge - #319
Merged
Conversation
📝 WalkthroughWalkthroughThe PR defines atomic chunk-state contracts, adds a validator for synchronized state projections, integrates the check into Agent Gates, expands regression tests, and records completed initiative state. ChangesAtomic chunk-state enforcement
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant AgentGates
participant ChunkStateValidator
participant StateProjections
PullRequest->>AgentGates: trigger checks with base SHA
AgentGates->>ChunkStateValidator: validate changed chunk state
ChunkStateValidator->>StateProjections: compare chunk map, status, and current state
StateProjections-->>ChunkStateValidator: return projection results
ChunkStateValidator-->>AgentGates: return validation status
AgentGates-->>PullRequest: report gate result
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent-loop/initiatives/WS-CI-003-atomic-chunk-state/chunks/WS-CI-003-01-atomic-chunk-state.md:
- Around line 49-51: Update scripts/check_chunk_state_sync.py’s projection-state
validation to parse the state field or perform token-aware, negation-safe
matching rather than substring checks, so “complete” does not match “not
complete” or “incomplete.” Add regression tests covering these false positives
while preserving valid exact outcome matches.
- Line 46: Update scripts/check_chunk_state_sync.py to parse the canonical
“Outcome on merge” field only within the contract’s “## Merge state” section,
rejecting contracts with zero or multiple declarations instead of accepting the
first match. Add regression tests covering missing, duplicate, and
misplaced/conflicting outcome declarations while preserving valid single-outcome
contracts.
In `@scripts/check_chunk_state_sync.py`:
- Around line 62-64: Update _projection_lines to match chunk_id only as a
complete identifier using an identifier-boundary regular expression, preventing
longer IDs from satisfying shorter-ID checks. Add a regression test where the
projection contains only the longer identifier and assert that validation raises
CHUNK_STATE_ID_MISSING.
- Around line 36-45: Update changed_paths to be asynchronous and use
asyncio.create_subprocess_exec instead of subprocess.run, awaiting communicate
and preserving the existing changed-path parsing. Convert the checker entry
point to async and update main to run that async entry point.
- Around line 98-115: The outcome validation in the chunk-state checks must stop
using substring matching, which lets values such as “Incomplete” satisfy
“complete.” Update the checks around OUTCOME_WORDS in the chunk-map and
projection validation to match complete tokens or explicitly parse the state
field, while preserving valid outcome handling. Add regression coverage for
“Incomplete” and comparable non-outcome text.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 195d80bd-8841-4e65-b267-c1dc4059b73c
📒 Files selected for processing (13)
.agent-loop/CURRENT_STATE.md.agent-loop/initiatives/WS-CI-002-deterministic-agent-gates/STATUS.md.agent-loop/initiatives/WS-CI-003-atomic-chunk-state/CHUNK_MAP.md.agent-loop/initiatives/WS-CI-003-atomic-chunk-state/STATUS.md.agent-loop/initiatives/WS-CI-003-atomic-chunk-state/chunks/WS-CI-003-01-atomic-chunk-state.md.agent-loop/templates/PR_TRUST_BUNDLE.md.github/pull_request_template.md.github/workflows/agent-gates.ymlAGENTS.mdCONTRIBUTING.mdscripts/check_chunk_state_sync.pyscripts/test_chunk_state_sync.pyscripts/test_lightweight_agent_gates.py
Comment on lines
+36
to
+45
| 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] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use async subprocess execution.
changed_paths implements a synchronous-first checker with subprocess.run.
Replace it with asyncio.create_subprocess_exec. Make main run the async entry point.
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
Context: subprocess.run(
["git", "diff", "--name-only", f"{base_ref}...{head_ref}"],
cwd=ROOT,
check=True,
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check_chunk_state_sync.py` around lines 36 - 45, Update changed_paths
to be asynchronous and use asyncio.create_subprocess_exec instead of
subprocess.run, awaiting communicate and preserving the existing changed-path
parsing. Convert the checker entry point to async and update main to run that
async entry point.
Source: Coding guidelines
abiorh-claw
self-requested a review
August 11, 2026 17:42
abiorh-claw
approved these changes
Aug 11, 2026
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Workstream PR Trust Bundle
Chunk
WS-CI-003-01— Atomic Chunk StateGoal
Make human merge the only transition needed to complete a chunk: implementation
and its durable state land in the same PR.
Root cause
Chunk state was duplicated across contracts, chunk maps, initiative status, and
current state, but nothing required those files to describe the post-merge
outcome. PR #318 repaired earlier stale records and still landed its own HK1 row
as
In review.What changed
Outcome on merge.chunk and final outcome.
What did not change
Evidence
origin/main...HEAD.git diff --checkpassed.Human review focus
Confirm this makes chunk state atomic without introducing another workflow or
turning historical review evidence into authority.
Human merge ownership
Summary by CodeRabbit
New Features
Documentation
Tests