feat(sync): validate sparse-checkout scope before leasing

[coygeek]

Summary

Validate sparse-checkout and skip-worktree omissions before Crabbox acquires, resolves, prepares, or borrows an SSH lease for an ordinary local sync. Keep the existing post-acquisition manifest rebuild so files created during acquisition still participate, and make the rejection name the supported recovery paths.

Problem to solve

Crabbox correctly fails closed when a tracked path is hidden by sparse-checkout or skip-worktree state but remains in the effective sync manifest. sync-plan detects that condition locally, but the error does not explain how to recover. A sync-enabled run discovers the same local condition only after provisioning and preparing a lease.

This reproduces against exact current main:

git clone https://github.com/openclaw/crabbox.git
cd crabbox
git checkout 58323474be23508b971c5e5f1fb3eb43dd6b97af
go build -trimpath -o bin/crabbox-main ./cmd/crabbox
crabbox_main="$PWD/bin/crabbox-main"

repro_dir="$(mktemp -d)"
cd "$repro_dir"
git init
git config user.name "Crabbox sparse repro"
git config user.email "repro@example.invalid"
mkdir -p visible hidden
printf 'visible\n' > visible/keep.txt
printf 'hidden\n' > hidden/drop.txt
git add visible/keep.txt hidden/drop.txt
git commit -m "test: add sparse fixture"
git sparse-checkout init --cone
git sparse-checkout set visible

"$crabbox_main" sync-plan

sync-plan exits 6 with:

build sync file list: tracked path "hidden/drop.txt" is hidden by sparse checkout or skip-worktree state but remains in sync manifest scope

The message identifies the safety condition but does not say that the checkout can be materialized or the hidden path can be removed from the effective manifest with sync.include, sync.exclude, or .crabboxignore.

Running the equivalent workload provisions before returning the same local error:

"$crabbox_main" run --provider local-container --target linux -- true

The exact-main reproduction spent 23.5 seconds creating an Ubuntu 26.04 ARM64 container, waiting for SSH, and printing a run context before releasing the lease and exiting 6. The user command never ran. The lease cleanup completed successfully.

Two controls restore the local plan. git sparse-checkout disable materializes both tracked paths and makes sync-plan report two candidate files. Keeping the checkout sparse while excluding hidden/drop.txt through .crabboxignore also succeeds and reports only the visible tracked file plus the root ignore file.

Proposed behavior

Before Crabbox mutates provider state for an ordinary Git-backed sync, run a bounded local validation that detects in-scope sparse-checkout and skip-worktree omissions. If validation fails, return the existing exit class and path evidence without acquiring, resolving, preparing, or borrowing a lease.

Keep rebuilding the authoritative manifest after acquisition. The current post-acquisition rebuild includes files created while a lease is being acquired, and the new preflight must not turn that throwaway validation result into the transfer manifest.

Make the rejection actionable. It should direct the user to materialize a full checkout, or to remove the hidden path from the effective manifest with an include or ordered exclude when that path is intentionally outside the remote workload. A concise documentation link is enough if the CLI text remains bounded.

Acceptance criteria

• A Git checkout with an in-scope hidden tracked file exits 6 before any new SSH lease acquisition or ready-pool borrow.
• Existing-lease routes reject the same local input before provider resolve or prepare work that is unnecessary for the failed sync.
• The diagnostic retains the affected path and explains both supported recovery categories: materialize the checkout, or remove the path from effective sync scope.
• sync-plan and sync-enabled run use consistent recovery guidance for the same hidden path.
• Hidden paths outside sync.include or removed by ordered excludes remain accepted.
• A sparse configuration that materializes every tracked path remains accepted.
• The authoritative manifest is still rebuilt after successful acquisition, and files created during acquisition remain included.
• --no-sync, fresh remote checkouts, delegated providers that own synchronization, and other paths that do not use the ordinary local manifest retain their current behavior.
• A failed local sparse preflight creates no provider lease, container, or ready-pool side effect.
• Focused tests cover new acquisition, existing-lease preparation, ready-pool borrowing, actionable error text, excluded hidden paths, fully materialized sparse checkouts, and the post-acquisition rebuild control.

Affected area

• Local sync-manifest validation in internal/cli/repo.go.
• Pre-acquisition routing and the post-acquisition manifest rebuild in internal/cli/run.go.
• Sparse-manifest and run-ordering coverage in internal/cli/repo_test.go and internal/cli/run_test.go.
• Sparse-checkout recovery guidance in docs/commands/sync-plan.md and docs/features/sync.md.

Non-goals

• Syncing an in-scope tracked file whose contents are absent locally.
• Automatically disabling sparse checkout or changing Git index state.
• Reusing a pre-acquisition file list as the final transfer manifest.
• Changing ordered include or exclude semantics.
• Adding arbitrary non-Git directory synchronization, which remains tracked separately in issue #1334.
• Weakening the fail-closed protection added for sparse and skip-worktree omissions.

Alternatives considered

Document the recovery only

Documentation would help users who already know where to look, but run would still provision a lease for a locally knowable error. The existing non-Git sync preflight already establishes that local manifest-source failures can stop before provider work.

Build the final manifest before acquisition

This avoids duplicate local work but creates a stale transfer list. Current regression coverage requires files created during acquisition to appear in the final sync. A bounded validation followed by the existing rebuild preserves both behaviors.

Materialize the checkout automatically

Changing sparse-checkout or index state mutates the user's Git workspace and can pull a much larger tree than intended. Crabbox should explain the choice and leave that mutation to the caller.

Supporting context

• Release baseline: v0.46.0 at 8ba71f913bbe57285ae29af45ef0d8ec6712477d.
• Exact current main: 58323474be23508b971c5e5f1fb3eb43dd6b97af.
• Current-main binary SHA-256: a2e42cd24c453fffa6b51cb074ba311632a166acf3dad6308b4266f401ed615e.
• Reproduction host: macOS ARM64 with Go 1.26.5 and Docker 29.7.2; disposable target: Ubuntu 26.04 ARM64 through local-container.

The complete 65-commit range from the release baseline through exact current main, all 24 open issues, all 16 open pull requests, every open issue authored by coygeek, and relevant closed issues and merged pull requests were checked. No current item covers pre-acquisition sparse-scope validation plus actionable recovery guidance.

Merged pull request #1241 added the exact fail-closed sparse-manifest protection and deliberately preserved post-acquisition manifest behavior. Merged pull request #1253 is the closest lifecycle precedent: it made the non-Git manifest-source error actionable and rejected that locally knowable condition before provider work while still rebuilding the transfer manifest later. Neither change covers the sparse-scope ordering and recovery gap above.

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed August 28, 2026, 1:39 AM ET / 05:39 UTC.

Summary
Keep open. Current main still performs only a Git-source check before leasing; the scope-aware sparse/skip-worktree rejection occurs after lease acquisition, so the reported unnecessary provider work remains possible.

Reproducibility: yes. —source-reproducible with high confidence: an in-scope hidden tracked path passes the early gitSyncFileList check and reaches the later scope-aware manifest failure after lease routing begins.

Root-cause cluster
Relationship: canonical
Canonical: #1567
Summary: This is the canonical open report for the remaining sparse-manifest lifecycle-ordering gap.

Members:

• partial_overlap: fix(sync): stop sparse checkouts from deleting scoped files #1241 - It implemented fail-closed sparse-manifest detection but not validation before lease lifecycle work.
• partial_overlap: fix(sync): report actionable error for non-Git workdir #1253 - It moved non-Git manifest-source errors before lease work but its helper does not evaluate sparse effective scope.
• adjacent_distinct: feat(sync): support explicit synchronization from non-Git workspaces #1334 - It requests opt-in synchronization for non-Git directories, a separate feature from validating Git sparse scope earlier.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Next step
The source establishes a bounded repair, but the bulk-filed and no-new-fix-PR routing signals mean it should remain available for maintainer selection rather than automated fix dispatch.

Review details

Best possible solution:

Share a lightweight scope-aware validation between sync-plan and pre-lease run routing, retain the existing final manifest rebuild, and cover new, existing-lease, and ready-pool paths with actionable recovery text.

Do we have a high-confidence way to reproduce the issue?

Yes—source-reproducible with high confidence: an in-scope hidden tracked path passes the early gitSyncFileList check and reaches the later scope-aware manifest failure after lease routing begins.

Is this the best way to solve the issue?

Yes. Extending the existing early local-source preflight with throwaway scope validation is the narrowest solution, provided the post-acquisition authoritative rebuild remains unchanged.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• The early check must validate effective scope without retaining its manifest, because the final manifest deliberately includes files created during lease acquisition.

Codex review notes: model internal, reasoning high; reviewed against 58323474be23.

Label changes

Label changes:

• remove clawsweeper:fix-shape-clear: Current issue advisory state no longer selects this label.

Label justifications:

• P2: An invalid local checkout can currently trigger avoidable lease provisioning and preparation before failing.
• impact:other: The concrete impact is unnecessary provider resource activity, outside the more specific owned impact classes.

Evidence reviewed

What I checked:

• Current preflight gap: The SSH-run path invokes validateLocalWorkspaceSyncSource before lease work, but that helper only calls gitSyncFileList; it cannot evaluate effective sparse-checkout scope. (internal/cli/run.go:948, 58323474be23)
• Late sparse validation: Effective include/exclude scope and hidden sparse or skip-worktree paths are evaluated by syncManifestFilteredRules, which the run path calls only after acquisition/resolve work. (internal/cli/repo.go:994, 58323474be23)
• Existing behavioral controls: Tests already establish the sparse-scope distinction and separately prove that non-Git errors stop before acquire, ready-pool borrow, and existing-lease preparation; no equivalent sparse pre-acquisition test exists in these controls. (internal/cli/run_test.go:1085, 58323474be23)
• Intentional rebuild contract: The current run test proves that a file created during acquisition appears in the final rsync manifest, so an early scope check must remain throwaway rather than become the transfer manifest. (internal/cli/run_test.go:1147, 58323474be23)
• Feature provenance: The non-Git pre-acquisition helper was introduced by merged pull request fix(sync): report actionable error for non-Git workdir #1253; the sparse-manifest protection dates to merged pull request fix(sync): stop sparse checkouts from deleting scoped files #1241. Their current combination leaves the reported ordering gap. (internal/cli/run.go:948, d6731ba47744)
• Current documentation contract: Documentation confirms sparse omissions are rejected only when in effective scope, but it does not make the CLI rejection actionable with the two proposed recovery categories. (docs/commands/sync-plan.md:38, 58323474be23)

Likely related people:

• bunlongheng: Merged pull request fix(sync): report actionable error for non-Git workdir #1253 introduced the pre-lease Git-source validation that this repair should extend. (role: introduced early local-manifest validation; confidence: high; commits: d6731ba47744; files: internal/cli/run.go, internal/cli/repo.go, internal/cli/run_test.go)
• vincentkoc: Merged pull request fix(sync): stop sparse checkouts from deleting scoped files #1241 introduced the sparse-manifest safety behavior, and later sync work continues in the same paths. (role: sparse-manifest safety owner and recent sync contributor; confidence: high; commits: 1568766f0782, 24d76cb35dc4; files: internal/cli/repo.go, internal/cli/repo_test.go, internal/cli/run.go)

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.