fix(exports): keep private output outside source checkouts - #19
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: blocked before merge. Reviewed September 11, 2026, 6:57 PM ET / 22:57 UTC. ClawSweeper reviewWhat this changesBoth offline exporters reject output inside their source checkout, linked worktrees, or input archive using filesystem identity checks, with corresponding tests and documentation. Merge readiness⛔ Blocked before merge - 3 items remain This remains useful: current main lacks the checkout boundary, and no merged replacement supplies it. No blocking code defect was found; the intentional restriction on existing output destinations needs maintainer acceptance. Priority: P2 Review scores
Verification
How this fits togetherTelemetry's offline analysis commands read saved captures and produce private reports without contacting the deployed Worker or external services. The changed destination guard runs immediately before report directories and files are created. flowchart TD
A[Saved captures] --> B[Offline analysis]
B --> C[Output destination guard]
D[Checkout and archive identities] --> C
C -->|Protected destination| E[Reject before writing]
C -->|External destination| F[Private report files]
Decision needed
Why: The restriction is deliberate and documented, but accepting the interruption of existing operator commands is a compatibility decision rather than a code correctness judgment. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the shared privacy guard with explicit acceptance of the destination restriction and an operator transition to external private paths. Do we have a high-confidence way to reproduce the issue? Yes, from source: current main permits a new checkout-local destination outside the input archive. This review did not execute the writers. Is this the best way to solve the issue? Yes, subject to accepting the compatibility change: one shared filesystem-identity guard covers both writers without changing report formats or adding dependencies. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 6acfa31f5dad. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Maintainer decision: accept the intentional checkout-local output restriction in 23f03ee1b088a200ff0c1fc819a57c16108b442c. Commands using the former This accepts the compatibility risk and resolves the three maintainer-decision/transition items in the ClawSweeper review. Source is unchanged, and exact-head CI passed all 347 tests, vocabulary validation, typechecking, and the deployment dry run. |
Offline
npm:qualityandtelemetry:historyexports could write private results into this source repository, making accidental commits possible. Reject destinations inside the executing checkout, its linked worktrees, or the input archive before creating output. Shared filesystem identity checks also close case-alias bypasses and keep nested repositories from hiding a containing checkout.Unrelated private repositories remain supported, with existing private file modes and idempotency preserved. Update the examples and changelog, and ignore root
captures/andresults/directories as an additional precaution. This guard covers these two offline exporters; it does not prevent every possible way of publishing data.Validation:
npm run check(vocabulary, typecheck, all 347 tests) andnpx wrangler deploy --dry-runat23f03ee1b088a200ff0c1fc819a57c16108b442c. The deployment job was skipped.