Skip to content

feat(portable): consume backed-up sources during export - #207

Merged
obviyus merged 1 commit into
mainfrom
fix/portable-consume-source
Sep 17, 2026
Merged

obviyus merged 1 commit into
mainfrom
fix/portable-consume-source

Conversation

@obviyus

@obviyus obviyus commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Portable export currently copies the entire source database before removing runtime-only data. For batch jobs with a verified backup, that copy can exhaust the runner disk.

Add explicit --consume-source support. It locks and moves an exclusively owned source into private staging on the same filesystem, then uses the existing shaping, validation, compression, and atomic publication path. Ordinary exports preserve their source. Consumption requires rollback journal mode, no sidecars, and a regular file with one link; errors after transfer discard the source, so recovery must already exist.

Validation: portable and CLI suites pass, including artifact-identity equivalence, original-file identity through handoff, active reader/writer rejection, aliases, sidecars, cancellation, and recovery after failures during shaping, compaction, and manifest creation. Cross-filesystem refusal is covered on Linux. Vet passes. Hosted checks cover Linux and macOS. No database schema change.

Real CLI proof on a 128 MiB filesystem: an 84,508,672-byte source with 46,538,752 bytes free fails ordinary export with database or disk is full; consuming export succeeds, producing a validated 233,472-byte database and 9,014-byte gzip archive. An induced post-handoff archive-budget failure preserves the external checkpoint; restoring it and exporting again produces the same artifact identity. An ordinary export with sufficient disk preserves the source and matches that identity. A reader query succeeds, and the CLI rejects a hard-linked source without changing it.

The working-space requirement is reduced, not eliminated: a separate fixture containing one 80 MiB row exhausted the constrained filesystem while rewriting that row. Its checkpoint remained available for recovery. Oversized individual records and the final compact database still require free space.

@clawsweeper

clawsweeper Bot commented Sep 17, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@obviyus
obviyus marked this pull request as ready for review September 17, 2026 13:15
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 17, 2026
@clawsweeper

clawsweeper Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 17, 2026, 9:38 AM ET / 13:38 UTC (Revision 2).

ClawSweeper review

What this changes

Adds an opt-in Unix export flag that moves a backed-up SQLite database into staging to reduce disk requirements, with safety checks, documentation, and tests.

Merge readiness

Blocked before merge - 5 items remain

This PR retains distinct value: neither main nor v0.10.0 supports source consumption. No concrete code defect was found; the updated runtime report is useful, but the earlier request for inspectable recovery proof remains incomplete.

Priority: P2
Reviewed head: 94e91422991d24c149b2f3d1f8e5dfd423c672cb
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused implementation and safety coverage are solid, while the detailed runtime account still lacks inspectable output.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: The updated body reports the real portable-export CLI succeeding under disk pressure and recovering from a post-handoff failure, directly matching the changed consumption path. It improves on test-only evidence, but the requested terminal transcript, logs, or recording showing validation, checkpoint restoration, and the ordinary-export control is still absent. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The updated body reports the real portable-export CLI succeeding under disk pressure and recovering from a post-handoff failure, directly matching the changed consumption path. It improves on test-only evidence, but the requested terminal transcript, logs, or recording showing validation, checkpoint restoration, and the ordinary-export control is still absent. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 7 items Repository and applicable policy: The origin identifies openclaw/gitcrawl. No root or changed-path ancestor AGENTS.md exists; the only tracked AGENTS.md is scoped to the unrelated autoreview skill. No maintainer-notes directory is present.
Distinct from current main: Main unconditionally invokes snapshotSQLite before opening and shaping the disposable database. Its export options and CLI do not provide source consumption.
Latest release still copies the source: The latest release endpoint identifies v0.10.0, published September 14, 2026. Its export implementation also unconditionally snapshots the source; this feature is not already shipped.
Findings None None.
Security None None.

How this fits together

Gitcrawl exports its local GitHub archive into compact, validated database artifacts for distribution. This change affects how the source enters staging; shaping, compression, validation, and publication use the existing export pipeline.

flowchart TD
 A[Configured SQLite archive] --> B{Consume source enabled?}
 B -->|No| C[Copy consistent snapshot]
 B -->|Yes| D[Check file and lock database]
 D --> E[Move source into private staging]
 C --> F[Shape and compact archive]
 E --> F
 F --> G[Validate and publish artifact]
 F --> H[On failure discard staging]
Loading

Decision needed

Question Recommendation
Should portable export offer a mode that discards the source after handoff failures and relies on an operator-verified external backup? Accept the constrained opt-in mode: Support consumption for closed, backed-up batch databases once the reported success and recovery runs have inspectable evidence.

Why: This is an intentional destructive capability; implementation checks cannot decide whether that operational contract belongs in the supported CLI.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The updated body reports the real portable-export CLI succeeding under disk pressure and recovering from a post-handoff failure, directly matching the changed consumption path. It improves on test-only evidence, but the requested terminal transcript, logs, or recording showing validation, checkpoint restoration, and the ordinary-export control is still absent. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - After opting in, any post-handoff failure can leave neither the source path nor a published artifact; recovery depends on an external backup that Gitcrawl does not verify.
  • Resolve merge risk (P1) - The reported large-row failure shows that consumption still needs workload-dependent working space; reduced disk use does not guarantee completion.
  • Complete next step (P2) - Attach the output behind the reported CLI runs; terminal screenshots or a recording are welcome, and copied output or logs count. Redact private paths, endpoints, IP addresses, credentials, and other private data. Updating the PR body should trigger review automatically; otherwise ask a maintainer to comment @clawsweeper re-review.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth Production +100/-3; tests +251/-0; docs +16/-1 Production growth is justified by the new platform-specific handoff and CLI option, with most added lines devoted to safety and recovery coverage.

Merge-risk options

Maintainer options:

  1. Accept documented source loss after recovery proof (recommended)
    Approve the opt-in loss contract after inspecting the reported checkpoint restoration and source-preserving control runs.
  2. Pause the destructive mode
    Leave the feature unmerged if reliance on an externally verified backup is outside the supported export contract.

Technical review

Best possible solution:

Preserve copying as the default and admit source consumption only for disposable batch databases under an explicitly accepted, demonstrably recoverable loss contract.

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

Not applicable as an existing-behavior bug: this adds a disk-saving mode. The body reports a concrete disk-pressure comparison, but this review did not execute it.

Is this the best way to solve the issue?

Yes, technically: replacing only initial snapshot acquisition reuses the existing artifact pipeline and preserves defaults; acceptance of the destructive operating contract remains a product choice.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 0bb13412e4c8.

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This is a bounded improvement for disk-constrained batch exports, with existing export behavior preserved by default.
  • merge-risk: 🚨 other: The new opt-in mode intentionally makes source recovery depend on an external backup after export failures.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The updated body reports the real portable-export CLI succeeding under disk pressure and recovering from a post-handoff failure, directly matching the changed consumption path. It improves on test-only evidence, but the requested terminal transcript, logs, or recording showing validation, checkpoint restoration, and the ordinary-export control is still absent. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Repository and applicable policy: The origin identifies openclaw/gitcrawl. No root or changed-path ancestor AGENTS.md exists; the only tracked AGENTS.md is scoped to the unrelated autoreview skill. No maintainer-notes directory is present. (94e91422991d)
  • Distinct from current main: Main unconditionally invokes snapshotSQLite before opening and shaping the disposable database. Its export options and CLI do not provide source consumption. (internal/portable/export.go:195, 0bb13412e4c8)
  • Latest release still copies the source: The latest release endpoint identifies v0.10.0, published September 14, 2026. Its export implementation also unconditionally snapshots the source; this feature is not already shipped. (internal/portable/export.go:195, 9898b678a4d8)
  • Destructive handoff boundary: Consumption checks file type, link count, filesystem, sidecars, journal mode, and an exclusive SQLite transaction before renaming. Export then uses its existing discard-on-failure staging cleanup. Backup existence and recoverability remain operator preconditions, explicitly documented. (internal/portable/consume_unix.go:71, 94e91422991d)
  • Updated proof and review continuity: The complete supplied body, sourceRevision cf332c842f4bc75e1d2a83daf686302f53d54497d759a58f9eab4fae15bc45c9, reports an 84,508,672-byte source on a 128 MiB filesystem, ordinary-export disk exhaustion, successful consuming export, checkpoint recovery, and ordinary-export equivalence. The live body matches that text. These are specific reported observations, but no commands, terminal transcript, logs, or linked artifacts accompany them. The prior review at feat(portable): consume backed-up sources during export #207 (comment) requested a transcript or recording covering those scenarios. (94e91422991d)
  • Focused supplemental coverage: New tests cover artifact equivalence, inode-preserving handoff, unsafe-source rejection, cancellation at four stages, external-backup preservation, recovery, and cross-filesystem refusal. Existing tests cover ordinary export preserving database and WAL bytes. Tests were inspected, not executed, under the read-only review constraint. (internal/portable/consume_unix_test.go:34, 94e91422991d)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • obviyus: 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.

  • Attach the redacted terminal transcript or recording underlying the reported constrained-disk success, post-handoff recovery, and ordinary-export control.

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 (1 earlier review cycle)
  • reviewed 2026-09-17T13:19:11.270Z sha 94e9142 :: needs real behavior proof before merge. :: none

@obviyus
obviyus merged commit d99af2e into main Sep 17, 2026
19 checks passed
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant