Skip to content

Add safe streamed prune inspection#7

Merged
flyingrobots merged 3 commits into
mainfrom
feat/safe-prune-inspection
Jul 13, 2026
Merged

Add safe streamed prune inspection#7
flyingrobots merged 3 commits into
mainfrom
feat/safe-prune-inspection

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Jul 13, 2026

Copy link
Copy Markdown
Member

Summary

  • add GitPlumbing.inspectPrunableObjects() with a canonical UTC cutoff
  • construct exact git prune --dry-run --verbose --no-progress --expire=<cutoff> argv
  • admit generic git prune only when a dry-run flag appears before end-of-options
  • replace the lossy sanitizer cache key and validate before memoization
  • prove the candidate stream is non-mutating across Node, Bun, and Deno

Why

git-cas repository diagnostics need Git's own age-aware loose-object classification without exposing a destructive prune path. This is the narrow upstream prerequisite for git-stunts/git-cas#49 and git-stunts/git-cas#55.

Verification

  • npm run lint
  • npm test (Node, Bun, Deno; Deno 210 steps)
  • Graft structural review: additive class method; no removals or breaking signature changes
  • CodeRabbit finding fixed in 4fe6575; all threads resolved

Closes #6

@flyingrobots

Copy link
Copy Markdown
Member Author

Self-review

Result: no blocking findings.

Reviewed the complete main..8519a84 diff with emphasis on command safety, stream lifecycle, and cross-runtime behavior. The sanitizer requires --dry-run/-n before --, rejects every other prune flag outside the narrow allowlist, and uses an exact validated-argument memoization key. The public method canonicalizes its cutoff before process execution. The synthetic-repository test observes the candidate and then proves cat-file can still read it.

Validation:

  • lint clean
  • Node: 173 tests pass
  • Bun: 173 tests pass
  • Deno: 22 suites / 209 steps pass
  • Graft: no removals or breaking signatures

Residual limitation is deliberate: this streams Git's loose-object dry-run candidates; packed-object and higher-level reachability reconciliation belong to the consuming diagnostics layer.

@flyingrobots

Copy link
Copy Markdown
Member Author

Code Lawyer

API contract

  • inspectPrunableObjects({ expiresBefore }) is additive and therefore targets v3.1.0.
  • expiresBefore accepts only canonical millisecond UTC timestamps and invalid/omitted values fail as InvalidArgumentError before runner invocation.
  • the result is the existing GitStream; callers retain the established obligation to consume it and inspect stream.finished.

Safety contract

  • the constructed command always contains --dry-run --verbose.
  • generic execution admits prune only with exact --dry-run or -n before end-of-options.
  • destructive forms, post--- lookalikes, and unapproved flags are rejected.
  • the integration witness verifies inspection leaves the object present.

Claim boundary

  • “prunable” means the loose unreachable records reported by Git for the supplied expiry cutoff. It does not claim packed-object collectibility, reflog reconciliation, or ownership attribution. Those remain explicit responsibilities of git-cas#49/#55.

No contract ambiguity or release-blocking issue found.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a validated inspectPrunableObjects streaming API for Git prune previews, enforces dry-run-only prune sanitization, fixes sanitizer cache-key collisions, and adds tests and documentation.

Changes

Prune inspection and command safety

Layer / File(s) Summary
Prune command safety and memoization
src/domain/services/CommandSanitizer.js, test/domain/services/CommandSanitizer.test.js
Adds prune allowlisting and dry-run enforcement, validates arguments before caching, uses complete argument arrays as cache keys, and tests destructive-form rejection.
Validated streamed prune inspection
index.js, test/PruneInspection.test.js
Validates canonical UTC expiry timestamps and streams git prune --dry-run --verbose --no-progress output with the normalized expiry; tests output, command construction, non-mutation, and invalid inputs.
Usage documentation and test registration
README.md, CHANGELOG.md, test/deno_entry.js
Documents the non-mutating inspection API and sanitizer fix, and includes the inspection suite in the Deno test process.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant GitPlumbing
  participant CommandSanitizer
  participant GitStream
  Caller->>GitPlumbing: inspectPrunableObjects(expiresBefore)
  GitPlumbing->>CommandSanitizer: validate prune dry-run command
  GitPlumbing->>GitStream: executeStream with normalized expiry
  GitStream-->>Caller: stream prunable objects
Loading

Possibly related PRs

Poem

A bunny inspects each unreachable byte,
With dry-run hops and no files in flight.
UTC carrots mark the expiry line,
Safe cache keys keep commands fine.
Stream the objects—then rest tonight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR meets issue #6 by adding a streamed dry-run prune inspection, validating expiry, rejecting destructive prune, and testing non-mutation.
Out of Scope Changes check ✅ Passed The changes stay focused on the requested inspection API, sanitizer updates, docs, and tests without unrelated scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly matches the main change: adding a safe streamed prune inspection path.
Description check ✅ Passed The description is directly about the new prune inspection API, sanitizer changes, and tests.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@CHANGELOG.md`:
- Around line 8-17: Add a blank line immediately after the “### Added” and “###
Fixed” headings in CHANGELOG.md, before each corresponding list, to satisfy
Markdown heading-spacing requirements.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e7a7399b-0a9b-431e-96fe-2da4ab7f9027

📥 Commits

Reviewing files that changed from the base of the PR and between 0e0f155 and 8519a84.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • README.md
  • index.js
  • src/domain/services/CommandSanitizer.js
  • test/PruneInspection.test.js
  • test/deno_entry.js
  • test/domain/services/CommandSanitizer.test.js
📜 Review details
🧰 Additional context used
🪛 markdownlint-cli2 (0.23.0)
CHANGELOG.md

[warning] 10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🔇 Additional comments (7)
src/domain/services/CommandSanitizer.js (1)

47-49: LGTM!

Also applies to: 108-130, 213-237, 245-248, 278-279, 289-289

index.js (2)

31-51: LGTM!


223-236: 🎯 Functional Correctness

--no-progress is not a valid git prune flag. git prune supports --progress; the current command and sanitizer are not missing a real option here.

			> Likely an incorrect or invalid review comment.
test/PruneInspection.test.js (1)

1-52: LGTM!

test/domain/services/CommandSanitizer.test.js (1)

24-50: LGTM!

Also applies to: 241-241

test/deno_entry.js (1)

9-9: LGTM!

README.md (1)

77-93: LGTM!

Comment thread CHANGELOG.md
@flyingrobots

Copy link
Copy Markdown
Member Author

Self-review update

Result: no blocking findings after b0fbde2.

The issue-to-code audit found and corrected one pre-merge contract mismatch: #6 requires progress suppression, so the public method now constructs the exact prune --dry-run --verbose --no-progress --expire=<cutoff> argv. A runner-level test captures that complete invocation, while the synthetic repository test still proves the candidate remains readable afterward. The sanitizer admits --no-progress but continues to reject --progress, destructive forms, and end-of-options dry-run lookalikes.

Validation remains clean: lint, Node, Bun, and Deno (210 steps). No residual issue/implementation mismatch found.

@flyingrobots

Copy link
Copy Markdown
Member Author

Code Lawyer update

Issue contract: --no-progress is now an explicit, tested part of the public command construction, matching #6 exactly.

Mutation contract: the only admitted prune forms still require exact --dry-run or -n before --; the public facade always chooses --dry-run. The integration witness confirms the reported loose object survives inspection.

Release contract: the change remains additive and belongs in v3.1.0; no existing value export or method signature is removed or altered.

No contract ambiguity or release-blocking issue remains.

@flyingrobots flyingrobots merged commit 4a9c8f3 into main Jul 13, 2026
3 checks passed
@flyingrobots flyingrobots deleted the feat/safe-prune-inspection branch July 13, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose a safe streamed dry-run prune inspection

1 participant