fix: report portable-store reset recovery failures

[steipete]

What Problem This Solves

Fixes an issue where gitcrawl init --portable-store repeats a dirty-merge error when the subsequent reset actually fails. The reported error now comes from the failed recovery operation, preserving the existing credential-safe Git diagnostics and underlying exit status.

Why This Change Was Made

This is the current-main diagnostic correction from #164, with regression coverage for both reset paths. The original PR conflicts with the portable-store rewrite; its additional clone timer is omitted because current main already bounds the entire operation to two minutes. Thanks @SebTardif; contributor credit is retained in the commit and changelog.

User Impact

Operators get the actual reset failure, including existing disk-space guidance when applicable, instead of being directed back to the preceding merge conflict. Clone, reset, and data-preservation policies are unchanged.

Evidence

Built and ran the real CLI on macOS against a synthetic SQLite archive and real Git, with no Git executable wrapper in the live proof. A new upstream file collides with a local untracked file; an immutable checkout index prevents the subsequent reset.

Paths are normalized in the command excerpt below. Save the reproducer as proof-reset.py.

$ GOWORK=off go build -o /tmp/gitcrawl-after ./cmd/gitcrawl
$ python3 proof-reset.py /tmp/gitcrawl-after
initial CLI init: cloned
tracked worktree status: ''
CLI exit: 1
CLI stderr: portable git failed: Git command failed; verify Git version, repository state and remote access: exit status 128
untracked data preserved: True
config preserved: True

The equivalent current-main binary instead reported Your local changes would be overwritten by merge: exit status 1. The regression test fails on that baseline and checks that both reset paths retain the reset exit code, classified disk-space guidance, and diagnostic redaction.

go vet ./... passes. Codex autoreview of the complete change is clean, with no actionable findings. The exact candidate commit passed the full Linux and macOS CI gates, including go test ./... -covermode=atomic, coverage enforcement, formatting, module tidiness, vet, vulnerability scan, deadcode, build, CLI smoke, release-script tests, and snapshot packaging: https://github.com/openclaw/gitcrawl/actions/runs/33362738281

A separate live smart-HTTP test confirmed that current main cancels a stalled initial clone after 121.18 seconds but leaves a partial checkout that prevents retry. That existing clone-ownership/recovery issue is documented for a separate maintainer decision; this diagnostic-only change does not alter it.

macOS real-Git reproducer

import json, os, pathlib, sqlite3, stat, subprocess, sys, tempfile

binary = pathlib.Path(sys.argv[1]).resolve()
root = pathlib.Path(tempfile.mkdtemp(prefix='reset-', dir=pathlib.Path(__file__).parent))
env = dict(os.environ, GIT_CONFIG_GLOBAL=os.devnull, GIT_CONFIG_SYSTEM=os.devnull, GIT_CONFIG_NOSYSTEM='1', GIT_TERMINAL_PROMPT='0')
for key in ('GITCRAWL_PORTABLE_GIT', 'GITCRAWL_CONFIG', 'GITCRAWL_DB_PATH'):
    env.pop(key, None)
publisher, subscriber = root / 'publisher', root / 'subscriber'
publisher.mkdir()
(publisher / 'data').mkdir()
with sqlite3.connect(publisher / 'data/archive.db') as db:
    db.execute('create table fixture(value text)')
    db.execute("insert into fixture values('synthetic archive')")

def git(where, *args):
    p = subprocess.run(['git', '-C', str(where), *args], env=env, capture_output=True, text=True)
    if p.returncode:
        raise RuntimeError(p.stderr)
    return p.stdout.strip()

def commit():
    git(publisher, 'add', '--all')
    git(publisher, '-c', 'commit.gpgsign=false', 'commit', '-m', 'test: seed synthetic portable archive')

git(publisher, 'init', '-b', 'main')
git(publisher, 'config', 'user.name', 'Peter Steinberger')
git(publisher, 'config', 'user.email', 'steipete@gmail.com')
commit()
args = [str(binary), '--config', str(root/'config.toml'), 'init', '--portable-store', str(publisher), '--store-dir', str(subscriber), '--portable-db', 'data/archive.db', '--json']
first = subprocess.run(args, env=env, capture_output=True, text=True, timeout=90)
assert first.returncode == 0, first.stderr
print('initial CLI init:', json.loads(first.stdout)['portable_store'])
saved_config = (root/'config.toml').read_bytes()
(publisher/'incoming.txt').write_text('incoming remote data\n')
commit()
(subscriber/'incoming.txt').write_text('local untracked data\n')
print('tracked worktree status:', repr(git(subscriber, 'status', '--porcelain', '--untracked-files=no')))
index = subscriber/'.git/index'
os.chflags(index, stat.UF_IMMUTABLE)
try:
    result = subprocess.run(args, env=env, capture_output=True, text=True, timeout=90)
    print('CLI exit:', result.returncode)
    print('CLI stderr:', result.stderr.strip())
    print('untracked data preserved:', (subscriber/'incoming.txt').read_text() == 'local untracked data\n')
    print('config preserved:', (root/'config.toml').read_bytes() == saved_config)
finally:
    os.chflags(index, 0)
print('fixture:', root)

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 31, 2026, 2:12 AM ET / 06:12 UTC.

ClawSweeper review

What this changes

The branch makes portable-store initialization return a failed hard-reset error instead of the earlier dirty-merge error, and adds regression coverage plus operator documentation.

Regression provenance

Possible regression — probable (reviewed change; reproduction). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: this focused correction fixes a current-main error-propagation defect, with a regression test and supplied real CLI evidence showing the recovery failure is now reported.

Priority: P2
Reviewed head: e5c00332cce34e1c87c5e5b3235d4b73bd61b841

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A concise, source-aligned correction with strong real-path evidence and focused regression coverage.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The changed owner is portable-store synchronization used by CLI init; the supplied macOS real-Git run makes its reset fail after a dirty-update scenario and records the patched diagnostic versus the current-main merge-error baseline.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed owner is portable-store synchronization used by CLI init; the supplied macOS real-Git run makes its reset fail after a dirty-update scenario and records the patched diagnostic versus the current-main merge-error baseline.
Evidence reviewed	6 items	Introduced correction: The dirty-merge recovery path now returns the hard-reset failure itself, rather than retaining the earlier fast-forward error. Current-main defect: The PR base returned the previous dirty-merge error after a failed reset, so an operator could be directed to the wrong recovery step. Regression coverage: The new Unix test exercises both dirty-before-pull and dirty-during-merge recovery entrances, verifies the reset exit status and safe disk-space classification, and rejects raw diagnostic leakage.
Findings	None	None.
Security	None	None.

How this fits together

Portable-store initialization prepares a Git-backed SQLite cache for local Gitcrawl use. It validates an existing checkout, attempts a fast-forward update and recovery when necessary, then either makes the validated store available or returns a credential-safe diagnostic.

flowchart LR
  A[Operator runs portable init] --> B[Existing portable checkout]
  B --> C{Can fast-forward?}
  C -->|Dirty merge| D[Hard-reset recovery]
  D --> E[Sanitized recovery diagnostic]
  C -->|Update succeeds| F[Validated portable store]
  E --> G[Operator recovery action]

Loading

Before merge

• Complete next step (P2) - No discrete repair remains; this bounded, evidence-backed change needs ordinary maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Implementation and regression delta	production +0/-0, tests +51, docs/changelog +5	The runtime change is a one-line error-source correction backed by coverage for both recovery branches.

Technical review

Best possible solution:

Preserve this narrow reset-error propagation so portable-store operators receive the actionable, sanitized failure from the recovery step that actually failed.

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

Yes—the supplied macOS real-CLI scenario identifies a current-main baseline error and an after-fix reset failure, and the source shows the exact incorrect return path.

Is this the best way to solve the issue?

Yes—returning the existing sanitized reset error is the narrowest solution and matches the equivalent earlier recovery branch without changing recovery behavior.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

• add P2: This repairs an actionable portable-store diagnostic error with limited blast radius and no change to reset or preservation policy.
• add proof: sufficient: Contributor real behavior proof is sufficient. The changed owner is portable-store synchronization used by CLI init; the supplied macOS real-Git run makes its reset fail after a dirty-update scenario and records the patched diagnostic versus the current-main merge-error baseline.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed owner is portable-store synchronization used by CLI init; the supplied macOS real-Git run makes its reset fail after a dirty-update scenario and records the patched diagnostic versus the current-main merge-error baseline.

Label justifications:

• P2: This repairs an actionable portable-store diagnostic error with limited blast radius and no change to reset or preservation policy.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed owner is portable-store synchronization used by CLI init; the supplied macOS real-Git run makes its reset fail after a dirty-update scenario and records the patched diagnostic versus the current-main merge-error baseline.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed owner is portable-store synchronization used by CLI init; the supplied macOS real-Git run makes its reset fail after a dirty-update scenario and records the patched diagnostic versus the current-main merge-error baseline.

Evidence

What I checked:

• Introduced correction: The dirty-merge recovery path now returns the hard-reset failure itself, rather than retaining the earlier fast-forward error. (internal/cli/app.go:3759, e5c00332cce3)
• Current-main defect: The PR base returned the previous dirty-merge error after a failed reset, so an operator could be directed to the wrong recovery step. (internal/cli/app.go:3759, 7c3e960344df)
• Regression coverage: The new Unix test exercises both dirty-before-pull and dirty-during-merge recovery entrances, verifies the reset exit status and safe disk-space classification, and rejects raw diagnostic leakage. (internal/cli/portable_safety_unix_test.go:139, e5c00332cce3)
• Diagnostic safety remains in force: Portable Git converts raw command diagnostics into bounded credential-safe classifications, including disk-space failures, before this returned error reaches the caller. (internal/cli/portable_git.go:110, e5c00332cce3)
• Real behavior proof supplied: The PR body records a macOS real-Git CLI run where an immutable checkout index makes reset fail; the patched binary reports the reset failure while the described baseline reports the preceding merge error. (e5c00332cce3)
• Feature-history routing: Portable-store work immediately preceding this PR includes the v0.9.3 portable refresh implementation authored by the same current-main area contributor. (internal/cli/app.go:3716, 47232a77749f)

Likely related people:

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

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.