zero repo-info: local (network-free) repository characterizer - #150
Conversation
git applies -1 before --reverse, so the old call returned the LATEST commit (age always ~0). Use --max-parents=0 (root commits) and take the oldest.
…tests Adversarial-review fixes: - DirectoryCount now counts every directory on the path to a file (expand each file's ancestors), so directories holding only subdirectories are included. git ls-tree -r lists blobs only, so the previous parent-of-file count undercounted. Verified against 'git ls-tree -r -t | awk $2==tree'. - ls-tree now uses -z (NUL-terminated, unquoted paths) instead of newline scanning, so non-ASCII/special filenames (which git C-quotes by default) are parsed correctly instead of being silently dropped. Drops the bufio scanner. - Tests: assert DirectoryCount/MaxDepth; add an age-from-oldest-root-commit test (would catch the old log --reverse -1 bug); make the CLI JSON test hermetic via a temp git repo.
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughImplements a new ChangesRepository Info Command
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as zero (runRepoInfo)
participant Collect as repoinfo.Collect
participant Git as git
participant Formatter as formatRepoInfo
User->>CLI: zero repo-info [--json|-C/--cwd]
CLI->>Collect: Collect(ctx, opts)
Collect->>Git: ls-tree -r -z HEAD
Git-->>Collect: NUL-separated entries
Collect->>Git: rev-parse, remote get-url, logs (oldest/90d/30d)
Collect-->>CLI: repoinfo.Info
CLI->>Formatter: format (json/text)
Formatter-->>User: stdout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Verdict: request changes.
Finding:
- P1
internal/repoinfo/repoinfo.go:200:zero repo-infostores and returnsgit remote get-url originverbatim, andinternal/cli/repoinfo.go:120prints it verbatim. Git remote URLs can include credentials, for examplehttps://x-access-token:ghp_...@github.com/org/repo.gitor token-as-user forms. That would leak secrets in both text output and--jsonviaremoteURL. Please sanitize the remote before storing/outputting it, or omit userinfo entirely. Add tests for both text and JSON output so credentials never appear.
Validation run locally on the PR worktree:
git diff --check 2ea36cf138e559af947b58e360d8f0d8fc8bdf80...HEADgo test ./internal/repoinfo ./internal/cligo test ./...
All validation passed, but the remote URL credential exposure should be fixed before merge.
Addresses #150 review (@Vasanthdev2004 P1): a git remote can embed secrets (e.g. https://x-access-token:ghp_...@github.com/o/r.git), which would leak in both text and --json output. sanitizeRemoteURL strips userinfo from URL forms and the leading user@ from scp-like forms. Tests: unit table for the sanitizer, Collect-level strip, and end-to-end CLI text + JSON assertions that the credential never appears.
|
@Vasanthdev2004 Fixed in 625a3fa. The remote URL is now sanitized before it's stored or printed: Tests added:
Full suite + -race + Windows build green. |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Requesting changes.
The repo-info implementation is additive in intent, but this branch is not additive against current main: it removes the notification feature that already exists on main.
Concrete regression:
internal/notify/notify.goandinternal/notify/notify_test.goare deleted.internal/cli/exec.godrops completion notification emission andexecNotifyMode.internal/cli/exec_parse.godrops--notify/--no-notifyparsing and validation.internal/config/types.go/resolver.godropNotifyConfigresolution.internal/tui/model.go,options.go, andrun.godrop notifier setup, focus reporting, completion notifications, and awaiting-input notifications.internal/cli/app.goalso removes the--notify/--no-notifyhelp text and stops passingresolved.Notifyinto TUI options.
This looks like branch drift from an older base, not part of the repo-info feature. Please rebase/merge latest main and restore the notification files/wiring, leaving only the repo-info command/package changes in this PR.
CI is green because the tests for the removed feature are also deleted, so this needs to be fixed before merge.
|
@Vasanthdev2004 Both addressed:
|
|
@Vasanthdev2004 both points are addressed (pushed at 8e186ef):
build / tests / Windows green. Re-requesting review. |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Verdict: approve. I re-reviewed the latest head after the merge from main. The previous blockers are addressed: notification files/wiring are no longer removed, and remote URLs are sanitized before text/JSON repo-info output. Validation in an isolated worktree passed: git diff --check origin/main...HEAD, go test ./internal/repoinfo ./internal/cli, go build ./cmd/zero, go test ./..., and go run ./cmd/zero repo-info --json. No blockers found.
Summary
Adds
zero repo-info [--json] [--cwd <dir>]— a repository characterizer that reads local git only (no network). Dep-free, additive (a brand-new command + package; nothing else changes).Reports: file / directory / max-depth / estimated-LOC counts; per-language breakdown (LOC + file counts) with primary language; workspace type + package count; build / test / CI tooling; and bounded git-history metrics (branch, remote URL, age, contributors-90d, commit-velocity-30d).
This is the network-free residue of the upstream telemetry module — the OTEL/metrics-egress half is intentionally not ported.
Design
internal/repoinfo(new, stdlib-only):Collect(ctx, Options{Cwd, Now, RunGit}) (Info, error)behind an injectableRunGit(default shells localgit). Streamsgit ls-tree -r -l -z HEAD, plusrev-parse/remote get-url origin/log/rev-list.languages.go(extension→language map) anddetect.go(build/test/CI + workspace tables) are pure helpers.internal/cli/repoinfo.go:runRepoInfoformats text or--json; dispatchcase "repo-info"inapp.go+ help line.Guarantees
ls-tree,rev-parse,remote get-url,log,rev-listare ever invoked; manifest reads are localos.ReadFile. Enforced by a test that records every subcommand against an allowlist.-z(NUL-terminated, unquoted paths) so non-ASCII/special filenames aren't dropped; gitlinks (size-) skipped;DirectoryCountcounts passthrough directories (expands each file's ancestors), verified againstgit ls-tree -r -t.log --max-parents=0(NOTlog --reverse -1, which returns the latest commit); oldest root chosen for multi-root repos.--max-countbounds history walking.Test Plan
go build ./.../GOOS=windows GOARCH=amd64 go build ./...cleango vet ./...clean;gofmtcleango test ./...green (repoinfo: parse/languages/detect/age/contributors/network-free; cli: parse/format/hermetic-JSON)go test -race ./internal/{repoinfo,cli}/...greenzero repo-info/--jsonon this repo (Go primary, DirectoryCount matchesgit ls-tree -r -t, real age)core.quotePathfilename corruptionReviewer focus
-zparsing + ancestor-based DirectoryCount.Summary by CodeRabbit
New Features
Tests