feat: add repository usage diagnostics#66
Conversation
📝 WalkthroughWalkthroughAdds a non-mutating repository inspection port and Git adapter, a repository-wide ChangesRepository diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ContentAddressableStore
participant RepositoryDoctor
participant GitRepositoryInspectionAdapter
Client->>ContentAddressableStore: diagnostics.doctor(options)
ContentAddressableStore->>RepositoryDoctor: doctor(options)
RepositoryDoctor->>GitRepositoryInspectionAdapter: stream repository evidence
GitRepositoryInspectionAdapter-->>RepositoryDoctor: objects, refs, reachability, prunable records
RepositoryDoctor-->>ContentAddressableStore: frozen diagnostic report
ContentAddressableStore-->>Client: repository and usage diagnostics
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Self-reviewVerdict: no blocking findings. Reviewed the complete branch against #49, #55, and API-0047 for destructive Git paths, ref/reflog reachability, grace-policy semantics, concurrent repository writes, bounded residency, collection-total completeness, shared-byte attribution, Vault privacy, facade initialization, declaration/runtime parity, and cross-runtime behavior. Key evidence:
Verification: Full source witness: |
Code Lawyer reviewDisposition: all identified ambiguity and adversarial cases are addressed; no blocking finding remains.
The committed witness records the complete argument and real-Git proof: |
Self-review follow-upThe final declaration-parity pass found and fixed one additive omission: runtime truncation limitations expose Focused declaration tests and the complete 1,866-test Node unit gate passed before push. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/domain/services/RepositoryDoctor.js`:
- Around line 136-163: Update unhealthyCollection and its callers in
`#inspectCache`, `#inspectRootSet`, and `#inspectExpiringSet` to produce kind-specific
error usage shapes matching the declared RepositoryCacheUsage,
RepositoryRootSetUsage, and RepositoryExpiringSetUsage contracts. Ensure
root-set errors omit cache-only fields, expiring-set errors omit retention and
other cache-only fields, and preserve the existing error details and
success-path helpers.
In `@src/infrastructure/adapters/GitRepositoryInspectionAdapter.js`:
- Line 6: Update the timeout used by reachablePhysicalBytes() for the full
rev-list inventory scan so it does not inherit the shared DEFAULT_POLICY
30-second limit. Assign this operation a dedicated longer timeout or use its
supported per-call override, while leaving DEFAULT_POLICY unchanged for other
operations.
In `@test/unit/infrastructure/adapters/GitRepositoryInspectionAdapter.test.js`:
- Around line 77-112: Add negative-path tests covering the validation branches
in iterateRefs and iteratePrunableObjects. Verify iterateRefs rejects streamed
lines without a refs/ prefix and iteratePrunableObjects rejects lines that do
not contain exactly two fields, asserting both failures expose code
REPOSITORY_INSPECTION_INVALID while preserving the existing successful-stream
tests.
🪄 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: e86b610f-807b-49ba-85c1-cc49f8227238
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
CHANGELOG.mdREADME.mddocs/API.mddocs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.mddocs/design/0047-application-storage-cache-boundary/witness/repository-diagnostics.mdindex.d.tsindex.jspackage.jsonsrc/domain/errors/Codes.jssrc/domain/services/RepositoryDoctor.jssrc/infrastructure/adapters/GitRepositoryInspectionAdapter.jssrc/ports/RepositoryInspectionPort.jstest/integration/repository-diagnostics.test.jstest/unit/domain/services/RepositoryDoctor.test.jstest/unit/facade/ContentAddressableStore.diagnostics.test.jstest/unit/infrastructure/adapters/GitRepositoryInspectionAdapter.test.jstest/unit/types/declaration-accuracy.test.js
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: test-docker (bun)
- GitHub Check: test-docker (deno)
- GitHub Check: test-docker (node)
🧰 Additional context used
📓 Path-based instructions (3)
docs/design/**
📄 CodeRabbit inference engine (AGENTS.md)
Use
docs/design/directory for durable design contracts and proof plans
Files:
docs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.mddocs/design/0047-application-storage-cache-boundary/witness/repository-diagnostics.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
README.mdas the public front door, core value prop, and quick start documentation
Files:
README.md
CHANGELOG.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
CHANGELOG.mdto record the historical truth of merged behavior
Files:
CHANGELOG.md
🧠 Learnings (3)
📚 Learning: 2026-02-28T19:21:13.982Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 15
File: test/unit/domain/services/CasService.envelope.test.js:326-330
Timestamp: 2026-02-28T19:21:13.982Z
Learning: In fuzz tests for cryptographic operations, use a seeded PRNG (e.g., xorshift32) for control-flow variables such as plaintext size selection and recipient index selection to ensure reproducibility. Continue to use randomBytes() for cryptographic keys and nonces to preserve realistic randomness and avoid security anti-patterns. This guideline applies to test files that perform fuzz testing of crypto logic; implement a consistent seed setup (e.g., fixed seed in test initialization) and document the rationale to enable deterministic replays across runs.
Applied to files:
test/unit/facade/ContentAddressableStore.diagnostics.test.jstest/integration/repository-diagnostics.test.jstest/unit/types/declaration-accuracy.test.jstest/unit/domain/services/RepositoryDoctor.test.jstest/unit/infrastructure/adapters/GitRepositoryInspectionAdapter.test.js
📚 Learning: 2026-07-13T17:00:46.222Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 65
File: src/domain/helpers/isCanonicalCollectionKey.js:21-35
Timestamp: 2026-07-13T17:00:46.222Z
Learning: In git-stunts/git-cas, the codebase is intended to run on multiple JavaScript runtimes (Node, Bun, and Deno), not just Node. During code review, don’t suggest replacing existing portable helper logic with newer Node-only built-ins (e.g., Node 20+ APIs like `String.prototype.isWellFormed()`) unless the change preserves Bun/Deno compatibility (via portable/polyfill implementations or safe runtime gating). If a file includes manual implementations (such as `src/domain/helpers/isCanonicalCollectionKey.js`), treat them as intentional to avoid requiring a higher host built-in baseline.
Applied to files:
src/domain/errors/Codes.jssrc/ports/RepositoryInspectionPort.jssrc/infrastructure/adapters/GitRepositoryInspectionAdapter.jssrc/domain/services/RepositoryDoctor.js
📚 Learning: 2026-03-30T19:53:48.000Z
Learnt from: flyingrobots
Repo: git-stunts/git-cas PR: 29
File: docs/design/TR-010-planning-index-consistency-review.md:121-131
Timestamp: 2026-03-30T19:53:48.000Z
Learning: In this repo, CHANGELOG.md entries should be user-facing and descriptive (bullet points) rather than cycle-ID headings (e.g., use a phrase like “Planning-index consistency review” instead of a “TR-010 — …” heading). When searching the changelog, don’t rely on grepping for cycle IDs (e.g., “TR-010”) because it may cause false negatives; search for the descriptive keywords/phrases from the bullet entries instead.
Applied to files:
CHANGELOG.md
🪛 ast-grep (0.44.1)
test/integration/repository-diagnostics.test.js
[warning] 4-4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process)
🔇 Additional comments (18)
CHANGELOG.md (1)
56-61: LGTM!Also applies to: 65-67
README.md (1)
107-107: LGTM!Also applies to: 138-141
docs/design/0047-application-storage-cache-boundary/application-storage-cache-boundary.md (1)
803-805: LGTM!docs/design/0047-application-storage-cache-boundary/witness/repository-diagnostics.md (1)
1-159: LGTM!test/unit/infrastructure/adapters/GitRepositoryInspectionAdapter.test.js (2)
34-58: LGTM!Also applies to: 60-75, 90-112
114-144: LGTM!test/unit/domain/services/RepositoryDoctor.test.js (1)
117-291: LGTM!test/unit/facade/ContentAddressableStore.diagnostics.test.js (1)
15-48: LGTM!test/integration/repository-diagnostics.test.js (1)
1-169: LGTM!test/unit/types/declaration-accuracy.test.js (1)
42-43: LGTM!Also applies to: 56-58, 100-108
index.d.ts (1)
531-555: LGTM!Also applies to: 1065-1249, 1361-1364, 1414-1414
docs/API.md (1)
20-29: LGTM!Also applies to: 1375-1377, 1688-1750, 2641-2665, 3082-3082
src/ports/RepositoryInspectionPort.js (1)
1-34: LGTM!src/infrastructure/adapters/GitRepositoryInspectionAdapter.js (1)
1-163: Remaining logic in this file (constructor validation,iterateObjects,iterateReachableObjectIds,iteratePrunableObjects,iterateRefs,#stream,consumeLines, and theparseOid/parseType/parseBytes/invalidOutputhelpers) is correct: git command syntax/order verified against git-cat-file/git-rev-list docs, structured-output parsing is strict and defensive, and streaming/error-propagation is consistent.package.json (1)
101-101: LGTM!src/domain/errors/Codes.js (1)
63-63: LGTM!src/domain/services/RepositoryDoctor.js (1)
1-24: Remaining logic (constructor/dependency validation,doctor()composition, object/usage inventory arithmetic with fail-closed nulling, success-path usage builders, coverage/truncation accounting, options validation, anddeepFreeze) is sound and matches the documented design precisely.Also applies to: 25-69, 71-134, 208-513, 532-626, 627-635
index.js (1)
19-26: LGTM!Also applies to: 72-75, 106-106, 194-196, 242-243, 306-320, 404-412
Review-fix verificationAll three CodeRabbit findings were valid and are fixed on the current branch:
The hardening push passed ESLint and all 1,869 Node unit tests. CodeRabbit has recognized each fix and automatically resolved all three review threads. |
Linked Issue
Design / Proof
cas.diagnostics.doctor()capability for repository object, reachability, and managed-storage evidence.@git-stunts/plumbingto 3.1.0.Validation
Release Impact