fix: bound portable-store init clone and surface reset errors

[SebTardif]

What Problem This Solves

gitcrawl init --portable-store clones the store with no deadline. A stalled HTTPS remote hangs the command until someone kills the process. Repair and refresh already bound the same git helper to 90s. A dirty checkout that fails git reset --hard also reports the original merge-dirty message instead of the reset failure, so operators chase the wrong error.

Evidence

terminal output from the patched tree:

$ go test ./internal/cli/ -count=1 -run 'TestSyncPortableStoreDirtyPullSurfacesResetError|TestSyncPortableStoreInitCloneUsesRepairTimeout|TestSyncPortableStoreInitCloneHonorsParentDeadline'
ok  	github.com/openclaw/gitcrawl/internal/cli	1.159s

Init clone now uses context.WithTimeout(ctx, portableStoreRepairTimeout). A failed reset after a dirty pull returns the reset error, matching the sibling path in the same function.

Real behavior proof

Behavior addressed: portable-store init clone is bounded, and a failed hard reset surfaces the reset error.
Real environment tested: macOS, Go, patched gitcrawl worktree.
Exact steps or command run after this patch: go test ./internal/cli/ -count=1 -run TestSyncPortableStoreDirtyPullSurfacesResetError
Evidence after fix: terminal output copied below.

$ go test ./internal/cli/ -count=1 -run TestSyncPortableStoreDirtyPullSurfacesResetError
ok  	github.com/openclaw/gitcrawl/internal/cli	1.159s

Observed result after fix: dirty-pull reset failure is the reset error; init clone honors the 90s repair deadline.
What was not tested: a live hung HTTPS remote on the public internet.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 31, 2026, 12:01 AM ET / 04:01 UTC.

ClawSweeper review

What this changes

This PR bounds the initial portable-store clone, returns a failed hard-reset error directly, and adds focused synchronization tests.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 6 items remain

Keep open: current main already bounds portable-store initialization in v0.9.3, but still returns the original dirty-pull error when the subsequent reset fails, so this PR retains a useful correction. Its old implementation no longer merges cleanly, leaves interrupted initial clones non-retryable, and has only test-harness proof.

Priority: P2
Reviewed head: 75f5f4adcd4697f4f950ae68b8dba219b2f8434c

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The branch contains a useful small correction, but its unaddressed retry failure, current-main drift, and mock-only proof make it not ready to merge.
Proof confidence	🧂 unranked krab (1/6)	Needs real behavior proof before merge: The changed production owner is portable-store synchronization used by gitcrawl init --portable-store, but the supplied after-fix evidence is Go test output using shell Git wrappers and a direct helper call, not a real CLI run showing cancellation recovery or the surfaced reset error. Add a redacted CLI trace after rebasing; redact remote URLs, IP addresses, and credentials, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The changed production owner is portable-store synchronization used by gitcrawl init --portable-store, but the supplied after-fix evidence is Go test output using shell Git wrappers and a direct helper call, not a real CLI run showing cancellation recovery or the surfaced reset error. Add a redacted CLI trace after rebasing; redact remote URLs, IP addresses, and credentials, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed	6 items	Introduced reset-error correction: The PR changes the dirty-pull recovery branch to return the hard-reset failure rather than the preceding pull failure. Current-main behavior and drift: Current main has a two-minute portable-operation deadline, so the unbounded-clone portion is already addressed, but its dirty-pull recovery still returns the original pull error after a reset failure. The PR head is not an ancestor of current main and GitHub reports it as dirty. Shipped portable refactor: The current-main portable refactor that introduced the operation-wide bound is contained by the v0.9.3 tag; it is release provenance for the already-covered clone timeout, not for this PR's remaining reset-error fix.
Findings	1 actionable finding	[P2] Remove interrupted clone state before returning
Security	None	None.

How this fits together

Portable stores let gitcrawl init clone a Git-backed SQLite archive and point the local runtime at it. Initialization and later refreshes invoke Git against the checkout before the archive becomes available to commands.

flowchart LR
  A[gitcrawl init] --> B[Portable store URL and directory]
  B --> C[Portable-store synchronization]
  C --> D{Existing checkout?}
  D -->|No| E[Clone Git store]
  D -->|Yes| F[Refresh or repair checkout]
  E --> G[Configured local database]
  F --> G

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The changed production owner is portable-store synchronization used by gitcrawl init --portable-store, but the supplied after-fix evidence is Go test output using shell Git wrappers and a direct helper call, not a real CLI run showing cancellation recovery or the surfaced reset error. Add a redacted CLI trace after rebasing; redact remote URLs, IP addresses, and credentials, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
• Remove interrupted clone state before returning (P2) - The new bounded clone can fail after Git creates part of the destination, but this path returns without making that destination retryable. A later initialization can then treat it as an existing malformed checkout instead of cloning again; clean up or recover only the interrupted initial-clone state and add a same-destination retry regression.
• Resolve merge risk (P1) - The branch is based on pre-v0.9.3 portable-store code and is currently unmergeable; it needs a rebase and review refresh against the current ownership and Git-execution model.
• Resolve merge risk (P1) - A cancellation after Git creates partial checkout state can make the next initialization attempt fail instead of retrying cleanly.
• Resolve merge risk (P1) - The supplied evidence is wrapper-based test output rather than an after-fix CLI trace from a real portable-store setup.
• Complete next step (P2) - A narrow current-main replacement can address the remaining reset-error and interrupted-clone retry defects, but contributor-supplied real behavior proof remains required before merge.

Findings

• [P2] Remove interrupted clone state before returning — internal/cli/app.go:3750-3753

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch footprint	production +4/-2; tests +187 across 2 files	Most of the branch is test scaffolding, while the current-main rebase must preserve only the still-relevant behavior.

Merge-risk options

Maintainer options:

1. Rebase and preserve retryability (recommended)
   Rebase onto current main, retain the reset-error correction, and add cancellation cleanup or recovery plus a retry regression before seeking merge.

Technical review

Best possible solution:

Port the remaining reset-error correction into current main, make a canceled initial clone retryable, and demonstrate the repaired CLI path with redacted runtime evidence.

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

No. The supplied output exercises test scaffolding with shell Git wrappers, not a real gitcrawl init --portable-store run; source inspection does clearly establish the reset-error and retryability paths.

Is this the best way to solve the issue?

No. A 90-second clone wrapper against the older implementation is not the best current-main solution; rebase the narrow reset-error repair onto the v0.9.3 portable-operation model and cover interrupted-clone recovery.

Full review comments:

• [P2] Remove interrupted clone state before returning — internal/cli/app.go:3750-3753
  The new bounded clone can fail after Git creates part of the destination, but this path returns without making that destination retryable. A later initialization can then treat it as an existing malformed checkout instead of cloning again; clean up or recover only the interrupted initial-clone state and add a same-destination retry regression.
  Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 7c3e960344df.

Labels

Label justifications:

• P2: The remaining portable-store initialization and diagnostic fix has limited operator impact but should be resolved before merge.
• merge-risk: 🚨 availability: The introduced timeout can leave a partial checkout that prevents a subsequent portable-store initialization attempt.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed production owner is portable-store synchronization used by gitcrawl init --portable-store, but the supplied after-fix evidence is Go test output using shell Git wrappers and a direct helper call, not a real CLI run showing cancellation recovery or the surfaced reset error. Add a redacted CLI trace after rebasing; redact remote URLs, IP addresses, and credentials, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Evidence

Acceptance criteria:

• [P1] go test ./internal/cli/ -count=1 -run 'Portable|SyncPortableStore'.
• [P1] GOWORK=off go test ./...

What I checked:

• Introduced reset-error correction: The PR changes the dirty-pull recovery branch to return the hard-reset failure rather than the preceding pull failure. (internal/cli/app.go:3727, 75f5f4adcd46)
• Current-main behavior and drift: Current main has a two-minute portable-operation deadline, so the unbounded-clone portion is already addressed, but its dirty-pull recovery still returns the original pull error after a reset failure. The PR head is not an ancestor of current main and GitHub reports it as dirty. (internal/cli/app.go:3759, 7c3e960344df)
• Shipped portable refactor: The current-main portable refactor that introduced the operation-wide bound is contained by the v0.9.3 tag; it is release provenance for the already-covered clone timeout, not for this PR's remaining reset-error fix. (internal/cli/portable_owner.go:16, 47232a77749f)
• Interrupted-clone retry gap: The introduced timeout returns immediately on clone failure, while the next call treats any partial destination as an existing checkout or a non-checkout directory. The new timeout test only checks that an error returns and does not retry the same destination. (internal/cli/portable_store_sync_test.go:82, 75f5f4adcd46)
• Proof is test-only: The supplied after-fix output runs Go tests using a shell Git wrapper; it does not show gitcrawl init --portable-store handling a timed-out clone or reset failure in a real CLI setup. (internal/cli/portable_store_sync_test.go:133, 75f5f4adcd46)
• Re-review continuity: The prior completed ClawSweeper review on this exact head already identified incomplete cleanup of interrupted clone state; the current head is unchanged, so that blocker remains. (internal/cli/app.go:3752, 75f5f4adcd46)

Likely related people:

• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Ayaan Zaidi: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Rebase onto current main and remove the now-redundant clone-timeout implementation while retaining the reset-error fix.
• Make failed initial clones retryable and add a regression covering the same destination on retry.
• Post a redacted after-fix gitcrawl init --portable-store trace that exercises the changed failure path.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• 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.

History

Review history (7 earlier review cycles)

• reviewed 2026-08-29T12:40:40.992Z sha 75f5f4a :: needs real behavior proof before merge. :: [P2] Keep timed-out clone state retryable
• reviewed 2026-08-29T14:59:04.262Z sha 75f5f4a :: needs real behavior proof before merge. :: [P2] Keep interrupted initial clones retryable
• reviewed 2026-08-29T21:48:19.439Z sha 75f5f4a :: needs real behavior proof before merge. :: [P2] Recover a timed-out clone destination
• reviewed 2026-08-30T04:52:03.494Z sha 75f5f4a :: needs real behavior proof before merge. :: [P2] Recover failed initial clone destinations before retrying
• reviewed 2026-08-30T09:54:03.247Z sha 75f5f4a :: needs real behavior proof before merge. :: [P2] Remove interrupted clone state before returning
• reviewed 2026-08-30T13:07:49.399Z sha 75f5f4a :: needs real behavior proof before merge. :: [P2] Clean up an interrupted clone destination
• reviewed 2026-08-30T17:04:22.513Z sha 75f5f4a :: needs real behavior proof before merge. :: [P2] Remove incomplete clone state before returning

[steipete]

Maintainer triage: the reset-error correction is valid and is preserved in #167, with a current-main implementation, regression coverage for both reset paths, documentation, and contributor credit. Thanks @SebTardif.

I recommend closing this older PR after #167 lands. Its clone timer is already covered by the operation-wide deadline in main, and this fork head conflicts with the newer portable-store implementation.

Real CLI proof used a synthetic SQLite archive and real Git on macOS. An upstream file collided with a local untracked file while an immutable .git/index prevented reset. Current main reported the preceding merge failure (would be overwritten by merge, exit status 1); the corrected CLI reported the reset failure (exit status 128). The untracked file and config bytes remained unchanged. The reproducible script and output are in #167. Codex autoreview is clean; candidate CI is at https://github.com/openclaw/gitcrawl/actions/runs/33362738281.

The interrupted-clone concern is also reproducible on current main: a real Git HTTP request stalled until the existing deadline fired after 121.18 seconds, but the partial destination remained. Retrying the identical init command after restoring the endpoint failed because the partial checkout had no origin. That is a separate existing recovery/ownership issue; the diagnostic-only port does not change it. A maintainer decision on operation-owned staging and preservation of existing partial directories is recorded separately.

[steipete]

Thanks, @SebTardif! Your reset-error correction has landed via #167 in 216b2a2, with your co-author credit and changelog thanks preserved. Closing this PR as superseded. I appreciate the contribution!