feat: bound STATUS.md history with status_history_keep keep-last-N prune - #26
Merged
Conversation
WalkthroughAdds an opt-in ChangesSTATUS.md history pruning
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ardd-init-update
participant constitution
participant ardd-status
participant status-prune.sh
participant STATUS.md
User->>ardd-init-update: choose status_history_keep
ardd-init-update->>constitution: stamp status_history_keep N
ardd-status->>STATUS.md: prepend _Updated block
ardd-status->>status-prune.sh: prune STATUS.md with --keep N
status-prune.sh->>STATUS.md: retain newest N blocks verbatim
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
moui72
marked this pull request as ready for review
July 24, 2026 21:33
added 6 commits
July 24, 2026 22:16
…date wiring (T004-T008)
…rune-history (T014-T015)
moui72
force-pushed
the
feat/status-md-trim-prune
branch
from
July 25, 2026 02:16
e49983e to
217a4a7
Compare
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 @.github/workflows/lint.yml:
- Around line 69-73: Update the status-prune job in the workflow to grant only
contents read permission, and configure its actions/checkout step with
persist-credentials disabled before running test-status-prune.sh.
In `@CLAUDE.md`:
- Around line 48-49: Add status_history_keep to the exact frontmatter-field
inventory in CLAUDE.md and to the corresponding ardd-state.sh stamp inventory in
docs/reference/scripts.md, keeping both public lists synchronized.
In `@docs/reference/scripts.md`:
- Around line 107-110: Update the pruning script’s rewrite flow to create a
secure temporary file in the target file’s directory, register cleanup on exit,
and avoid the predictable $file.prune.$$ path. Ensure failures from generating
or writing the pruned content stop processing and report pruned=false; emit
pruned=true only after the temporary file is successfully renamed into place.
In `@scripts/ardd-state.sh`:
- Around line 391-396: Use the same range-safe positive-integer validation
contract as status-prune.sh for status_history_keep, rejecting digit strings
outside the consumer’s supported arithmetic range. Apply this change in
scripts/ardd-state.sh lines 391-396 within the status_history_keep case and
scripts/lint-project.sh lines 206-216, keeping both validators identical so lint
and stamp accept only processable values.
In `@scripts/status-prune.sh`:
- Around line 77-78: Update the STATUS.md rewrite flow around the temporary
variable and the head/mv pipeline to use a collision-resistant temporary file,
register a cleanup trap for it, and explicitly handle failures from file
creation, head, and mv by returning nonzero. Ensure cleanup runs on both success
and failure, and never report success unless the rename completes.
In `@scripts/test-lint-project.sh`:
- Around line 118-127: Update scripts/test-lint-project.sh to create one per-run
temporary directory with mktemp -d, register a cleanup trap, and store all
captured lint outputs there instead of using the predictable /tmp/lint-bad.out
path. Update the assertions around the bad-project output to reference the
run-specific file while preserving existing cleanup behavior.
In `@skills/ardd-update/SKILL.md`:
- Around line 262-265: Update the reconfigure prompt text near the
status_history_keep description to remove the extra closing parenthesis, so it
ends with “leave it unbounded)” while preserving the surrounding wording and
prompt semantics.
- Around line 262-271: Update the reconfigure workflow and its handling of
status_history_keep so choosing “keep the full chronology” removes an existing
status_history_keep field instead of leaving the previous value. Add or use an
unset/removal path alongside ardd-state.sh stamp, while preserving stamping for
positive integer selections and leaving unrelated fields unchanged.
- Around line 232-244: Make invalid status_history_keep values behave
consistently across all configuration paths: in skills/ardd-update/SKILL.md
lines 232-244, validate the value before treating field presence as suppressing
backfill; in skills/ardd-status/SKILL.md lines 319-323, treat invalid values as
absent or use the documented refusal path; and in
docs/reference/configuration.md lines 137-154, document the workflow and pruner
behavior actually implemented.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b55b4721-89cd-47d5-ac08-c7c08c5beaf7
📒 Files selected for processing (19)
.github/workflows/lint.yml.project/features/status-md-trim-prune-history.md.project/plans/plan-status-md-trim-prune-history-2026-07-24-1038.md.project/plans/research-status-md-trim-prune-history-2026-07-24-e8a4.md.project/tasks/tasks-status-md-trim-prune-history-485d.mdCLAUDE.mddocs/reference/configuration.mddocs/reference/scripts.mdinstall.shscripts/ardd-state.shscripts/lint-project.shscripts/status-prune.shscripts/test-lint-project.shscripts/test-status-prune.shskills/ardd-init/SKILL.mdskills/ardd-status/SKILL.mdskills/ardd-update/SKILL.mdtests/fixtures/bad-project/.project/artifacts/constitution.mdtests/fixtures/good-project/.project/artifacts/constitution.md
- status-prune.sh fails closed: mktemp temp file in the target's own directory, cleanup trap, explicit nonzero handling on head/mv — pruned=true only after the rename lands (reason=write-failed otherwise) - one range-safe contract across all three validators: status_history_keep is a positive integer of at most 4 digits in ardd-state.sh stamp, lint-project.sh, and status-prune.sh --keep (shell arithmetic stays safe) - ardd-state.sh gains 'unstamp <file> <field>' (allowlisted optional fields, frontmatter-scoped removal) — the scripted way back to unbounded history; ardd-update --reconfigure wires it for status_history_keep - invalid status_history_keep contract made explicit and consistent: ardd-status treats it as absent (skip prune, note in report), ardd-update backfill re-asks instead of letting garbage suppress it - test-lint-project.sh captures output in a per-run mktemp -d dir - stamp inventory in docs/reference/scripts.md gains status_history_keep + unstamp; malformed '))' prompt fixed - new regression cases: >4-digit keep refusals (status-prune + stamp), unstamp removal/no-op/body-line-preserved/non-removable Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/test-lint-project.sh (1)
123-132: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a fixture case for the four-digit maximum.
This only verifies
-3. Add a bad-project fixture/assertion forstatus_history_keep: 10000so lint’s new upper-bound contract cannot regress.As per coding guidelines, every deterministic check must have a fixture-based regression test, and new checks must add their CI job and regression test in the same commit.
🤖 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-lint-project.sh` around lines 123 - 132, Add a bad-project fixture setting status_history_keep to 10000 and extend the assertions in the test-lint-project.sh regression checks to require the field name, value, and allowed-range message for this four-digit upper-bound violation, while preserving the existing -3 and good-project coverage.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@scripts/test-lint-project.sh`:
- Around line 123-132: Add a bad-project fixture setting status_history_keep to
10000 and extend the assertions in the test-lint-project.sh regression checks to
require the field name, value, and allowed-range message for this four-digit
upper-bound violation, while preserving the existing -3 and good-project
coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ad48b2b8-c72a-417c-8145-74a8925b733c
📒 Files selected for processing (10)
docs/reference/configuration.mddocs/reference/scripts.mdscripts/ardd-state.shscripts/lint-project.shscripts/status-prune.shscripts/test-ardd-state.shscripts/test-lint-project.shscripts/test-status-prune.shskills/ardd-status/SKILL.mdskills/ardd-update/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (7)
- scripts/lint-project.sh
- docs/reference/configuration.md
- docs/reference/scripts.md
- skills/ardd-status/SKILL.md
- skills/ardd-update/SKILL.md
- scripts/status-prune.sh
- scripts/test-status-prune.sh
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.
Gives
/ardd-statusa deterministic, opt-in keep-last-N prune of STATUS.md's_Updated:chronology, so the live file stays slim in long-running projects. Recent blocks stay verbatim; older history stays recoverable from git.Vetted first in
.project/plans/research-status-md-trim-prune-history-2026-07-24-e8a4.md(the key insight: STATUS.md is git-committed, so "durable re-entry chronology" never required unbounded file length — git backs it).What changed
scripts/status-prune.sh <file> --keep <N>— POSIX tail-cut: preserves head matter + newest N_Updated:blocks byte-for-byte, drops the older tail, refuses (never corrupts) on a missing file or non-positive N. Never summarizes — the only edit it makes is removing whole older blocks. +scripts/test-status-prune.sh+ a CI job.status_history_keep: <N>(absent = unbounded, so existing installs are unchanged): validated bylint-project.sh(+ good/bad fixtures), accepted byardd-state.sh stamp, asked by/ardd-init, backfilled/re-asked by/ardd-update./ardd-statusstep 6: runs the prune after each prepend when the field is set. The prepend-and-preserve invariant is narrowed — never-summarize stays absolute; unbounded-retention relaxes to keep-last-N.install.shships the script;CLAUDE.md+docs/reference/{configuration,scripts}.mdsynced.Design notes
STATUS-archive.mdfile (redundant with git), age-based caps (fragile for bursty/dormant projects), a separate/ardd-pruneskill (breaks single-writer ownership).All gates green:
lint-docs,lint-project,test-status-prune,test-lint-project,test-install-manifest-complete.🤖 Generated with Claude Code
Summary by CodeRabbit
status_history_keepto bound.project/STATUS.mdby retaining only the newest N_Updated:blocks.scripts/status-prune.shto deterministically prune older history while preserving kept blocks./ardd-init,/ardd-update, and/ardd-statusnow support the retention option.status_history_keepvalues.stamp/unstamp, plus CI coverage (status-prune job).