CLI: doctor config validation, autonomy ceiling, changes --base, usage report - #142
Merged
Conversation
added 28 commits
June 8, 2026 14:03
- Remove the osReadFile wrapper; call os.ReadFile directly at its one call site - Add config.ValidateBytes([]byte) so each config file is read and JSON-parsed once in configValidationCheck instead of twice (once for position, once via ValidateFile); ValidateFile now reads then delegates to its own unmarshal for the path-prefixed error message, keeping its public signature unchanged - Fix the misleading skip comment in configValidationCheck: configFilesCheck only checks path-string presence; the skip is a defensive guard for the unreachable case where DefaultResolveOptions passed a non-empty path that still fails to read - Add utf8 multibyte case to TestOffsetToLineCol documenting byte-column semantics (column counts bytes, not runes)
Evaluate now treats an empty Policy.MaxAutonomy (from a directly-constructed Policy that bypasses DefaultPolicy) as High instead of letting it normalize to Low, which would silently clamp every Medium/High decision to Prompt. The ceiling is a no-op unless explicitly configured. Also extracts the "above policy ceiling" clamp reason into a package const referenced at both engine clamp sites and the related tests, and adds a non-vacuous engine test proving the empty-ceiling trap is closed.
- redactChangeSummary now redacts the Base field alongside Root/Branch/Commit/etc. - --base= equals-form rejects leading-dash values via flagValueLooksLikeOption, closing the option-smuggling gap - TestParseNameStatusRenameAndCopy asserts rename/copy three-field lines use the new/destination path with correct status, and confirms two-field modify is unaffected
Fix 1 (BLOCKING): parseUsageArgs now validates --since with time.Parse
against YYYY-MM-DD; returns exitUsage + "invalid --since %q: expected
YYYY-MM-DD" for bare strings like "foo", unpadded "2026-6-1", or
wrong-separator "06/01/2026". Valid dates are stored unchanged for the
existing lexical comparison.
Fix 2: Replace false "pre-redaction stat" claims in diffstat.go and
usage.go with accurate comments: the --stat summary line carries no
secret-bearing tokens so parsing the already-redacted DiffStat is safe.
Fix 3: Add three tests to usage_test.go —
- TestRunUsageDaysFilter: stubs deps.now, seeds events on two dates,
asserts --days 3 includes the recent date and excludes the old one.
- TestRunUsageInvalidSince: table-driven; verifies "foo", "2026-6-1",
"06/01/2026" each return exitUsage + validation message, while
"2026-06-01" returns exitSuccess.
- TestRunUsageEmptyStore: zero usage events → exitSuccess, output
contains header and "total" row, no panic.
…l keys Probe config validation against config.FileConfig instead of a bare any so a structurally-valid document with a wrong field type (e.g. maxTurns as a string) surfaces a *json.UnmarshalTypeError carrying the offset, instead of losing line/col. Remove the flat top-level details["line"]/["col"] keys that were overwritten by the last malformed file when several were present; keep only the unambiguous per-path map entry. Tighten the existing malformed test to assert concrete per-path line/col and add a type-mismatch test that exercises the previously-dead UnmarshalTypeError branch.
An invalid non-empty sandbox.maxAutonomy (e.g. "moderate") previously survived Resolve (only trimmed, never validated), failed to normalize at the sandbox bridge, and left the policy unchanged at the default High ceiling. A typo therefore silently disabled the admin's intended ceiling: fail-open on a security boundary. Validate at resolve time (fail loud): Resolve now rejects a non-empty maxAutonomy that does not normalize, returning a clear error. As defense in depth, applyConfiguredAutonomyCeiling now clamps an unrecognised non-empty value to the most restrictive ceiling (low) instead of returning the policy unchanged, so a bad value can never widen the ceiling even via direct use. Empty/unset stays valid and keeps the default High ceiling. Add resolver tests for invalid-fails / valid-resolves and bridge tests for the fail-closed clamp and valid mappings.
Contributor
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 (7)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds config-driven sandbox autonomy ceilings, a new ChangesSandbox autonomy ceiling and usage reporting
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
internal/cli/observability_test.go (1)
266-269: ⚡ Quick winAssert quiet
stderron JSON doctor output in this failure path.This test validates JSON content but not that
stderrstays empty, which is important for machine-readable CLI behavior consistency.Proposed test hardening
if exitCode != exitProvider { t.Fatalf("expected provider exit %d, got %d: %s", exitProvider, exitCode, stderr.String()) } + if stderr.Len() != 0 { + t.Fatalf("expected empty stderr, got %q", stderr.String()) + } var report struct { OK bool `json:"ok"` Checks []struct {🤖 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 `@internal/cli/observability_test.go` around lines 266 - 269, The test currently checks exit codes and JSON parsing but doesn't assert that stderr is empty; update the failing assertion (the block comparing exitCode and exitProvider that references stderr.String()) to explicitly assert stderr is empty (e.g., check stderr.Len() == 0 or stderr.String() == "") and include that check either as part of the t.Fatalf message or as a separate t.Fatalf/t.Errorf after the exit-code check so the test fails if any non-JSON output was written to stderr; refer to the existing exitCode, exitProvider, and stderr variables to locate where to add the assertion.internal/cli/usage_test.go (1)
148-157: ⚡ Quick winAdd coverage for empty session-value flags.
Please add cases for
--session=and--session-id=expectingexitUsage, so parser behavior stays explicit and doesn’t silently broaden scope.🤖 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 `@internal/cli/usage_test.go` around lines 148 - 157, Add test coverage for empty session-value flags by extending TestRunUsageUnknownFlag (or adding a similar test) to call runWithDeps with arguments including "--session=" and "--session-id=" and assert the returned exitCode equals exitUsage; also assert stderr contains the parser error (e.g., the unknown/empty-session message) so the test fails if the CLI silently accepts empty session values. Use the existing helpers and symbols runWithDeps, exitUsage, and stderr buffer to implement these additional assertions.internal/usage/report_test.go (1)
31-68: ⚡ Quick winAdd a timezone-offset regression case for UTC bucketing.
Current tests only use
...Ztimestamps, so they won’t catch offset-date rollover bugs. Add one event with a non-UTC offset and assert the bucket date is normalized to UTC day.🤖 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 `@internal/usage/report_test.go` around lines 31 - 68, Update the TestBuildReportBucketsByDayAndSumsTokens test to include a timezone-offset event to catch UTC bucketing regressions: add another sessions.Event (via usageEvent) with a timestamp that includes a non-UTC offset (e.g. "+02:00" or "-05:00") whose UTC day differs from its local date, call BuildReport as before, and assert that the resulting report.Buckets contain the event under the normalized UTC date (i.e., the bucket Date matches the UTC day) and that totals/requests still aggregate correctly; locate changes around TestBuildReportBucketsByDayAndSumsTokens, usageEvent usage, and assertions on report.Buckets, BuildReport and report.Total to update expectations accordingly.
🤖 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 `@internal/cli/sandbox.go`:
- Around line 59-61: The code currently swallows errors from deps.resolveConfig,
leaving policy as DefaultPolicy() (high autonomy) when config parsing fails;
change the behavior in the surrounding function to fail loudly by checking the
error returned from deps.resolveConfig(workspaceRoot, config.Overrides{}) and
returning that error (or an explicit "unable to resolve sandbox config" error)
instead of silently continuing, so that applyConfiguredAutonomyCeiling(policy,
resolved.Sandbox.MaxAutonomy) only runs on success; reference
deps.resolveConfig, resolved.Sandbox.MaxAutonomy,
applyConfiguredAutonomyCeiling, policy, and DefaultPolicy to locate and update
the logic.
In `@internal/cli/usage.go`:
- Around line 198-209: The parser currently accepts empty session flags and
silently treats "--session=" / "--session-id=" as no-op; update the flag
handling in the switch (the branches using nextFlagValue and the
strings.HasPrefix("--session=") / "--session-id=" cases) to validate the trimmed
value is non-empty and return an error when empty instead of setting
options.sessionID or falling through; specifically after calling
nextFlagValue(...) and before assigning options.sessionID, check value != "" and
return a descriptive error, and likewise for the strings.TrimSpace(...)
branches, reject empty strings and return an error rather than accepting them.
- Around line 63-69: The current filtering slices event.CreatedAt as a
local-date string and compares it to since, which mis-handles timestamps with
timezones; instead parse event.CreatedAt into a time.Time, convert to UTC (e.g.,
t.UTC()), normalize to a YYYY-MM-DD date string or date-only value, and compare
that UTC-normalized date to the since cutoff before appending to filtered;
update the logic around the date variable and the comparison with since
(references: event.CreatedAt, date, since, filtered) so all comparisons are
performed on UTC-normalized dates.
In `@internal/config/resolver_test.go`:
- Around line 655-656: Tests calling Resolve(ResolveOptions{...}) are using the
ambient environment because Env is nil; update the four Resolve calls (the ones
constructing ResolveOptions in resolver_test.go around the blocks using variable
name 'resolved' and error checks) to include an explicit empty environment map
by setting Env: map[string]string{} in the ResolveOptions so the resolver runs
deterministically regardless of host os.Getenv values.
In `@internal/doctor/doctor.go`:
- Around line 310-312: In ValidateBytes (internal/doctor/doctor.go) stop
fabricating coordinates for non-positional JSON unmarshal errors: instead of
returning the fake (1,1,true) use a zeroed coordinate with the same
parse-failure marker (e.g., return 0,0,true) so callers see a parse error
without misleading line/column information; update the return at the
non-positional-error branch that currently returns 1,1,true to return 0,0,true.
- Around line 247-254: When os.ReadFile(path) returns readErr in the config file
loop, don't just continue silently; treat this as a failing validation detail:
mark the associated config.validation as failed (set Passed/OK flag to false)
and append a descriptive detail/error entry including the file path and readErr
message so it surfaces in the overall report. Keep iterating to collect other
failures (don't panic), but ensure the code paths around the existing
os.ReadFile(path) invocation update the validation result (e.g., set
validation.Passed = false or similar and append to validation.Details) instead
of skipping with continue.
In `@internal/sandbox/engine.go`:
- Around line 101-109: The new ceiling check uses a rewritten/normalized value
for request.Autonomy, letting malformed inputs become AutonomyLow and bypass the
clamp; update the logic in the autonomy ceiling checks (the call to
autonomyAllowed and the Decision returning ActionPrompt/reasonAboveCeiling) to
validate the original request.Autonomy before normalization (or use a separate
variable like rawAutonomy vs normalizedAutonomy), and treat malformed/unknown
autonomy as exceeding the ceiling so it clamps to prompt; ensure the same fix is
applied in both places referenced (around the autonomyAllowed call and the
similar block at lines ~123-125) and preserve existing symbols Decision,
ActionPrompt, reasonAboveCeiling, grant, and policy.MaxAutonomy.
In `@internal/usage/report.go`:
- Around line 84-87: The code currently slices event.CreatedAt into date by
taking the first 10 characters which ignores timezone offsets; update the logic
in report.go (where date := event.CreatedAt is assigned) to parse
event.CreatedAt as a time.Time, convert it to UTC, then format it to the
YYYY-MM-DD string (e.g., using time.Parse/time.ParseInLocation and
t.UTC().Format("2006-01-02")) before assigning to date so bucketization uses the
UTC day; ensure you handle parse errors appropriately (fallback or logging) in
the surrounding function that builds the report.
---
Nitpick comments:
In `@internal/cli/observability_test.go`:
- Around line 266-269: The test currently checks exit codes and JSON parsing but
doesn't assert that stderr is empty; update the failing assertion (the block
comparing exitCode and exitProvider that references stderr.String()) to
explicitly assert stderr is empty (e.g., check stderr.Len() == 0 or
stderr.String() == "") and include that check either as part of the t.Fatalf
message or as a separate t.Fatalf/t.Errorf after the exit-code check so the test
fails if any non-JSON output was written to stderr; refer to the existing
exitCode, exitProvider, and stderr variables to locate where to add the
assertion.
In `@internal/cli/usage_test.go`:
- Around line 148-157: Add test coverage for empty session-value flags by
extending TestRunUsageUnknownFlag (or adding a similar test) to call runWithDeps
with arguments including "--session=" and "--session-id=" and assert the
returned exitCode equals exitUsage; also assert stderr contains the parser error
(e.g., the unknown/empty-session message) so the test fails if the CLI silently
accepts empty session values. Use the existing helpers and symbols runWithDeps,
exitUsage, and stderr buffer to implement these additional assertions.
In `@internal/usage/report_test.go`:
- Around line 31-68: Update the TestBuildReportBucketsByDayAndSumsTokens test to
include a timezone-offset event to catch UTC bucketing regressions: add another
sessions.Event (via usageEvent) with a timestamp that includes a non-UTC offset
(e.g. "+02:00" or "-05:00") whose UTC day differs from its local date, call
BuildReport as before, and assert that the resulting report.Buckets contain the
event under the normalized UTC date (i.e., the bucket Date matches the UTC day)
and that totals/requests still aggregate correctly; locate changes around
TestBuildReportBucketsByDayAndSumsTokens, usageEvent usage, and assertions on
report.Buckets, BuildReport and report.Total to update expectations accordingly.
🪄 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: 2c63756c-b830-4a43-9c9c-e9b0444466fc
📒 Files selected for processing (35)
internal/cli/app.gointernal/cli/app_test.gointernal/cli/exec.gointernal/cli/observability.gointernal/cli/observability_test.gointernal/cli/sandbox.gointernal/cli/sandbox_test.gointernal/cli/usage.gointernal/cli/usage_test.gointernal/cli/workflow_test.gointernal/cli/workflows.gointernal/config/resolver.gointernal/config/resolver_test.gointernal/config/types.gointernal/config/validate.gointernal/config/validate_test.gointernal/doctor/doctor.gointernal/doctor/doctor_test.gointernal/sandbox/batch.gointernal/sandbox/batch_test.gointernal/sandbox/engine.gointernal/sandbox/engine_test.gointernal/sandbox/normalize.gointernal/sandbox/normalize_test.gointernal/sandbox/types.gointernal/usage/report.gointernal/usage/report_test.gointernal/zerocommands/sandbox_snapshots.gointernal/zerocommands/sandbox_snapshots_test.gointernal/zerogit/contracts.gointernal/zerogit/contracts_test.gointernal/zerogit/diffstat.gointernal/zerogit/diffstat_test.gointernal/zerogit/zerogit.gointernal/zerogit/zerogit_test.go
added 4 commits
June 8, 2026 15:47
… errors Treat a genuinely-invalid request autonomy as the highest tier in the engine so it exceeds a Medium/Low ceiling and clamps to Prompt instead of being sanitized to Low and auto-allowing on the grant/unsafe path. Surface resolveConfig failures in the policy command instead of silently falling back to the permissive DefaultPolicy, which would misreport trust posture.
Normalize RFC3339 timestamps to their UTC calendar date before day bucketing (report) and before the --since/--days cutoff comparison (CLI) so offset timestamps land on the correct UTC day and the bucket and cutoff agree; malformed timestamps fall back to the leading-10 slice. Reject empty --session/--session-id values with a value-required error matching the --since validation style.
Surface a present-but-unreadable config path (permissions, is-a-directory) as a failing per-path validation detail instead of silently skipping it; a genuinely-missing path stays a skip. Return ok=false for a JSON parse error that carries no offset so it routes through the no-position ValidateBytes path rather than fabricating a (1,1) line/col. Assert empty stderr on the JSON doctor failure path.
Pass an explicit empty Env to the maxAutonomy Resolve tests so host environment variables cannot leak in through the nil-Env os.Getenv fallback.
Collaborator
Author
|
Thanks for the thorough review — all 8 actionable items + 3 nitpicks addressed in Security (the important one):
Other fixes:
@coderabbitai please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the PR details and recent commits
gh pr view 142 --repo Gitlawb/zero --json title,state,headRefName,baseRefName,additions,deletions,changedFiles
# Get the list of commits
gh pr view 142 --repo Gitlawb/zero --json commits --jq '.commits[-6:] | .[] | {oid: .oid, message: .messageHeadline}'Length of output: 1339 🏁 Script executed: #!/bin/bash
# Get the diff for the fix commits
git log --oneline -10Length of output: 839 🏁 Script executed: #!/bin/bash
# Read the key security-relevant files that were modified in the fix commits
cat internal/sandbox/engine.goLength of output: 6089 🏁 Script executed: #!/bin/bash
cat internal/sandbox/normalize.goLength of output: 2502 🏁 Script executed: #!/bin/bash
cat internal/cli/sandbox.goLength of output: 16861 🏁 Script executed: #!/bin/bash
cat internal/usage/report.goLength of output: 5092 🏁 Script executed: #!/bin/bash
cat internal/cli/usage.goLength of output: 10434 🏁 Script executed: #!/bin/bash
cat internal/doctor/doctor.goLength of output: 12016
Security (
UTC bucketing (
No remaining actionable issues. The PR is in good shape. [approve] 🐇 ✓ 🧠 Learnings used |
Vasanthdev2004
requested changes
Jun 8, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: Request changes
Finding:
[P1] Preserve invalid autonomy through the ceiling checks.
In internal/sandbox/engine.go, Evaluate normalizes an invalid request.Autonomy to AutonomyHigh before the persistent grant and unsafe-mode ceiling checks. That still auto-allows when policy.MaxAutonomy is High or default, because autonomyAllowed(High, High) returns true. This contradicts the fail-closed comment and bypasses the unknown-tier guard that autonomyAllowed already has for invalid requested autonomy.
Please preserve the raw invalid autonomy or track an invalidAutonomy flag and force Prompt before the grant/unsafe allow paths. Add regression coverage for invalid autonomy under the default/High ceiling for both unsafe escalation and persistent grant allow.
Validation context:
- GitHub CI checks are green.
- CodeRabbit is still pending at the time of this review.
- I did not run the full suite locally on this PR branch.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/sandbox/engine_test.go (1)
393-430: ⚡ Quick winPin the grant metadata on the fail-closed path.
Lines 426-430 only prove the result gets clamped to
ActionPrompt. IfEvaluatestill setsGrantMatched=truebefore the ceiling clamp, this regression would still pass while the decision keeps claiming that a persistent allow matched. Add an explicit!decision.GrantMatchedassertion here.Suggested assertion
if decision.Action != ActionPrompt { t.Fatalf("invalid-autonomy grant decision = %#v, want prompt (fail closed above ceiling, not grant allow)", decision) } if decision.Reason != reasonAboveCeiling { t.Fatalf("decision.Reason = %q, want %q", decision.Reason, reasonAboveCeiling) } + if decision.GrantMatched { + t.Fatalf("invalid-autonomy grant decision = %#v, want no matched grant after ceiling clamp", decision) + }🤖 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 `@internal/sandbox/engine_test.go` around lines 393 - 430, The test TestEngineInvalidAutonomyFailsClosedOnGrantAllow currently only checks that the action is ActionPrompt and reason is reasonAboveCeiling; add an assertion that the persistent grant flag is not set so the fail-closed path pins the grant metadata: after obtaining decision from engine.Evaluate, assert that decision.GrantMatched is false (i.e., !decision.GrantMatched) so the test fails if Evaluate incorrectly reports a matching persistent allow even when autonomy was clamped.
🤖 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.
Nitpick comments:
In `@internal/sandbox/engine_test.go`:
- Around line 393-430: The test TestEngineInvalidAutonomyFailsClosedOnGrantAllow
currently only checks that the action is ActionPrompt and reason is
reasonAboveCeiling; add an assertion that the persistent grant flag is not set
so the fail-closed path pins the grant metadata: after obtaining decision from
engine.Evaluate, assert that decision.GrantMatched is false (i.e.,
!decision.GrantMatched) so the test fails if Evaluate incorrectly reports a
matching persistent allow even when autonomy was clamped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e697934-6a59-43fa-af98-78616bfb5bba
📒 Files selected for processing (12)
internal/cli/observability_test.gointernal/cli/sandbox.gointernal/cli/sandbox_test.gointernal/cli/usage.gointernal/cli/usage_test.gointernal/config/resolver_test.gointernal/doctor/doctor.gointernal/doctor/doctor_test.gointernal/sandbox/engine.gointernal/sandbox/engine_test.gointernal/usage/report.gointernal/usage/report_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/sandbox/engine.go
- internal/cli/observability_test.go
- internal/doctor/doctor.go
- internal/config/resolver_test.go
- internal/usage/report_test.go
- internal/cli/usage_test.go
- internal/cli/usage.go
…der default ceiling Defaulting invalid request.Autonomy to High only clamped under a Medium/Low ceiling; under the default High ceiling autonomyAllowed(High, High) returned true and still auto-allowed (fail-open). Keep the raw requested value for the ceiling check so autonomyAllowed's unknown-tier guard fails it closed under ANY ceiling. Adds default-High-ceiling regression tests for the unsafe and grant-allow paths.
Collaborator
Author
|
@Vasanthdev2004 good catch — you're right, the fix was incomplete. Normalizing invalid autonomy to
Full gates green ( |
# Conflicts: # internal/config/resolver.go
Vasanthdev2004
approved these changes
Jun 8, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: Approve
Conflict resolution reviewed at 1e9a862.
The previous sandbox blocker is fixed: invalid autonomy is preserved through ceiling checks and covered under the default High ceiling for unsafe and persistent grant paths. The merge with latest main keeps the provider catalog/env-key hardening and the sandbox max-autonomy validation together; internal/config/resolver.go now validates sandbox.maxAutonomy while still resolving provider apiKeyEnv through the safe catalog-aware path.
Local validation on the conflict-resolution worktree:
go test ./...passedgo run ./cmd/zero-release buildpassedgo run ./cmd/zero-release smokepassed
GitHub checks are still running after the new merge commit, but the PR is now mergeable and I did not find code blockers.
This was referenced Jun 11, 2026
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
2 participants
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.
Summary
Four small, dependency-free CLI improvements, batched into one PR:
zero doctornow validates config files. A newconfig.validationcheck parses each resolved config file and reports malformed JSON with line/col (including type-mismatch offsets, e.g. a string where an int is expected) plus semantic provider issues — all redaction-safe. Also fixes a pre-existing gap wheredoctorreceived no config paths and aborted on bad JSON before any check ran; it now degrades gracefully. (internal/config,internal/doctor,internal/cli)Admin autonomy ceiling for the sandbox. A new config key
sandbox.maxAutonomy(low|medium|high, envZERO_SANDBOX_MAX_AUTONOMY) caps the autonomy a persistent grant or unsafe mode can reach. Enforced insidesandbox.Engine.Evaluate()— a request above the ceiling is clamped to a permission prompt (not silently auto-allowed). Defaults tohigh(a no-op, fully backward compatible); an invalid value fails loud at config resolve and closed at the engine bridge. Surfaced inzero sandbox policy. (internal/sandbox,internal/config,internal/cli,internal/zerocommands)zero changes --base <ref>. Inspect the changes a branch introduced via a three-dot merge-base diff (git diff <base>...HEAD) instead of the working tree, withFilesfrom--name-status. Additive flag onchanges inspect/status(rejected oncommit); the existing working-tree path is unchanged. (internal/zerogit,internal/cli)zero usage report. A local token/cost report aggregated from already-persisted session usage events (no new persistence), bucketed per day, with a tokens/cost per net-LOC efficiency metric. Cost is reconstructed from the session model and labeled an estimate; net-LOC is a working-tree diff proxy, also labeled an estimate.--json,--days N,--since YYYY-MM-DD,--session <id>. (internal/zerogit,internal/usage,internal/cli)No new module dependencies. All changes are additive and backward-compatible.
Test Plan
go build ./...andgo vet ./...cleango test ./...green (new tests acrossconfig,doctor,sandbox,zerogit,usage,cli,zerocommands)go test -race ./internal/{sandbox,config,doctor,zerogit,usage,cli}/...greenGOOS=windows GOARCH=amd64 go build ./...greengit diff --checkcleanReviewer focus
Evaluate()(not just UI), clamps to Prompt (Deny still wins), and an invalidsandbox.maxAutonomyfails closed — confirm the tests pin this.changes --basediff/base field.Summary by CodeRabbit
New Features
zero usage report(defaultusage) with token/cost estimation, date/session filters, JSON/text output, and help listingusage--base <ref>tozero changes inspectto diff against a specified refImprovements
max_autonomydoctorreports config validation with JSON parse positions and semantic issuesBug Fixes
--session/--session-idand tighten CLI flag validation