Mark loop bootstrap merged and guard main memory#24
Merged
Conversation
📝 WalkthroughWalkthroughUpdates four ChangesLoop Memory Validator and Post-Merge State Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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: 1
🧹 Nitpick comments (1)
scripts/test_agent_gates.py (1)
687-754: ⚡ Quick winAdd a regression for missing initiative
STATUS.mdPlease add a fixture test where
.agent-loop/initiatives/<name>/exists butSTATUS.mdis absent, and assertchecker.main() == 1. This prevents silent contract drift in the loop-memory guard.🤖 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/test_agent_gates.py` around lines 687 - 754, Add a new test function following the same pattern as test_loop_memory_state_rejects_pre_merge_status and test_loop_memory_state_accepts_merged_fixture that validates the checker correctly handles the case where the initiatives directory exists but the STATUS.md file is missing. Create the .agent-loop/initiatives/example directory structure and write the necessary state files (LOOP_STATE.md, WORK_QUEUE.md, REVIEW_LOG.md) similar to the existing tests, but deliberately omit creating the STATUS.md file in the initiatives/example subdirectory. Configure checker.INITIATIVE_STATUS_FILES to point to this missing file, call checker.main(), and assert it returns 1 to verify the checker properly detects and rejects this missing file condition, then restore the original checker state in the finally block.
🤖 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 `@scripts/check_loop_memory_state.py`:
- Around line 16-19: The INITIATIVE_STATUS_FILES variable uses a glob pattern to
only capture existing STATUS.md files, which silently skips any initiative
folders that are missing the required STATUS.md file. To fix this, detect
initiative folders in the .agent-loop/initiatives directory and explicitly
verify that each folder contains a STATUS.md file, raising an error or failing
validation if any are missing. Apply this same validation pattern to all similar
glob operations in the file that are mentioned in the related sections (lines
49-53 and 59-67) to ensure complete contract enforcement across all loop-memory
integrity checks.
---
Nitpick comments:
In `@scripts/test_agent_gates.py`:
- Around line 687-754: Add a new test function following the same pattern as
test_loop_memory_state_rejects_pre_merge_status and
test_loop_memory_state_accepts_merged_fixture that validates the checker
correctly handles the case where the initiatives directory exists but the
STATUS.md file is missing. Create the .agent-loop/initiatives/example directory
structure and write the necessary state files (LOOP_STATE.md, WORK_QUEUE.md,
REVIEW_LOG.md) similar to the existing tests, but deliberately omit creating the
STATUS.md file in the initiatives/example subdirectory. Configure
checker.INITIATIVE_STATUS_FILES to point to this missing file, call
checker.main(), and assert it returns 1 to verify the checker properly detects
and rejects this missing file condition, then restore the original checker state
in the finally block.
🪄 Autofix (Beta)
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: 06157862-1da5-49f3-b688-12f341c5acc4
📒 Files selected for processing (7)
.agent-loop/LOOP_STATE.md.agent-loop/REVIEW_LOG.md.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-ENG-001-codex-zero-trust-loop-bootstrap/STATUS.md.github/workflows/loop-memory.ymlscripts/check_loop_memory_state.pyscripts/test_agent_gates.py
Comment on lines
+16
to
+19
| INITIATIVE_STATUS_FILES = tuple( | ||
| str(path.relative_to(ROOT)) | ||
| for path in (ROOT / ".agent-loop/initiatives").glob("*/STATUS.md") | ||
| ) |
There was a problem hiding this comment.
Detect missing initiative STATUS.md files explicitly
Line 16–19 only captures existing */STATUS.md files, so an initiative folder missing STATUS.md is silently skipped and never fails validation. That leaves a contract hole in loop-memory integrity checks.
Proposed fix
-INITIATIVE_STATUS_FILES = tuple(
- str(path.relative_to(ROOT))
- for path in (ROOT / ".agent-loop/initiatives").glob("*/STATUS.md")
-)
+INITIATIVE_DIR = ROOT / ".agent-loop/initiatives"
+
+def initiative_status_paths() -> tuple[Path, ...]:
+ """Return expected STATUS.md paths for each initiative directory."""
+ if not INITIATIVE_DIR.exists():
+ return ()
+ return tuple(path / "STATUS.md" for path in INITIATIVE_DIR.iterdir() if path.is_dir())
@@
def checked_paths() -> list[Path]:
"""Return loop memory paths that must not contain pre-merge state."""
paths = [ROOT / path for path in CHECKED_FILES]
- paths.extend(ROOT / path for path in INITIATIVE_STATUS_FILES)
+ paths.extend(initiative_status_paths())
return pathsAlso applies to: 49-53, 59-67
🤖 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_loop_memory_state.py` around lines 16 - 19, The
INITIATIVE_STATUS_FILES variable uses a glob pattern to only capture existing
STATUS.md files, which silently skips any initiative folders that are missing
the required STATUS.md file. To fix this, detect initiative folders in the
.agent-loop/initiatives directory and explicitly verify that each folder
contains a STATUS.md file, raising an error or failing validation if any are
missing. Apply this same validation pattern to all similar glob operations in
the file that are mentioned in the related sections (lines 49-53 and 59-67) to
ensure complete contract enforcement across all loop-memory integrity checks.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-ENG-001-codex-zero-trust-loop-bootstrap/reviews/WS-ENG-001-post-merge-loop-memory-internal-review-evidence.md:
- Line 19: The statement on line 19 is ambiguous about what changed after the
reviewed SHA. Reword the sentence to clearly specify whether it means: (a) no
other files were modified after the review was completed, (b) this is the only
new evidence artifact created post-review despite other files being in the PR
scope, or (c) something related to the chronological review sequence. Make the
intent explicit and specific so readers understand exactly what the statement is
documenting about the scope and timeline of changes, rather than leaving room
for misinterpretation about which files were or were not modified.
- Line 39: The markdown file at line 39 references the workflow file path, and
the capitalization should be verified to match the actual file in the
repository. Ensure the filepath reference `.github/workflows/loop-memory.yml`
uses the correct casing—specifically that `.github/` (the reserved GitHub
directory) is in the exact casing as it exists in your repository, and the
complete path matches the actual file location where the loop-memory workflow is
stored.
🪄 Autofix (Beta)
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: 05f08728-4356-4a60-9ae3-5d82928cf249
📒 Files selected for processing (2)
.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-ENG-001-codex-zero-trust-loop-bootstrap/reviews/WS-ENG-001-post-merge-loop-memory-internal-review-evidence.md
✅ Files skipped from review due to trivial changes (1)
- .agent-loop/WORK_QUEUE.md
|
|
||
| - Local Workstream directory confusion: identified `/home/abiorh/flow/workstream` as a separate dirty feature branch, not `main`, and left unrelated checker/test changes untouched. | ||
| - Stale merged-loop memory: updated `.agent-loop/LOOP_STATE.md`, initiative `STATUS.md`, `WORK_QUEUE.md`, and `REVIEW_LOG.md` to reflect that PR #23 is merged. | ||
| - Missing main enforcement: added `.github/workflows/loop-memory.yml` so merged loop memory is checked on pushes to `main`. |
There was a problem hiding this comment.
Fix GitHub capitalization.
Line 39 uses lowercase "github" in the context of workflow files. The official product name is "GitHub" (capital G and H).
📝 Proposed fix
- Missing main enforcement: added `.github/workflows/loop-memory.yml` so merged lo...
+ Missing main enforcement: added `.github/workflows/loop-memory.yml` so merged lo...Actually, looking at the full line, the error is in the word "github" within the filename. Since .github/ is a reserved directory name in GitHub repositories (case-sensitive), it should be capitalized as .github/. The actual fix needed depends on context—if this is quoting a filename, verify the filename matches the actual file casing in the repository.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~39-~39: The official name of this software platform is spelled with a capital “H”.
Context: ...rged. - Missing main enforcement: added .github/workflows/loop-memory.yml so merged lo...
(GITHUB)
🤖 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
@.agent-loop/initiatives/WS-ENG-001-codex-zero-trust-loop-bootstrap/reviews/WS-ENG-001-post-merge-loop-memory-internal-review-evidence.md
at line 39, The markdown file at line 39 references the workflow file path, and
the capitalization should be verified to match the actual file in the
repository. Ensure the filepath reference `.github/workflows/loop-memory.yml`
uses the correct casing—specifically that `.github/` (the reserved GitHub
directory) is in the exact casing as it exists in your repository, and the
complete path matches the actual file location where the loop-memory workflow is
stored.
Source: Linters/SAST tools
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
1 participant
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.
Chunk
WS-ENG-001-01post-merge loop-memory hardening.Goal
Make the Codex engineering loop state on
mainreflect that PR #23 is merged, and add a main-branch guard so stale pre-merge loop memory does not remain after future merges.Intent
This is engineering-loop infrastructure around Workstream. It is not Workstream product/runtime functionality and does not change backend APIs, auth, task lifecycle behavior, database schema, or frontend product UI.
Design
scripts/check_loop_memory_state.pyto reject stale pre-merge phrases in loop memory onmain..github/workflows/loop-memory.ymlto run the guard on pushes tomainand manual dispatch.scripts/test_agent_gates.pyso PR pre-merge state is not blocked by the main-only guard.*-internal-review-evidence.md; external CodeRabbit/GitHub feedback remains separate.Validation
python3 scripts/check_internal_review_evidence.pypython3 scripts/check_loop_memory_state.pypython3 scripts/test_agent_gates.pypython3 scripts/check_markdown_links.pypython3 scripts/check_stale_workstream_wording.pypython3 -m py_compile scripts/check_loop_memory_state.py scripts/test_agent_gates.pygit diff --checkReview Evidence
Internal evidence:
.agent-loop/initiatives/WS-ENG-001-codex-zero-trust-loop-bootstrap/reviews/WS-ENG-001-post-merge-loop-memory-internal-review-evidence.mdRequired internal reviewer tracks ran and were closed: senior engineering, QA/test, security/auth, product/ops, architecture, docs, CI integrity, reuse/dedup, and test delta.
Current Status
Notes
/home/abiorh/flow/workstreamis a separate dirty worktree oncodex/submission-artifact-policy-docs. Those checker/revision testing changes are unrelated to this PR and were not modified here.