Collect analytics evidence with durable review-state recovery

[hannesrudolph]

GitHub conversation history did not retain complete current review membership, and rejected collection attempts were absent from success-only health views. This adds gitcrawl analytics for publication repair, native actor evidence, ongoing collection, and durable review-state recovery. Core conversation coverage and review enrichment remain separate; targeted recovery preserves canonical conversations, revisions, and vectors.

Collection rejects incomplete discovery pages and requires historical receipts bound to the selected repository, including valid empty histories. Actor enrichment isolates forbidden nodes, publication repair preserves unknown events, and both exports and command failures exclude private diagnostics. Operational analytics data stays out of exported snapshots. Thanks @hannesrudolph.

Validation:

• make check: 85.7% coverage with the unchanged 85% floor; formatting, module tidiness, vet, vulnerability/dead-code checks, CLI smoke, release-script tests, docs, and six-platform snapshot builds passed.
• Focused race tests and SIMD/scalar-fallback suites passed.
• Synthetic regressions failed before the fixes and passed afterward.
• A bounded live GitHub read into a temporary archive collected one PR and three comments, enriched three actor profiles, and preserved canonical history during audit/repair/enrichment.
• Independent Codex review: no actionable P0–P2 findings.

[clawsweeper]

🦞👀
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.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 29, 2026, 11:02 PM ET / September 30, 2026, 03:02 UTC (Revision 7).

ClawSweeper review

What this changes

The branch adds an analytics command that collects GitHub actor and review evidence, records collection failures in SQLite, and recovers missing review state.

Merge readiness

⛔ Blocked before merge - 13 items remain

This PR remains useful and is absent from current main and v0.13.0. The latest head still has a stale-credential dispatch path, an unverified historical-coverage bootstrap, and a cross-writer retry race. The maintainer’s review also leaves credential-wide admission and upgrade safety unresolved.

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

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The branch has substantial focused tests and a reported live collection, but material authority, retry-ordering, and upgrade gaps prevent merge confidence.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: Authority-chain proof required: The PR body reports one real GitHub collection into a temporary archive, but provides no final request trace showing that helper rotation or failure prevents stale-token I/O, nor a forbidden-origin receipt rejection before coverage is written. The changed SQLite schema also lacks verified upgrade evidence from a populated released schema-13 archive. Redacted terminal or log traces would cover these paths; remove tokens, IP addresses, phone numbers, and private endpoints before posting. Updating the PR body should trigger a fresh review; if it does not, a maintainer can 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)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Authority-chain proof required: The PR body reports one real GitHub collection into a temporary archive, but provides no final request trace showing that helper rotation or failure prevents stale-token I/O, nor a forbidden-origin receipt rejection before coverage is written. The changed SQLite schema also lacks verified upgrade evidence from a populated released schema-13 archive. Redacted terminal or log traces would cover these paths; remove tokens, IP addresses, phone numbers, and private endpoints before posting. Updating the PR body should trigger a fresh review; if it does not, a maintainer can 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	11 items	Current main boundary: Current main has the earlier GraphQL history sync and schema 13; the analytics command and schema 15 are branch additions. Release exclusion: The maintainer explicitly states that this PR was not included in v0.13.0 and identifies three remaining design blockers. Stale helper credential: The analytics token provider returns its cached value for up to 45 minutes; the GitHub client consumes that value immediately before the final HTTP request.
Findings	4 actionable findings	[P1] Refresh the helper credential at every analytics dispatch [P1] Bind historical receipts to the GitHub origin and repository [P1] Fence retry outcomes by acquisition attempt
Security	Needs attention	Revoked helper credential can reach GitHub: The analytics cache can bypass the helper's current authorization decision for up to 45 minutes before final HTTP dispatch. Unverified receipt can authorize skipped collection: A local completion receipt is accepted without a checked origin or producer contract, allowing incorrect repository coverage to become durable.

How this fits together

Gitcrawl collects GitHub conversations into a local archive. The new analytics path reads GitHub and local discovery receipts, then writes coverage, retry, actor, and review-state evidence alongside the archive.

flowchart LR
  A[GitHub conversations] --> C[Analytics collector]
  B[Historical discovery receipt] --> C
  C --> D{Evidence complete?}
  D -->|Yes| E[Coverage and actor evidence]
  D -->|No| F[Durable retry queue]
  F --> G[Review-state recovery]
  G --> E

Loading

Decision needed

Question	Recommendation
Which shared receipt-provenance and cross-writer retry-ownership contracts must be in place before analytics recovery can ship?	Define shared contracts before merge: Agree an origin-bound receipt format and conditional attempt ownership across writers, then validate them with integration and upgrade evidence.

Why: The external backfill producer and ordinary sync writer participate in these contracts, and the maintainer explicitly identified them as design decisions.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: The PR body reports one real GitHub collection into a temporary archive, but provides no final request trace showing that helper rotation or failure prevents stale-token I/O, nor a forbidden-origin receipt rejection before coverage is written. The changed SQLite schema also lacks verified upgrade evidence from a populated released schema-13 archive. Redacted terminal or log traces would cover these paths; remove tokens, IP addresses, phone numbers, and private endpoints before posting. Updating the PR body should trigger a fresh review; if it does not, a maintainer can 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.
• Refresh the helper credential at every analytics dispatch (P1) - The 45-minute cache returns a previously authorized token after the helper rotates or fails. That value reaches the final GitHub Authorization header, so a revoked credential can still be dispatched and helper failure is hidden.
• Bind historical receipts to the GitHub origin and repository (P1) - The bootstrap treats a local receipt's repository string as authority for complete historical coverage. The maintainer reproduced a mismatched receipt that left zero archived threads while reporting complete coverage; require a versioned provenance contract or decline the bootstrap.
• Fence retry outcomes by acquisition attempt (P1) - Success resolves and failure reopens the same retry row without checking attempt order. Because ordinary sync does not share the analytics lock, a late older result can erase a newer failure or reopen repaired work, as the maintainer reproduced.
• Verify upgrade from a populated released archive (P2) - Writable open raises the archive schema from the released version 13 to 15. The real v0.12.0 schema-13 fixture described by the maintainer is on a separate branch; this head does not demonstrate retained IDs, history, review state, or interrupted-upgrade recovery.
• Resolve security concern: Revoked helper credential can reach GitHub - The analytics cache can bypass the helper's current authorization decision for up to 45 minutes before final HTTP dispatch.
• Resolve security concern: Unverified receipt can authorize skipped collection - A local completion receipt is accepted without a checked origin or producer contract, allowing incorrect repository coverage to become durable.
• Resolve merge risk (P1) - A historical receipt can certify complete coverage without verified GitHub origin and repository provenance, so an archive may silently skip conversations.
• Resolve merge risk (P1) - Ordinary GraphQL sync and analytics recovery can finish out of order; retry obligations need a shared lease or conditional attempt generations.
• Resolve merge risk (P1) - Concurrent clients lack credential-wide GraphQL admission and secondary-limit cooldown, so a collector can trigger credential-wide throttling despite individual quota checks.
• Resolve merge risk (P1) - A real populated schema-13 archive upgrade is not proven on this head; writable upgrade also prevents v0.12.0 from reading the archive without a backup.
• Complete next step (P2) - Resolve the credential, receipt, and retry findings; agree credential-wide admission; then provide final-effect authority proof and a populated schema-13 upgrade demonstration before merge.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [P1] Refresh the helper credential at every analytics dispatch — internal/cli/analytics.go:39-48
• [P1] Bind historical receipts to the GitHub origin and repository — internal/cli/analytics.go:165-188
• [P1] Fence retry outcomes by acquisition attempt — internal/store/analytics_integrity.go:80-98
• [high] Revoked helper credential can reach GitHub — internal/cli/analytics.go:39
• [medium] Unverified receipt can authorize skipped collection — internal/cli/analytics.go:165

Agent review details

Security

Needs attention: The collector can dispatch a cached helper credential after authorization changes, and historical coverage trusts a receipt without origin provenance.

Review metrics

Metric	Value	Why it matters
Branch scope against main	51 files changed	The final merge head contains a substantial analytics and storage change beyond the earlier merged GraphQL feature.
Code and test growth	production +3,047/-100; tests +3,441/-6 lines	The new command and durable recovery path explain the production growth, while the breadth increases the importance of integration and upgrade proof.

Merge-risk options

Maintainer options:

1. Resolve the shared contracts (recommended)
   Repair receipt authority, retry ordering, and credential-wide admission before landing the new collector.
2. Pause the rollout
   Keep the branch open until the external receipt producer and ordinary sync writer can participate in the chosen contracts.

Technical review

Best possible solution:

Use an origin-bound, versioned discovery receipt; fence retry completion across all GraphQL writers; share admission per credential; and prove schema-13 upgrade and downgrade guidance with a real archive fixture.

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

Yes for the principal blockers: source shows the cached-token and unordered retry paths, and the maintainer reports concrete receipt and cross-writer reproductions. This read-only review did not execute the collector.

Is this the best way to solve the issue?

No. Durable analytics recovery addresses a real gap, but its receipt authority, attempt ownership, and credential admission need shared contracts before this implementation is safe to ship.

Full review comments:

• [P1] Refresh the helper credential at every analytics dispatch — internal/cli/analytics.go:39-48
  The 45-minute cache returns a previously authorized token after the helper rotates or fails. That value reaches the final GitHub Authorization header, so a revoked credential can still be dispatched and helper failure is hidden.
  Confidence: 0.98
• [P1] Bind historical receipts to the GitHub origin and repository — internal/cli/analytics.go:165-188
  The bootstrap treats a local receipt's repository string as authority for complete historical coverage. The maintainer reproduced a mismatched receipt that left zero archived threads while reporting complete coverage; require a versioned provenance contract or decline the bootstrap.
  Confidence: 0.94
• [P1] Fence retry outcomes by acquisition attempt — internal/store/analytics_integrity.go:80-98
  Success resolves and failure reopens the same retry row without checking attempt order. Because ordinary sync does not share the analytics lock, a late older result can erase a newer failure or reopen repaired work, as the maintainer reproduced.
  Confidence: 0.94
• [P2] Verify upgrade from a populated released archive — internal/store/store.go:19
  Writable open raises the archive schema from the released version 13 to 15. The real v0.12.0 schema-13 fixture described by the maintainer is on a separate branch; this head does not demonstrate retained IDs, history, review state, or interrupted-upgrade recovery.
  Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

No label changes.

Label justifications:

• P2: This is substantial unreleased functionality with bounded but material correctness and upgrade blockers.
• merge-risk: 🚨 security-boundary: Cached credentials can outlive helper authorization, and an unverified receipt can authorize skipped collection.
• merge-risk: 🚨 compatibility: Writable opens advance existing archives to schema 15, which the prior released client cannot read.
• merge-risk: 🚨 auth-provider: The analytics credential helper can rotate or fail while a cached token remains eligible for final dispatch.
• 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 stronger real behavior proof before merge: Authority-chain proof required: The PR body reports one real GitHub collection into a temporary archive, but provides no final request trace showing that helper rotation or failure prevents stale-token I/O, nor a forbidden-origin receipt rejection before coverage is written. The changed SQLite schema also lacks verified upgrade evidence from a populated released schema-13 archive. Redacted terminal or log traces would cover these paths; remove tokens, IP addresses, phone numbers, and private endpoints before posting. Updating the PR body should trigger a fresh review; if it does not, a maintainer can 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

Security concerns:

• [high] Revoked helper credential can reach GitHub — internal/cli/analytics.go:39
  The analytics cache can bypass the helper's current authorization decision for up to 45 minutes before final HTTP dispatch.
  Confidence: 0.98
• [medium] Unverified receipt can authorize skipped collection — internal/cli/analytics.go:165
  A local completion receipt is accepted without a checked origin or producer contract, allowing incorrect repository coverage to become durable.
  Confidence: 0.92

What I checked:

• Current main boundary: Current main has the earlier GraphQL history sync and schema 13; the analytics command and schema 15 are branch additions. (internal/store/store.go:19, d8d19effd37d)
• Release exclusion: The maintainer explicitly states that this PR was not included in v0.13.0 and identifies three remaining design blockers.
• Stale helper credential: The analytics token provider returns its cached value for up to 45 minutes; the GitHub client consumes that value immediately before the final HTTP request. (internal/cli/analytics.go:39, f62022fe6ff2)
• Final dispatch: The selected provider token becomes the Authorization header on the outgoing GitHub request. (internal/github/client.go:622, f62022fe6ff2)
• Historical receipt bootstrap: The bootstrap accepts repository text, phase, totals, and timestamps from status.json without a checked GitHub origin or producer identity. The maintainer reports a mismatched receipt yielding complete coverage with no archived threads. (internal/cli/analytics.go:153, f62022fe6ff2)
• Retry outcome ordering: Success resolves and failure reopens a retry row by repository, item, and operation, without checking which acquisition attempt owns the row. The maintainer reports both late-success and late-failure inversions across writers. (internal/store/analytics_integrity.go:80, f62022fe6ff2)

Likely related people:

• Hannes Rudolph: Raw commit bd91431 adds internal/github/history.go:23 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: bd9143104d03; files: internal/github/history.go)
• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: 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.

• Show final GitHub I/O for the allowed helper account and prove rotation or helper failure stops the old token before dispatch.
• Prove a foreign-origin receipt cannot mark coverage complete, and fence retry completion across ordinary sync and analytics writers.
• Demonstrate a populated v0.12.0 schema-13 archive upgrading to schema 15 with retained evidence and safe interrupted recovery.

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 (6 earlier review cycles)

• reviewed 2026-09-26T04:36:44.421Z sha 50a7055 :: blocked before merge. :: [P2] Sanitize watch errors before writing them to stderr
• reviewed 2026-09-26T04:50:17.259Z sha 50a7055 :: blocked before merge. :: [P2] Sanitize watch errors before writing them to stderr | [P2] Prove upgrades from the shipped schema 13 archive
• reviewed 2026-09-26T05:02:33.988Z sha 7e4d33e :: needs real behavior proof before merge. :: [P1] Refresh managed credentials before each analytics dispatch | [P2] Sanitize watch errors before writing them to stderr | [P2] Prove upgrades from the shipped schema-13 archive
• reviewed 2026-09-26T08:51:18.837Z sha abc4af8 :: needs real behavior proof before merge. :: [P1] Refresh managed credentials before each analytics dispatch | [P2] Sanitize watch errors before writing them to stderr | [P2] Prove upgrades from the shipped schema-13 archive
• reviewed 2026-09-26T08:57:22.200Z sha abc4af8 :: needs real behavior proof before merge. :: [P1] Refresh managed credentials before each analytics dispatch | [P2] Sanitize watch errors before writing them to stderr | [P2] Prove upgrades from the shipped schema-13 archive
• reviewed 2026-09-26T12:17:13.815Z sha a6f5799 :: needs real behavior proof before merge. :: [P1] Recheck the token helper before every analytics request | [P2] Use safe failure details in the direct watch log | [P2] Verify migration from a populated schema-13 archive

[steipete]

Thanks for this, @hannesrudolph. Review-state recovery fills a real gap left by #213, and keeping review-only writes separate from canonical content, revisions and vectors is the right boundary.

I reviewed it in depth before the 0.13.0 release. It's not in 0.13.0. There are three blockers that need design decisions with you (below). The bounded fixes I could make are on maintainer/pr-216-review-fixes: your branch plus a normal merge of main and twelve focused commits, each with a regression test. Your branch is untouched; pull or cherry-pick whatever you want.

Fixed on that branch:

• fix(auth): refresh analytics credentials at every dispatch. The 45-minute token cache could dispatch a revoked or rotated helper credential and hide helper failures.
• fix(analytics): redact provider failures in command output. Direct watch, one-shot errors and joined incomplete-coverage errors could render rejected provider bodies.
• fix(export): exclude private analytics source state. Whole-database portable and cloud copies kept private analytics receipts, queues, cursors and native actor datasets.
• fix(github): reserve full GraphQL request estimates. The observed-quota guard debited one point rather than the query estimate, so concurrent requests could cross the reserve.
• fix(store): recheck review membership when queueing recovery, and keep review coverage current with retry state.
• fix(analytics): stop one-shot enrichment after failed batches; accept complete empty discovery baselines (zero-issue/zero-PR repos) and reject duplicate or missing lanes.
• Tests: a real v0.12.0-generated schema-13 fixture for the 13 → 15 upgrade (interrupted and idempotent), plus two runtime-independent timing fixes.

Blockers (need a decision):

1. Bootstrap authority. A completed status.json receipt carries no checked repository or origin identity. Supplying a receipt for different/repository and then collecting fixture/repo finished with zero archived threads and coverage_complete=true, silently skipping history. This needs a versioned receipt contract (GitHub origin plus canonical repository) shared with your external backfill producer, and rejection on mismatch.
2. Retry ownership across writers. runner.lock excludes a second analytics collector, but ordinary GraphQL sync doesn't take part. Reproduced: a late older success clears a newer failure's retry obligation, and a late older failure can reopen repaired work. The fix is either to enforce the lease across every GraphQL history writer, or to allocate durable attempt generations before acquisition and use conditional completion. Full sync and review-only sync also allocate their observation sequence at different points (after vs before fetching).
3. Credential-wide admission. Core, review recovery and actor enrichment can reach 40 concurrent GraphQL requests per credential. Admission and Retry-After handling are per client, with no shared secondary-limit cooldown across clients or processes on the same credential. GitHub treats staying under 100 concurrent requests as necessary but not sufficient, so this wants one credential-scoped dispatcher with a shared cooldown rather than a smaller worker count.

Also worth knowing for the release that eventually carries this: writable open upgrades every archive to schema 15, which v0.12.0 then refuses to read, so downgrading needs a backup.

Happy to pair on the receipt contract. #217 stays parked with this, as you noted.