Skip to content

feat: add Go worktree and verification backend - #70

Merged
gnanam1990 merged 7 commits into
mainfrom
feat/m5-worktree-verify-backend
Jun 5, 2026
Merged

feat: add Go worktree and verification backend#70
gnanam1990 merged 7 commits into
mainfrom
feat/m5-worktree-verify-backend

Conversation

@gnanam1990

@gnanam1990 gnanam1990 commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add a Go worktree isolation backend with deterministic task names, safe worktree path handling, and reusable detached worktree preparation
  • add a Go verification backend that detects repository checks, runs filtered verification plans, redacts command output, and reports structured pass/fail/error summaries
  • wire the new backend into the CLI through zero worktrees prepare, zero verify, and zero exec --worktree / --worktree-dir with conflict validation

Local validation

  • go test -count=1 ./internal/worktrees ./internal/verify ./internal/cli
  • go test -count=1 -p 1 ./...
  • npx --yes bun run typecheck
  • npx --yes bun test ./tests --timeout 15000
  • npx --yes bun run build
  • npx --yes bun run smoke:build
  • npx --yes bun run smoke:go
  • ./zero verify --only go.test --timeout-ms 600000
  • temp git repo smoke for ./zero worktrees prepare --json

Summary by CodeRabbit

  • New Features

    • Added worktrees and verify CLI commands; "zero exec" gains --worktree and --worktree-dir. verify supports --only and timeout; both support --json output.
    • CLI can prepare isolated Git worktrees and run repository verification checks (Go/npm).
  • CLI

    • Help text updated to list the new commands and options; output redacts sensitive workspace/worktree path segments.
  • Tests

    • Added extensive tests for CLI, worktrees, and verify (success, failure, redaction, JSON).

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Zero automated PR review

Verdict: No blockers found

Blockers

  • None found.

Validation

  • [pass] Diff hygiene: git diff --check
  • [pass] Typecheck: bun run typecheck
  • [pass] Tests: bun run test
  • [pass] Build: bun run build
  • [pass] Smoke build: bun run smoke:build

Scope

Head: 0f1e35f2dd44
Changed files (10): internal/cli/app.go, internal/cli/app_test.go, internal/cli/exec.go, internal/cli/exec_parse.go, internal/cli/workflow_test.go, internal/cli/workflows.go, internal/verify/verify.go, internal/verify/verify_test.go, internal/worktrees/worktrees.go, internal/worktrees/worktrees_test.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c5aff4a4-ca56-4403-88d7-1ef1d2bf6251

📥 Commits

Reviewing files that changed from the base of the PR and between 15f5d40 and 0f1e35f.

📒 Files selected for processing (2)
  • internal/cli/workflow_test.go
  • internal/cli/workflows.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/cli/workflow_test.go
  • internal/cli/workflows.go

Walkthrough

Adds Git worktree management and a local verification system, exposes worktrees prepare and verify CLI commands, extends zero exec with worktree flags, wires app dependencies, and adds unit/integration tests covering these flows.

Changes

Worktree & Verification Infrastructure

Layer / File(s) Summary
Worktree Preparation Module
internal/worktrees/worktrees.go, internal/worktrees/worktrees_test.go
Implements worktree creation/reuse under per-repo base dirs with name validation, repo-key derivation, Git runner abstraction, Prepare() and DefaultBaseDir(); tests cover create/reuse, cross-repo rejection, name/dir validation, and base-dir behavior.
Verification Plan Detection & Execution Module
internal/verify/verify.go, internal/verify/verify_test.go
Adds verification data model, DetectPlan() (go.mod and package.json script detection) and Run() (pluggable Runner, timeout, per-check timing/output/status, redaction), with tests for detection, execution, filtering, unknown-only handling, and redaction.
CLI Workflow Commands
internal/cli/workflows.go
Implements worktrees prepare and verify command runners and parsers, supports --json, --cwd, --name/--dir, --only, --timeout-ms, and provides text/JSON formatters and help text.
App Integration & Routing
internal/cli/app.go, internal/cli/app_test.go
Extends appDeps with prepareWorktree, detectVerifyPlan, runVerify; wires defaults in defaultAppDeps; updates runWithDeps routing; injects missing deps in fillAppDeps; updates top-level and zero exec help text.
Exec Command: Worktree Support
internal/cli/exec.go, internal/cli/exec_parse.go
Adds --worktree [name], --worktree=<name>, and --worktree-dir <dir> flags, validates --fork incompatibility and --worktree-dir dependency, and updates runExec to prepare/switch to a worktree when requested.
Integration Tests
internal/cli/workflow_test.go
Integration tests for worktree prepare (success/error, text/JSON), verify (detection, --only, failure exit mapping, redaction), exec worktree integration and flag validation; uses stubs and asserts stdout/stderr/exit codes and JSON shapes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Gitlawb/zero#54: Prior changes to zero exec wiring that this PR extends with worktree preparation.
  • Gitlawb/zero#57: Also touches zero exec CLI argument handling and exec flow.

Suggested reviewers

  • anandh8x
  • Vasanthdev2004
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add Go worktree and verification backend' directly and clearly summarizes the main change—adding worktree and verification backends to the codebase and wiring them into the CLI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/m5-worktree-verify-backend

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

@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: 4

🧹 Nitpick comments (2)
internal/cli/workflow_test.go (1)

67-74: ⚡ Quick win

Stub getwd here to keep this test hermetic.

TestRunWorktreesPrepareReportsErrors currently relies on default dependency behavior for cwd. Add a fixed getwd stub (like other tests) so this stays isolated from ambient process state if command pre-validation changes.

Proposed patch
 func TestRunWorktreesPrepareReportsErrors(t *testing.T) {
+	cwd := t.TempDir()
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
 	exitCode := runWithDeps([]string{"worktrees", "prepare", "--name", "bad"}, &stdout, &stderr, appDeps{
+		getwd: func() (string, error) { return cwd, nil },
 		prepareWorktree: func(context.Context, worktrees.Options) (worktrees.Result, error) {
 			return worktrees.Result{}, errors.New("not a git repository")
 		},
 	})
🤖 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/workflow_test.go` around lines 67 - 74,
TestRunWorktreesPrepareReportsErrors is missing a getwd dependency stub which
makes the test rely on the real CWD; update the call to runWithDeps to pass an
appDeps.getwd function (similar to other tests) that returns a fixed path (e.g.,
"/", nil) so the test is hermetic; locate the runWithDeps invocation in
TestRunWorktreesPrepareReportsErrors and add getwd: func() (string, error) {
return "/", nil } to the appDeps struct passed in.
internal/worktrees/worktrees_test.go (1)

50-111: ⚡ Quick win

Add a regression test for “existing .git from different repo must not be reused”.

Current tests validate reuse presence, but not repository identity. Please add a case where target exists with .git metadata from another repo and assert Prepare fails.

🤖 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/worktrees/worktrees_test.go` around lines 50 - 111, Add a new test
that ensures Prepare does not reuse a directory whose .git belongs to a
different repo: create temp root/base, build an existing path like
filepath.Join(base, "zero-worktree-"+repoKey(root), "other-repo"), create the
directory and write minimal .git metadata that indicates a different repo (e.g.,
write a HEAD or config file with a different commit or repo id), set up a
fakeRunner with the same metadata calls used in other tests, call Prepare(ctx,
Options{Cwd: root, Name: "other-repo", BaseDir: base, RunGit: runner.Run}) and
assert it returns an error (or Result.Reused == false) indicating the worktree
could not be reused; reference Prepare, Options, repoKey and the
fakeRunner/CommandResult pattern to locate where to add this test.
🤖 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/workflow_test.go`:
- Around line 222-256: The tests TestRunExecRejectsForkWithWorktree and
TestRunExecRejectsWorktreeDirWithoutWorktree currently only check exitCode and
stderr; add assertions that the captured stdout (the variable stdout passed into
runWithDeps) is empty for these usage error cases to ensure no unintended output
is written to stdout. Locate the two tests and after the existing stderr/assert
checks, add a check like if stdout.String() != "" { t.Fatalf("expected no
stdout, got %q", stdout.String()) } so the assertion references the stdout
buffer used in runWithDeps and ensures CLI UX guarantees are enforced.

In `@internal/cli/workflows.go`:
- Around line 124-133: When parsing flags in the loop (the branches handling arg
== "--name" and strings.HasPrefix(arg, "--name=")) and when assigning a
positional name (the later block around the positional handling at ~154-157),
check if options.name is already set to a non-empty value before overwriting; if
it is and the new incoming name is non-empty, return an error (fail fast)
instead of silently replacing it. Update the logic in the loop that calls
nextFlagValue and in the strings.HasPrefix("--name=") branch and the
positional-assignment code to perform this conflict check and return a
descriptive error if a second non-empty name is provided.

In `@internal/verify/verify.go`:
- Around line 273-279: The unknown slice is populated by iterating the map
allowed (nondeterministic order) causing unstable output; after building unknown
(from the loop over allowed and seen) sort it deterministically before returning
(e.g., call sort.Strings on unknown) so the function in
internal/verify/verify.go (the slice unknown computed from allowed and seen and
returned alongside filtered) always returns a stable ordering.

In `@internal/worktrees/worktrees.go`:
- Around line 95-102: When deciding to mark a target as reused (the reused check
returned by inspectTarget and setting result.Reused = true), first resolve the
target path (use filepath.EvalSymlinks) and confirm the existing folder is the
same Git repository as the expected one: obtain the repo identity from the
existing path (e.g., git -C <path> rev-parse --show-toplevel and/or git -C
<path> config --get remote.origin.url or read .git/config) and compare it to the
expected repository identity used to create the deterministic path; if the
identities differ, do not set result.Reused and proceed as if not reused. Ensure
this validation is applied in the same code paths that detect .git presence (the
logic around inspectTarget/target and the block that currently returns
result.Reused = true) so symlinked or pre-existing directories pointing to other
repos are rejected.

---

Nitpick comments:
In `@internal/cli/workflow_test.go`:
- Around line 67-74: TestRunWorktreesPrepareReportsErrors is missing a getwd
dependency stub which makes the test rely on the real CWD; update the call to
runWithDeps to pass an appDeps.getwd function (similar to other tests) that
returns a fixed path (e.g., "/", nil) so the test is hermetic; locate the
runWithDeps invocation in TestRunWorktreesPrepareReportsErrors and add getwd:
func() (string, error) { return "/", nil } to the appDeps struct passed in.

In `@internal/worktrees/worktrees_test.go`:
- Around line 50-111: Add a new test that ensures Prepare does not reuse a
directory whose .git belongs to a different repo: create temp root/base, build
an existing path like filepath.Join(base, "zero-worktree-"+repoKey(root),
"other-repo"), create the directory and write minimal .git metadata that
indicates a different repo (e.g., write a HEAD or config file with a different
commit or repo id), set up a fakeRunner with the same metadata calls used in
other tests, call Prepare(ctx, Options{Cwd: root, Name: "other-repo", BaseDir:
base, RunGit: runner.Run}) and assert it returns an error (or Result.Reused ==
false) indicating the worktree could not be reused; reference Prepare, Options,
repoKey and the fakeRunner/CommandResult pattern to locate where to add this
test.
🪄 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: 93b8bc8a-980f-41ac-8e00-8b554b5b8102

📥 Commits

Reviewing files that changed from the base of the PR and between 53097ab and b495544.

📒 Files selected for processing (10)
  • internal/cli/app.go
  • internal/cli/app_test.go
  • internal/cli/exec.go
  • internal/cli/exec_parse.go
  • internal/cli/workflow_test.go
  • internal/cli/workflows.go
  • internal/verify/verify.go
  • internal/verify/verify_test.go
  • internal/worktrees/worktrees.go
  • internal/worktrees/worktrees_test.go

Comment thread internal/cli/workflow_test.go
Comment thread internal/cli/workflows.go
Comment thread internal/verify/verify.go
Comment thread internal/worktrees/worktrees.go
@gnanam1990
gnanam1990 force-pushed the feat/m5-worktree-verify-backend branch from b495544 to 2528f90 Compare June 5, 2026 10:39

@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 `@internal/worktrees/worktrees_test.go`:
- Around line 178-179: The test currently uses strings.Contains to check the
DefaultBaseDir result, which is too permissive; replace that check by
constructing the expected path via filepath.Join(profile, "AppData", "Local",
"zero", "worktrees"), then compare filepath.Clean(got) ==
filepath.Clean(expected) (or use os.PathSeparator-normalized equality) and call
t.Fatalf with both values if they differ so the assertion requires an exact
match; update the assertion around the variable `got` and the expected join to
use equality instead of strings.Contains.
🪄 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: 6e5e4365-02e6-4d3b-8358-b10a8076c29b

📥 Commits

Reviewing files that changed from the base of the PR and between b495544 and 2528f90.

📒 Files selected for processing (10)
  • internal/cli/app.go
  • internal/cli/app_test.go
  • internal/cli/exec.go
  • internal/cli/exec_parse.go
  • internal/cli/workflow_test.go
  • internal/cli/workflows.go
  • internal/verify/verify.go
  • internal/verify/verify_test.go
  • internal/worktrees/worktrees.go
  • internal/worktrees/worktrees_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
  • internal/cli/exec_parse.go
  • internal/verify/verify_test.go
  • internal/cli/app.go
  • internal/cli/exec.go
  • internal/cli/workflow_test.go
  • internal/cli/app_test.go
  • internal/worktrees/worktrees.go
  • internal/verify/verify.go
  • internal/cli/workflows.go

Comment thread internal/worktrees/worktrees_test.go Outdated

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blockers

  • zero verify is a new diagnostic-output surface, but it serializes the resolved workspace path without redaction. In internal/cli/workflows.go, JSON mode writes the raw verify.Report directly and text mode prints root: + report.Root; verify.Report.Root comes from DetectPlan/resolveRoot unchanged. I reproduced this on latest head 45086327868fedd88f263710799e03bcc3571e53 by creating a temp workspace whose directory name contained an assembled OpenAI-shaped key. Both ./zero.exe verify -C <secret-dir> --json and text ./zero.exe verify -C <secret-dir> included the exact raw key in the root field. Please run the entire report through the central redaction path before both JSON and text formatting, and add a regression that covers secret-looking workspace paths. The same fix should cover zero worktrees prepare output too, because that command currently writes raw path/repoRoot values in JSON mode and formatWorktreeResult prints raw paths in text mode.

Non-Blocking

  • CI was still finishing for the latest force-push when I posted this review: ubuntu/macOS/Performance/Zero Review had passed, Windows smoke was still pending on my latest check.

Looks Good

  • The worktree backend validates task names, avoids path traversal through the worktree name, creates detached worktrees under a repo-keyed base directory, and refuses to reuse a path from a different git repository.
  • The verify backend correctly detects Go/package-script checks, supports --only filtering, classifies pass/fail/error outcomes, applies per-check timeouts, and redacts command stdout/stderr.
  • zero exec --worktree correctly switches the workspace before config/tool/provider setup and rejects the conflicting --fork / --worktree combination.
  • Local validation passed on latest head:
    • go test -count=1 ./internal/worktrees ./internal/verify ./internal/cli
    • go test -count=1 -p 1 ./...
    • go build ./cmd/zero
    • bun install --frozen-lockfile
    • bun run typecheck
    • bun test ./tests --timeout 15000 — 291 pass / 0 fail
    • bun run build
    • bun run smoke:build
    • bun run smoke:go
    • ./zero.exe --help
    • ./zero.exe worktrees --help
    • ./zero.exe verify --help
    • ./zero.exe verify --only go.test --timeout-ms 600000 --json
    • temp git repo smoke for ./zero.exe worktrees prepare --json
    • git diff --check origin/main..HEAD

@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

🧹 Nitpick comments (1)
internal/cli/workflows.go (1)

255-272: 💤 Low value

Reconsider redacting non-sensitive fields.

The function redacts timestamps (StartedAt, EndedAt), check identifiers (ID, Name), and command arguments. Timestamps are typically just ISO8601 strings and check IDs/names are usually simple identifiers like "go.test", not sensitive data. Redacting these may obscure useful diagnostic information without security benefit.

Consider limiting redaction to fields that can contain secrets or sensitive data (Stdout, Stderr, Error) and only redacting Root if path redaction is truly needed.

🤖 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/workflows.go` around lines 255 - 272, The current
redactVerifyReport function over-redacts non-sensitive fields (StartedAt,
EndedAt, Results[].ID, Results[].Name, Results[].StartedAt, Results[].EndedAt,
and Results[].Command) which removes useful diagnostic info; update
redactVerifyReport to only call redactCLIString on truly sensitive fields (Root
if needed, and Results[].Stdout, Results[].Stderr, Results[].Error) and leave
timestamps, IDs, names, and command slices unchanged—modify the function body
around the redactVerifyReport declaration and stop applying redactCLIString to
Results[index].ID, Name, StartedAt, EndedAt, and Command entries.
🤖 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/workflows.go`:
- Around line 274-276: redactCLIString currently calls
redaction.RedactString(value, redaction.Options{}) which only redacts secrets,
so filesystem paths (e.g., Path, RepoRoot, report.Root) remain unredacted;
change this by adding path-redaction support in the redaction package (e.g.,
extend redaction.Options with a boolean/array like ObfuscatePaths or
PathsToRedact and implement a helper that normalizes and replaces filesystem
paths with a stable token such as "<REDACTED_PATH>"), then update
redactCLIString to either pass the new option to redaction.RedactString or call
the new RedactPaths helper after/before the existing secret redaction; add unit
tests in the redaction package to cover typical path cases (absolute, relative,
repo roots) and update any callers relying on redactCLIString to ensure CLI
fields (Path, RepoRoot, report.Root) are covered.

---

Nitpick comments:
In `@internal/cli/workflows.go`:
- Around line 255-272: The current redactVerifyReport function over-redacts
non-sensitive fields (StartedAt, EndedAt, Results[].ID, Results[].Name,
Results[].StartedAt, Results[].EndedAt, and Results[].Command) which removes
useful diagnostic info; update redactVerifyReport to only call redactCLIString
on truly sensitive fields (Root if needed, and Results[].Stdout,
Results[].Stderr, Results[].Error) and leave timestamps, IDs, names, and command
slices unchanged—modify the function body around the redactVerifyReport
declaration and stop applying redactCLIString to Results[index].ID, Name,
StartedAt, EndedAt, and Command entries.
🪄 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: 6514c519-0012-4be3-8ab5-8cc1565e8a4e

📥 Commits

Reviewing files that changed from the base of the PR and between 4508632 and 15f5d40.

📒 Files selected for processing (2)
  • internal/cli/workflow_test.go
  • internal/cli/workflows.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/cli/workflow_test.go

Comment thread internal/cli/workflows.go
@gnanam1990

Copy link
Copy Markdown
Collaborator Author

@Vasanthdev2004 requested-change blocker is fixed on latest head 0f1e35f.

What changed:

  • zero verify now emits redacted copies for text and JSON output, including report.Root.
  • zero worktrees prepare now emits redacted copies for text and JSON output, including path and repoRoot.
  • Added regressions covering secret-looking path segments for both commands and both output modes.
  • Kept normal filesystem paths visible because these commands intentionally report useful locations; only secret-looking tokens embedded in those paths are redacted through the central redaction path.

Validation rerun:

  • go test -count=1 ./internal/cli ./internal/worktrees ./internal/verify
  • go test -count=1 -p 1 ./...
  • npx --yes bun run typecheck
  • npx --yes bun test ./tests --timeout 15000
  • npx --yes bun run build
  • npx --yes bun run smoke:build
  • npx --yes bun run smoke:go
  • binary smoke proving secret-looking path segments are redacted in verify and worktrees prepare text/JSON output

Current GitHub checks are green.

@gnanam1990
gnanam1990 requested a review from Vasanthdev2004 June 5, 2026 11:03
@gnanam1990

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Vasanthdev2004

Copy link
Copy Markdown
Collaborator

@coderabbitai approve

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
✅ Action performed

Comments resolved and changes approved.

@Vasanthdev2004

Copy link
Copy Markdown
Collaborator

Looks Good

I re-reviewed latest head 0f1e35f2dd44df86721499b3896a07e97ea185be against the prior changes-requested head 45086327868fedd88f263710799e03bcc3571e53 and current origin/main. The previous blocker is fixed: the zero verify and zero worktrees prepare output paths now go through the central redaction path before both JSON and text formatting.

Blockers

  • None.

Non-Blocking

  • The new unit tests use a compact dummy token fragment (sk-pro...wxyz) rather than a fully realistic OpenAI-shaped key. I covered that gap with runtime CLI probes using assembled sk- + proj- + 48-character values for both verify and worktrees outputs.

Looks Good

  • redactVerifyReport redacts the report root plus per-result stdout/stderr/error before JSON/text output while preserving the unredacted report for exit-code decisions.
  • redactWorktreeResult redacts name/path/repo/branch/commit before JSON/text output.
  • The fix keeps ordinary paths useful while removing secret-looking path segments.
  • Runtime redaction probes passed for both JSON and text output:
    • ./zero.exe verify -C <secret-bearing-dir> --json
    • ./zero.exe verify -C <secret-bearing-dir>
    • ./zero.exe worktrees prepare --cwd <temp-git-repo> --dir <secret-bearing-base> --name secret-probe --json
    • ./zero.exe worktrees prepare --cwd <temp-git-repo> --dir <secret-bearing-base> --name secret-probe
  • Local validation passed:
    • go test -count=1 ./internal/cli ./internal/worktrees ./internal/verify ./internal/redaction
    • go test -count=1 -p 1 ./...
    • go build ./cmd/zero
    • bun install --frozen-lockfile
    • bun run typecheck
    • bun test ./tests --timeout 15000 — 291 pass / 0 fail
    • bun run build
    • bun run smoke:build
    • bun run smoke:go
    • ./zero.exe --help
    • ./zero.exe worktrees --help
    • ./zero.exe verify --help
    • git diff --check origin/main..HEAD
  • GitHub checks are green: Smoke ubuntu, Smoke macOS, Smoke Windows, Performance Smoke, Zero Review, and CodeRabbit.

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks Good

I re-reviewed latest head 0f1e35f2dd44df86721499b3896a07e97ea185be against the prior changes-requested head 45086327868fedd88f263710799e03bcc3571e53 and current origin/main. The previous blocker is fixed: the zero verify and zero worktrees prepare output paths now go through the central redaction path before both JSON and text formatting.

Blockers

  • None.

Non-Blocking

  • The new unit tests use a compact dummy token fragment (sk-pro...wxyz) rather than a fully realistic OpenAI-shaped key. I covered that gap with runtime CLI probes using assembled sk- + proj- + 48-character values for both verify and worktrees outputs.

Looks Good

  • redactVerifyReport redacts the report root plus per-result stdout/stderr/error before JSON/text output while preserving the unredacted report for exit-code decisions.
  • redactWorktreeResult redacts name/path/repo/branch/commit before JSON/text output.
  • The fix keeps ordinary paths useful while removing secret-looking path segments.
  • Runtime redaction probes passed for both JSON and text output:
    • ./zero.exe verify -C <secret-bearing-dir> --json
    • ./zero.exe verify -C <secret-bearing-dir>
    • ./zero.exe worktrees prepare --cwd <temp-git-repo> --dir <secret-bearing-base> --name secret-probe --json
    • ./zero.exe worktrees prepare --cwd <temp-git-repo> --dir <secret-bearing-base> --name secret-probe
  • Local validation passed:
    • go test -count=1 ./internal/cli ./internal/worktrees ./internal/verify ./internal/redaction
    • go test -count=1 -p 1 ./...
    • go build ./cmd/zero
    • bun install --frozen-lockfile
    • bun run typecheck
    • bun test ./tests --timeout 15000 — 291 pass / 0 fail
    • bun run build
    • bun run smoke:build
    • bun run smoke:go
    • ./zero.exe --help
    • ./zero.exe worktrees --help
    • ./zero.exe verify --help
    • git diff --check origin/main..HEAD
  • GitHub checks are green: Smoke ubuntu, Smoke macOS, Smoke Windows, Performance Smoke, Zero Review, and CodeRabbit.

@coderabbitai coderabbitai Bot mentioned this pull request Jun 10, 2026
@Vasanthdev2004
Vasanthdev2004 deleted the feat/m5-worktree-verify-backend branch June 28, 2026 08:27
euxaristia added a commit to euxaristia/zero that referenced this pull request Jul 22, 2026
- Prepare and Release now agree on repoKey regardless of which worktree
  (main or linked) Prepare runs from, by keying off git worktree list's
  first entry (always the main worktree) instead of --show-toplevel.
  A worktree prepared from a linked checkout previously failed its own
  ownership check on release and its lease could never be cleared.
- osProcessAlive on Windows no longer treats every OpenProcess failure
  as "process is dead": only ERROR_ACCESS_DENIED (a live process this
  caller lacks rights to query) is now distinguished from a genuinely
  missing PID, so Clean can no longer force-remove an active worktree
  whose owning process it simply couldn't query.
- The openai_key redaction pattern now recognizes sk-or-v1- (OpenRouter)
  alongside the existing sk-proj-/sk-svcacct-/sk-admin- prefixes, so
  hyphenated OpenAI-compatible provider keys are redacted again without
  reopening the sk-<kebab-case-phrase> false-positive this pattern was
  narrowed to avoid.
- Release/exec --worktree error text is redacted before reaching
  stderr, matching the already-redacted success path; ownership errors
  interpolate the caller-supplied path, which could carry a key-shaped
  segment.
- canonicalizePath resolves symlinks through the nearest existing
  ancestor when the target itself no longer exists, so the documented
  `release -C` recovery path works again for a worktree deleted by hand
  under a symlinked --worktree-dir.
- Prepare rolls back the worktree `git worktree add` just created if the
  subsequent lock call fails for a reason other than a concurrent
  racer, instead of leaking an unleased checkout until Clean's 24h
  staleness window reclaims it.

Not addressed here: the P2 finding that worktree ownership is provable
only by a directory-name convention plus a lock-reason prefix, both of
which a user can reproduce by hand. A durable per-worktree ownership
marker would close that gap, but internal/worktrees has been on main
since Gitlawb#70, so a marker requirement could reject worktrees an
already-installed zero created before this change existed. Needs a
decision on migration before implementing.
euxaristia added a commit to euxaristia/zero that referenced this pull request Jul 22, 2026
- Prepare and Release now agree on repoKey regardless of which worktree
  (main or linked) Prepare runs from, by keying off git worktree list's
  first entry (always the main worktree) instead of --show-toplevel.
  A worktree prepared from a linked checkout previously failed its own
  ownership check on release and its lease could never be cleared.
- osProcessAlive on Windows no longer treats every OpenProcess failure
  as "process is dead": only ERROR_ACCESS_DENIED (a live process this
  caller lacks rights to query) is now distinguished from a genuinely
  missing PID, so Clean can no longer force-remove an active worktree
  whose owning process it simply couldn't query.
- The openai_key redaction pattern now recognizes sk-or-v1- (OpenRouter)
  alongside the existing sk-proj-/sk-svcacct-/sk-admin- prefixes, so
  hyphenated OpenAI-compatible provider keys are redacted again without
  reopening the sk-<kebab-case-phrase> false-positive this pattern was
  narrowed to avoid.
- Release/exec --worktree error text is redacted before reaching
  stderr, matching the already-redacted success path; ownership errors
  interpolate the caller-supplied path, which could carry a key-shaped
  segment.
- canonicalizePath resolves symlinks through the nearest existing
  ancestor when the target itself no longer exists, so the documented
  `release -C` recovery path works again for a worktree deleted by hand
  under a symlinked --worktree-dir.
- Prepare rolls back the worktree `git worktree add` just created if the
  subsequent lock call fails for a reason other than a concurrent
  racer, instead of leaking an unleased checkout until Clean's 24h
  staleness window reclaims it.

Not addressed here: the P2 finding that worktree ownership is provable
only by a directory-name convention plus a lock-reason prefix, both of
which a user can reproduce by hand. A durable per-worktree ownership
marker would close that gap, but internal/worktrees has been on main
since Gitlawb#70, so a marker requirement could reject worktrees an
already-installed zero created before this change existed. Needs a
decision on migration before implementing.
kevincodex1 pushed a commit that referenced this pull request Aug 10, 2026
…worktrees (#855)

* fix(secrets,worktrees): fix secret redaction leakage and prune stale worktrees

1. Prevent trailing redaction leaks in github_token, aws_access_key_id, and google_api_key by adding trailing word boundaries and allowing variable lengths. Refine the openai_key pattern to cleanly distinguish legacy keys and modern prefixed keys (sk-proj-, sk-svcacct-) from ordinary kebab-case phrases.
2. Implement auto-pruning of zero-owned git worktrees older than 24 hours at the start of worktrees.Prepare to prevent indefinite disk space leaks.

* fix(secrets,worktrees): close tail-leak edge case and worktree data-loss risk

Drop the trailing \b anchor on the four secret patterns whose body
class allows "-" (slack_token, google_api_key, the modern openai_key
branch, jwt). \b requires a word/non-word transition, so a secret
ending in "-" right before a delimiter has none, and the engine
backtracked the greedy quantifier to drop that last character instead
of failing the match, leaking it. The body character class already
provides the real stopping boundary, so the anchor was unnecessary.

Fix two issues in worktree Clean flagged in review:

- Staleness was decided by the worktree directory's own mtime, which
  only changes when an entry is added/removed/renamed directly inside
  it, not when a long-running task edits existing files deeper in the
  tree. Clean now walks the tree and treats any recently modified entry
  as live, and also skips any worktree a caller has explicitly locked
  via git worktree lock.
- baseDir ownership used a raw strings.HasPrefix, so a sibling like
  "<baseDir>-other" would false-match. Replaced with a filepath.Rel
  path-boundary check.

* fix(worktrees): check exit codes on removal, fail closed on inspection errors

defaultRunGit deliberately returns a nil error alongside a nonzero
CommandResult.ExitCode for a failed git invocation, so the worktree
remove call must check ExitCode itself instead of trusting a nil error
to mean success. Route it through gitOutput, which already does that.

worktreeIsStale treated an inspection failure (an unreadable file, a
WalkDir error) the same as "keep walking," which can let an
incompletely-inspected worktree be judged stale. Any inspection error
now makes it ineligible for removal instead.

* fix(secrets,worktrees): catch appended-suffix keys and scope pruning to owned worktrees

The scanner's trailing \b anchors made a credential vanish entirely when
followed by a word character outside its body class (an appended suffix
like AKIA...EXTRA, or ghp_..._suffix): the fixed or unbounded-greedy
quantifier had no valid word boundary to land on and the whole match
failed, so the real secret reached the redaction output unredacted.
Dropping the trailing anchors lets the body class itself stop the match,
so the credential prefix still gets redacted even when noise follows it.
Also recognize sk-admin- alongside sk-proj-/sk-svcacct- so OpenAI admin
keys aren't skipped by the narrowed modern-key branch.

Clean pruned any worktree under the caller-supplied BaseDir, but Prepare
only ever creates worktrees under a per-repository
zero-worktree-<repoKey> subtree of it. Scope pruning to that subtree so a
worktree a user manages by hand elsewhere under a shared BaseDir is never
force-removed. Also refuse to force-remove a worktree whose mtime looks
stale but that still has uncommitted or untracked changes: a task can
hold live work while waiting on a model, network, or user for longer
than the staleness window without writing to the tree again.

* fix(worktrees): make nested-activity test exercise the deep walk it claims to

activePath/internal was created by the same MkdirAll as the nested pkg
dir but never backdated, so it kept a fresh mtime and worktreeIsStale's
walk reported "not stale" as soon as it hit that directory, before ever
reaching the freshly-written file two levels deeper. The test passed
without actually exercising recursion past the first directory.

* fix(worktrees): lock zero-created worktrees and treat ignored files as dirty

Prepare never called git worktree lock, so the entry.locked skip in Clean
only ever protected worktrees a human locked by hand, never zero's own; a
worktree that finished committing and sat idle (e.g. waiting on a slow
model or network retry) for more than 24h looked clean-and-stale and got
force-removed by the mtime+dirty heuristic alone. Lock every worktree
Prepare creates so it gets the same protection.

worktreeIsDirty also used git status --porcelain with no --ignored, so a
worktree holding only .gitignore-matched task data (credentials, generated
drafts, artifacts) reported as clean and got force-removed with --force,
silently discarding it. Add --ignored so those files count as dirty too.

* fix(worktrees): release Zero's Prepare lock so Clean can reclaim finished worktrees

Prepare locks every worktree it creates so Clean's mtime+dirty staleness
heuristic never force-removes one Zero is still using, but nothing ever
unlocked it, making the automatic disk-space cleanup permanently inert.

Add Release (git worktree unlock) and wire it in two ways: zero exec
--worktree defers a release once its own run finishes, since that flow's
use of the worktree is bound to its own process. zero worktrees prepare
hands the path to a longer-lived external caller with no defined
end-of-life, so a new zero worktrees release <path> subcommand lets that
caller release it explicitly when done.

* fix(worktrees): normalize release path, aggregate Clean errors, unlock deleted worktrees

Address CodeRabbit's review on the lock-release fix:

- zero worktrees release now resolves its path argument to absolute
  before calling Release, since git worktree unlock matches against
  the path git recorded at creation, not whatever directory the
  caller happens to be running from.
- Clean now aggregates removal failures with errors.Join instead of
  overwriting lastErr, so multiple stale worktrees failing removal in
  the same pass are all reported, not just the last one.
- Release falls back to options.Cwd as the git working directory when
  the worktree path itself no longer exists (e.g. a caller deleted a
  locked worktree by hand instead of releasing it first), so the
  orphaned lock can still be cleared.

Added regression coverage for all three.

* style(worktrees): align aggregation test comments to gofmt output

* fix(worktrees,cli): restore reuse lease and scope unlock to owned locks

- Prepare re-locks a reused worktree so Clean's staleness heuristic cannot
  force-remove it while the new caller is still using it; a lock already
  held by a live external caller is kept in place and reported through the
  new Result.LockAcquired field.
- exec --worktree only releases the lock its own Prepare call acquired and
  surfaces a failed release on stderr with the affected path instead of
  discarding the error.
- worktrees release wires the resolved workspace root into Options.Cwd so
  the deleted-path recovery works outside the worktree directory.
- The relative-path release test derives its expected value via
  filepath.Abs, matching the resolution the CLI uses, so macOS /var vs
  /private/var spellings no longer break it.

* fix(worktrees,cli): reject in-use leases, validate before cleanup, add release -C

- Prepare rejects a worktree whose lock another run still holds, on both
  the reuse and the create-race paths, instead of handing a second live
  caller an unprotected shared checkout whose sole Git lock the first
  caller's exit would release.
- The automatic stale-worktree pruning runs only after the request itself
  validates, so a rejected command (an invalid --name) has no destructive
  cleanup side effect; covered end to end with real git in both directions.
- worktrees release accepts -C/--cwd naming the source repository, which
  the deleted-path recovery needs when launched outside the repo (the
  deleted worktree path is a one-way hash with no way back to its source).

* test(worktrees): canonicalize test roots to physical spelling

git records worktree paths in physical form, so the CI runners' symlinked
(/var -> /private/var) and 8.3-short (RUNNER~1) temp spellings made Clean's
containment check skip the test's stale entry and the pruning assertion
fail on macOS and Windows.

* fix(worktrees): preserve orphaned commits, verify release ownership, canonicalize base dir

Four fixes from the latest review round:

- Clean now creates a durable ref (refs/zero/orphaned-worktree/<sha>)
  for a detached worktree's HEAD before force-removing it, when that
  commit isn't already reachable from any other ref. Prepare always
  creates worktrees with `worktree add --detach`, so a commit made
  there had no ref pointing at it once the worktree was deleted,
  making it immediately eligible for git gc despite never having been
  merged/pushed elsewhere.

- Clean resolves its configured base directory through EvalSymlinks
  before comparing it against git's reported worktree paths: `git
  worktree list --porcelain` reports each worktree's PHYSICAL location
  (resolving symlink components), so a symlinked --worktree-dir made
  every worktree created under it permanently unprunable.

- Release now verifies path has a zero-worktree-<repoKey> ancestor
  directory component before running `git worktree unlock`, so it
  can't be used to clear the lock on a worktree a user or another tool
  manages by hand. The check doesn't need to know which --dir a given
  Prepare call used (nothing records that against a specific worktree,
  and the CLI never threads BaseDir through to Release) — the repoKey
  component is Prepare's actual ownership signature regardless of
  which directory it was created under.

- Route the release command's printed path and exec's release-failure
  diagnostic through the existing CLI redaction helper, and split the
  worktrees help text into prepare-specific and release-specific flag
  sections (release only ever supported -C/--cwd, not the --name/--dir/
  --json the shared block advertised).

All four have regression tests confirmed to fail without their fix
(two using real git worktrees, not just fakeRunner sequences). Build,
vet, and gofmt clean on linux/windows/darwin.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(worktrees): recoverable PID leases and reclaimable released worktrees

Two review findings on the cleanup lifecycle:

- A lock left by an abnormal exit (SIGKILL, crash, power loss) was
  skipped by Clean forever, recreating the permanent disk leak this PR
  set out to fix. exec --worktree now records its PID in the lease
  reason; Clean expires a lease whose recorded owner is provably dead,
  unlocking only after the staleness, dirty, and HEAD-preservation
  guards all pass. Human locks and PID-less leases (external
  `worktrees prepare` owners) remain permanent until explicit release,
  and any ambiguity in the liveness probe counts as alive.

- An explicitly released worktree holding only gitignored residue
  (node_modules, build output) was skipped at every age. Release is the
  owner's completion signal, so unlocked entries now block removal only
  on tracked/untracked changes; expired crashed leases keep the
  conservative --ignored probe since they never signaled completion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(worktrees): address review feedback on lease detection and release safety

Split dead-lease PID checking into posix/windows implementations so
Windows can reliably tell a dead process from a live one. Fix a path
canonicalization mismatch in two release tests. Derive release
ownership from git worktree list instead of the git-dir parent, which
was wrong for repos with a separate git-dir. Refuse to clear a lock
that was not taken by Zero in the first place.

* fix(worktrees): canonicalize paths for clean/release ownership checks

Compare Clean containment and Release ownership against physical path
spellings so macOS /var vs /private/var and symlink TMPDIR layouts match
git worktree list. Require a registered porcelain entry and a Zero lease
reason before unlock; treat an already-unlocked Zero worktree as a no-op.

* fix(worktrees,secrets): address jatmn review findings on #632

- Prepare and Release now agree on repoKey regardless of which worktree
  (main or linked) Prepare runs from, by keying off git worktree list's
  first entry (always the main worktree) instead of --show-toplevel.
  A worktree prepared from a linked checkout previously failed its own
  ownership check on release and its lease could never be cleared.
- osProcessAlive on Windows no longer treats every OpenProcess failure
  as "process is dead": only ERROR_ACCESS_DENIED (a live process this
  caller lacks rights to query) is now distinguished from a genuinely
  missing PID, so Clean can no longer force-remove an active worktree
  whose owning process it simply couldn't query.
- The openai_key redaction pattern now recognizes sk-or-v1- (OpenRouter)
  alongside the existing sk-proj-/sk-svcacct-/sk-admin- prefixes, so
  hyphenated OpenAI-compatible provider keys are redacted again without
  reopening the sk-<kebab-case-phrase> false-positive this pattern was
  narrowed to avoid.
- Release/exec --worktree error text is redacted before reaching
  stderr, matching the already-redacted success path; ownership errors
  interpolate the caller-supplied path, which could carry a key-shaped
  segment.
- canonicalizePath resolves symlinks through the nearest existing
  ancestor when the target itself no longer exists, so the documented
  `release -C` recovery path works again for a worktree deleted by hand
  under a symlinked --worktree-dir.
- Prepare rolls back the worktree `git worktree add` just created if the
  subsequent lock call fails for a reason other than a concurrent
  racer, instead of leaking an unleased checkout until Clean's 24h
  staleness window reclaims it.

Not addressed here: the P2 finding that worktree ownership is provable
only by a directory-name convention plus a lock-reason prefix, both of
which a user can reproduce by hand. A durable per-worktree ownership
marker would close that gap, but internal/worktrees has been on main
since #70, so a marker requirement could reject worktrees an
already-installed zero created before this change existed. Needs a
decision on migration before implementing.

* fix(worktrees): prove Prepare ownership with a git-admin marker

Address review findings that path convention plus lease-reason prefix are
forgeable by hand. Prepare now writes a zero-owner marker into the worktree
admin dir; Release and Clean require it before force-touching a path. Clean
also keys its owned subtree off the main worktree root so linked-checkout
calls still prune the same bucket Prepare uses. Tests plant the marker and
list the main worktree first so fixtures match production.

* fix(cli): complete worktrees release in shell completions

* test(worktrees,cli): add coverage for linked-worktree Clean, forged lease rejection, and completions

- TestCleanFromLinkedWorktreePrunesStaleWorktree: pins Clean deriving its
  owned-subtree key from the main worktree root (not the invoking linked
  checkout's --show-toplevel) so Prepare/Clean run from a linked worktree
  actually reclaim the worktrees Prepare created there.
- TestReleaseRejectsForgedZeroLeaseWithoutOwnershipMarker: pins the
  ownership-marker requirement against the exact forgery jatmn described - a
  worktree under the predictable zero-worktree-<repoKey> path, manually
  locked with a reason that merely starts with the zero lease prefix.
- Fix TestPrepareCreatesDetachedGitWorktree: its fake git runner ran out of
  canned results at Prepare's post-lock ownership-marker write, so
  writeOwnershipMarker resolved gitDir to "" and os.WriteFile wrote
  "zero-owner" as a relative path into the test process's real working
  directory instead of failing loudly. Give it autoAbsoluteGitDir like the
  other Prepare-exercising tests use and assert on the marker-write call.
- completions_test.go: assert `worktrees`/`worktree` completions include
  `release` alongside `prepare`.

* fix(redaction,worktrees): sort extra secret values by length descending and canonicalize worktree paths

* fix(secrets,worktrees): restore Anthropic key redaction and handle legacy worktree cleanup

* fix(secrets): add bash output redaction regression test for Anthropic API keys

* fix(worktrees): touch worktree mtime on reuse to prevent stale pruning

When Prepare reuses an existing worktree, it now touches the directory
to refresh its mtime. This prevents Clean from force-removing a
long-running but idle worktree (e.g. waiting on a model) that has no
recent file changes but is still actively in use.

Addresses the data-loss risk flagged in the PR review where mtime-only
staleness + --force removal could discard live worktrees.

Co-authored-by: cairn-code
Co-Authored-By: cairn-code <282421612+cairn-code@users.noreply.github.com>

* fix(worktrees): properly handle os.Chtimes error on reused worktree path

Co-Authored-By: cairn-code <282421612+cairn-code@users.noreply.github.com>

* fix(worktrees,secrets): unlock using porcelain entry path, set RepoRoot to primaryRoot, and add anthropic_key pattern

Unlock using matched porcelain entry path, set Result.RepoRoot to primaryRoot in Prepare, add dedicated anthropic_key secret pattern, and canonicalize fake-runner test paths.

Refs #632

* fix(tools): correct redaction placeholder assertion in Anthropic key test

TestFormatBashOutputRedactsAnthropicKey checked for the openai_key
placeholder instead of anthropic_key, a copy-paste leftover. The
redaction itself was already correct; only the assertion was wrong.

* test(worktrees): physicalize test paths for macOS tempdir symlink resolution

* fix(secrets,worktrees): address CodeRabbit review findings

Align RedactString token boundaries with secrets.Scan, strengthen the
Anthropic bash redaction assertion, fix Clean fixtures so they reach
the guards under test, and fail closed on ambiguous processAlive probes.

* fix(secrets,worktrees): restore broad key redaction and legacy Clean safety

Address human review on #855: keep sk- bodies with a digit filter instead
of enumerated vendor prefixes, add a looser JWT form, restore the
sk-test fixture, probe legacy ownership before dirty, treat non-INVALID
Windows OpenProcess errors as alive, redact Abs/cwd release errors, and
reclaim dead-owner leases on Prepare reuse.

* fix(secrets,redaction): always redact known OpenAI key prefixes

Alphabet-only sk-proj-/sk-svcacct-/sk-admin- tokens are still credentials;
keep the digit filter only for unknown sk- vendor forms so kebab phrases
like sk-learn-… stay un-redacted.

* Preserve digit-free legacy keys during redaction.

Refs #855

* fix(secrets,worktrees): address CodeRabbit findings on PR #855

Redact full compact JWE tokens, pin git locale for lock parsing, write the
ownership marker atomically, surface missing-dir unlock failures, and
fail closed when HEAD probes are indeterminate. Align Clean fixtures and
digit-free known-prefix redaction tests with the shipped behavior.

Refs #855

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: cairn-code <282421612+cairn-code@users.noreply.github.com>
Co-authored-by: euxaristia <euxaristia@users.noreply.github.com>
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.

2 participants