Skip to content

build(deps): refresh dependency closure - #136

Merged
steipete merged 2 commits into
mainfrom
codex/dependency-refresh-2026-08-01
Aug 2, 2026
Merged

build(deps): refresh dependency closure#136
steipete merged 2 commits into
mainfrom
codex/dependency-refresh-2026-08-01

Conversation

@steipete

@steipete steipete commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve Gitcrawl's documented filesystem-only database-path contract across the SQLite 1.55 upgrade
  • refresh CrawlKit, SQLite and its modernc closure, current test/profiling helpers, and TruffleHog
  • document and regression-test URI rejection for top-level config, config-backed environment values, and process environment overrides

This supersedes #132, #133, and #135. It intentionally does not include #134: actions/stale@v11 is a write-capable major automation update and still needs controlled issue/PR behavior proof before landing.

Proof

  • GOWORK=off go test ./... -count=1
  • make build
  • make test-release
  • make lint (go vet, govulncheck, deadcode)
  • make tidy-check with the intended manifests staged
  • actionlint
  • source-blind built-CLI validation: ordinary filesystem paths returned status JSON and exit 0; process env, top-level config, and config [env] SQLite URI variants returned exit 1 with the filesystem-path contract error and created no database/sidecar files
  • TruffleHog 3.96.0 live filesystem scans across source, docs, workflows, changelog, and manifests: zero verified secrets
  • docs site build, local serve, and HTTP fetch of /configuration/
  • two clean structured autoreviews, one for the complete stack and one for the dependency-only commit

@steipete
steipete requested a review from a team as a code owner August 2, 2026 02:52
@clawsweeper clawsweeper Bot added 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. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 2, 2026
@clawsweeper

clawsweeper Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 1, 2026, 10:56 PM ET / August 2, 2026, 02:56 UTC.

ClawSweeper review

What this changes

The branch updates CrawlKit, SQLite and related Go dependencies, bumps the TruffleHog workflow action, and rejects SQLite URI or in-memory values from configured and environment-supplied database paths.

Merge readiness

Blocked until real behavior proof is added - 6 items remain

Keep this PR open: the SQLite upgrade changes an accepted runtime configuration into a startup error for existing URI-based setups, so maintainers need to confirm the upgrade policy and the contributor needs attachable real CLI proof. The repository’s deep pre-release history was only partly available because missing promisor objects could not be fetched during this read-only review, but the current source, exact PR diff, tests, and release state were inspected.

Priority: P1
Reviewed head: bcefa5c79e951acf8bbf5b1ebe2afe615e4edf89
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and has targeted unit coverage, but real runtime proof is absent and the upgrade compatibility decision remains unresolved.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR body reports a built-CLI validation, but no redacted terminal output, recording, log, or linked artifact is available to inspect; add proof showing accepted filesystem paths and rejected URI paths without database or sidecar creation, redacting private paths and tokens. Updating the PR body should trigger a fresh review; otherwise ask a maintainer to comment @clawsweeper re-review. 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 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body reports a built-CLI validation, but no redacted terminal output, recording, log, or linked artifact is available to inspect; add proof showing accepted filesystem paths and rejected URI paths without database or sidecar creation, redacting private paths and tokens. Updating the PR body should trigger a fresh review; otherwise ask a maintainer to comment @clawsweeper re-review. 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 6 items Existing documented contract: Current documentation already says init --db accepts durable filesystem paths rather than SQLite file: URIs or :memory:, and the current CLI test rejects those flag values.
Runtime compatibility change: The PR makes configured and environment-derived URI values fail from Normalize and ApplyRuntimeEnv; current users who had been using these previously accepted values will now receive an error before the archive opens.
Configuration coverage: The branch adds regression coverage for direct config values, process environment values, and config-backed environment values, and propagates runtime-environment errors through the no-config status, doctor, and TUI paths.
Findings 2 actionable findings [P1] Provide an approved upgrade path for URI database settings
[P3] Remove release-owned changelog entries
Security None None.

How this fits together

Gitcrawl resolves its local archive database location from saved configuration and runtime environment variables before commands such as status, doctor, and the TUI access SQLite. The patch enforces the documented filesystem-path boundary at that configuration layer while upgrading the SQLite driver that now gives URI query parameters additional meaning.

flowchart LR
  A[CLI flags and config file] --> B[Runtime environment overrides]
  B --> C[Database path normalization]
  C --> D[Filesystem-path decision]
  D --> E[SQLite archive]
  E --> F[Status, doctor, and TUI commands]
Loading

Decision needed

Question Recommendation
Should existing SQLite URI database configurations be rejected immediately as an unsupported and unsafe upgrade path, or retained temporarily behind a compatibility path? Ship fail-closed with migration proof: Keep URI rejection, add actionable migration guidance and a demonstrated upgrade path that preserves the intended filesystem archive.

Why: Current runtime behavior accepts these values while the proposed driver upgrade makes URI query parameters materially more consequential; choosing an immediate failure versus a compatibility period is an upgrade-policy decision, not a mechanical code repair.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body reports a built-CLI validation, but no redacted terminal output, recording, log, or linked artifact is available to inspect; add proof showing accepted filesystem paths and rejected URI paths without database or sidecar creation, redacting private paths and tokens. Updating the PR body should trigger a fresh review; otherwise ask a maintainer to comment @clawsweeper re-review. 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.
  • Provide an approved upgrade path for URI database settings (P1) - LoadRuntime now turns previously accepted file: and :memory: configuration values into startup errors. The existing docs and init flag contract support rejecting them, but current installed configurations can still contain these values; get the explicit policy decision, add actionable migration guidance, and prove the chosen upgrade behavior before merging.
  • Remove release-owned changelog entries (P3) - Keep release notes out of a normal dependency/configuration PR; the release process owns CHANGELOG.md, while the PR body can carry the user-visible context needed for review.
  • Resolve merge risk (P1) - Existing installations using a SQLite URI in db_path or GITCRAWL_DB_PATH currently run but will fail at startup after merge; the new error has no explicit migration guidance or demonstrated upgrade path.
  • Complete next step (P2) - A maintainer must decide the upgrade contract for existing URI-based configurations, and the contributor must supply real behavior proof; neither is a safe autonomous repair task.

Findings

  • [P1] Provide an approved upgrade path for URI database settings — internal/config/config.go:212-214
  • [P3] Remove release-owned changelog entries — CHANGELOG.md:8-9
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 9 files affected; 112 added, 32 removed The branch combines a runtime configuration behavior change with dependency, workflow, documentation, and test updates.
Dependency execution surface 2 direct Go modules and 1 GitHub Action updated SQLite and the secret-scanning action run in critical local archive and CI paths, so upgrade behavior needs proof beyond manifest changes.

Merge-risk options

Maintainer options:

  1. Confirm fail-closed migration before merge (recommended)
    Decide that URI paths remain unsupported, then add an actionable migration message and redacted CLI proof for a legacy URI configuration failing without creating archive sidecar files.
  2. Preserve legacy behavior temporarily
    Avoid breaking existing URI-based configurations until a separately approved compatibility or deprecation path is implemented and tested.
  3. Pause the SQLite upgrade
    Defer the dependency closure if maintainers do not want either an immediate configuration break or a new compatibility surface.

Technical review

Best possible solution:

Keep the filesystem-only policy if maintainers intend to protect archives from SQLite 1.55 URI semantics, but provide an explicit migration message and proof for an existing URI-based installation before merging; leave release notes to the release process.

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

Yes, at source level: on the PR head, set db_path or GITCRAWL_DB_PATH to file:archive.db or :memory: and configuration loading returns the new filesystem-path error. The branch adds unit coverage for config and environment sources, but this read-only review did not execute the CLI.

Is this the best way to solve the issue?

Unclear. Rejecting URI paths aligns with the existing documented and init --db contract, but the branch still needs an explicit maintainer decision and migration treatment for installations that currently rely on the accepted runtime behavior.

Full review comments:

  • [P1] Provide an approved upgrade path for URI database settings — internal/config/config.go:212-214
    LoadRuntime now turns previously accepted file: and :memory: configuration values into startup errors. The existing docs and init flag contract support rejecting them, but current installed configurations can still contain these values; get the explicit policy decision, add actionable migration guidance, and prove the chosen upgrade behavior before merging.
    Confidence: 0.92
  • [P3] Remove release-owned changelog entries — CHANGELOG.md:8-9
    Keep release notes out of a normal dependency/configuration PR; the release process owns CHANGELOG.md, while the PR body can carry the user-visible context needed for review.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

  • add P1: Merging can make existing database-path configurations fail before normal CLI workflows start.
  • add merge-risk: 🚨 compatibility: The patch intentionally changes currently accepted SQLite URI settings into errors during configuration loading.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports a built-CLI validation, but no redacted terminal output, recording, log, or linked artifact is available to inspect; add proof showing accepted filesystem paths and rejected URI paths without database or sidecar creation, redacting private paths and tokens. Updating the PR body should trigger a fresh review; otherwise ask a maintainer to comment @clawsweeper re-review. 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.

Label justifications:

  • P1: Merging can make existing database-path configurations fail before normal CLI workflows start.
  • merge-risk: 🚨 compatibility: The patch intentionally changes currently accepted SQLite URI settings into errors during configuration loading.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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 PR body reports a built-CLI validation, but no redacted terminal output, recording, log, or linked artifact is available to inspect; add proof showing accepted filesystem paths and rejected URI paths without database or sidecar creation, redacting private paths and tokens. Updating the PR body should trigger a fresh review; otherwise ask a maintainer to comment @clawsweeper re-review. 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:

  • Existing documented contract: Current documentation already says init --db accepts durable filesystem paths rather than SQLite file: URIs or :memory:, and the current CLI test rejects those flag values. (docs/configuration.md:60, dcdf94703540)
  • Runtime compatibility change: The PR makes configured and environment-derived URI values fail from Normalize and ApplyRuntimeEnv; current users who had been using these previously accepted values will now receive an error before the archive opens. (internal/config/config.go:212, bcefa5c79e95)
  • Configuration coverage: The branch adds regression coverage for direct config values, process environment values, and config-backed environment values, and propagates runtime-environment errors through the no-config status, doctor, and TUI paths. (internal/config/config_test.go:236, bcefa5c79e95)
  • Dependency and workflow scope: The dependency closure moves CrawlKit to v0.14.4, modernc SQLite to v1.55.0 with matching modernc runtime updates, and the secret-scan workflow moves TruffleHog from v3.95.9 to v3.96.0 without changing workflow permissions or inputs. (go.mod:12, bcefa5c79e95)
  • Current release and branch state: The current default branch is tagged v0.8.7, while the PR head is not contained by any local tag or branch; this work is neither merged nor released. (CHANGELOG.md:8, bcefa5c79e95)
  • History availability limitation: Direct current-file history and PR commit metadata were available, but deeper blame/history traversal required a missing promisor object and failed because the remote host could not be resolved. (internal/cli/app.go:1964, dcdf94703540)

Likely related people:

  • steipete: The current shipped configuration file is attributed to Peter Steinberger’s v0.8.7 release commit, and this branch’s configuration and dependency commits are authored by the same contributor. (role: recent configuration and release contributor; confidence: high; commits: 4f737bb846e0, 10e4d842db15, bcefa5c79e95; files: internal/config/config.go, docs/configuration.md, go.mod)
  • Josh Lehman: Recent local history for the command entrypoint includes work by Josh Lehman; the branch changes fallback paths in status, doctor, and TUI command handling. (role: recent adjacent CLI contributor; confidence: medium; commits: 8bb0a217fdfd, 2b532382962e; files: internal/cli/app.go)

Rank-up moves

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

  • Obtain the maintainer decision on immediate URI rejection versus temporary compatibility.
  • Attach a redacted terminal transcript or screenshot demonstrating the selected upgrade behavior.
  • Remove the release-owned CHANGELOG.md edits.

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.

@steipete
steipete merged commit 21852d3 into main Aug 2, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant