Repair delegated package audit dual writes

[kentcdodds]

Summary

• route package_scope_delegated_access audit events to both APP_DB and AUDIT_DB during the remaining expand phase
• retry known transient D1 write failures independently and return sink-specific partial-failure status
• test APP_DB failure, AUDIT_DB failure, successful counterpart persistence, and transient retry recovery

Production evidence

• Before repair: NOT VERIFIED — closed-window APP_DB=275, AUDIT_DB=274; the missing row was account/package_scope_delegated_access at 2026-08-01T20:09:48.495Z.
• Backfill: inserted exactly that one missing row after the repair deployed.
• After repair: VERIFIED — closed window 2026-07-31T23:47:19Z–2026-08-01T21:22:36Z had APP_DB=299, AUDIT_DB=299, missing=0, extra=0, exact multiset match.
• Retention: VERIFIED — a uniquely named 181-day-old AUDIT_DB canary inserted at 21:25Z had remaining=0 after the normal hourly retention run, checked at 2026-08-01T22:06:29Z.

Validation

• focused audit-log node tests passed
• focused package-owner Workers test passed
• CI=1 npm run validate passed
• CI passed after rerunning one unrelated inbound Workers-test flake

Review response

CodeRabbit correctly identified that Promise.all plus a broad catch obscured partial writes and lacked retry coverage. The implementation now uses the shared transient D1 retry policy, waits for both sinks with Promise.allSettled, reports failed sink names, preserves successful writes, and has explicit partial-failure tests.

System recap — extends existing primitives (medium risk)

Mode: recap · Base: main @ 38f3be9d · Head: 09e7d7de

Classification: extends — delegated package ownership reaches both audit sinks, whose transient and partial failures are now detected explicitly.

Primitives touched

Primitive	Group	Impact
saved-packages	assistant	extends — delegated scope resolution routes its audit sink correctly
community-listings	assistant	composes — forwards the environment through shared ownership resolution
audit-d1	storage	extends — retries transient writes and reports partial sink failures

System map

Delegated package and community operations resolve ownership and independently persist the resulting event to both expand-phase D1 sinks.

Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).

flowchart LR
	savedPackages["saved-packages<br/>Saved packages"]:::extended
	communityListings["community-listings<br/>Community package listings"]:::touched
	auditDb["audit-d1<br/>Audit database"]:::extended
	savedPackages -->|"delegated ownership environment"| auditDb
	communityListings -->|"shared delegated ownership path"| auditDb
	classDef touched fill:#1a7f37,color:#fff
	classDef extended fill:#9a6700,color:#fff
	classDef added fill:#cf222e,color:#fff
	classDef untouched fill:#57606a,color:#fff

Loading

Invariants

• Stable user ownership and package-scope authorization are unchanged.
• Successful writes survive a counterpart sink failure; audit failures remain fail-open for request behavior but are machine-detectable and logged by sink.
• This repair does not stop the legacy APP_DB write.

Conductor report

STATUS: done

What shipped: delegated package-scope audit events now dual-write correctly; known transient D1 failures retry independently and partial failures identify the failed sink. The one historical missing row was backfilled.

Production evidence: VERIFIED. Closed-window exact multiset parity is APP_DB=299 and AUDIT_DB=299 with zero missing/extra rows. AUDIT_DB retention is VERIFIED: the normal hourly lane removed a uniquely named 181-day-old canary by 22:06Z.

Risk self-assessment: medium — additive routing/retry repair and one evidence-driven backfill; no destructive schema change.

Merged/deployed: yes — PR #1142 / yes — production deploy.

Scope spill: package/community capability call sites and central audit sink only; no email, entitlements, vectorize, or index.ts changes.

  

Summary by CodeRabbit

• Bug Fixes

  • Improved package ownership and delegated-access handling across publishing, updating, deleting, and retrieving packages.
  • Audit events now use the appropriate audit environment when delegated access is recorded.
  • Improved audit-event reliability with retries and independent persistence, so one failed destination no longer prevents other audit records from being saved.
  • Audit failures now provide clearer information about which destinations were affected.

• Tests

  • Expanded coverage for ownership checks, delegated access, rejected requests, stale identity scenarios, retries, and partial audit failures.

[coderabbitai]

📝 Walkthrough

Walkthrough

resolvePackageOwnerContext now accepts the worker environment instead of only APP_DB. MCP handlers and worker tests pass the environment, while owner resolution derives application and audit database bindings from it. Audit event persistence now retries D1 writes and reports partial sink failures.

Changes

Package owner environment flow

Layer / File(s)	Summary
Update owner resolver database access packages/worker/src/package-registry/package-owner.ts	The resolver accepts APP_DB, AUDIT_DB, and SENTRY_ENVIRONMENT. It reads APP_DB and derives the audit database from the environment.
Update callers and worker tests packages/worker/src/mcp/capabilities/community/*, packages/worker/src/mcp/capabilities/packages/*, packages/worker/src/package-registry/package-owner.workers.test.ts	MCP handlers and worker tests pass the full environment to resolvePackageOwnerContext. Existing ownership, delegation, rejection, and identity scenarios remain covered.

Audit event persistence

Layer / File(s)	Summary
Retry and isolate audit writes packages/worker/src/audit-log.ts	Audit inserts use D1 retry handling. Application and audit sinks run independently. The function returns persistence status and failed sink names.
Validate retry and partial failures packages/worker/src/audit-log.node.test.ts	Tests cover transient retry success, permanent failures in either sink, persisted rows, failure metadata, and warning output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• kentcdodds/kody#970: Both changes update resolvePackageOwnerContext and worker tests for stable user identity handling.
• kentcdodds/kody#976: Both changes update the publish-external-push.ts owner-context invocation.
• kentcdodds/kody#1018: Both changes modify packages/worker/src/audit-log.ts and its import usage.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: repairing delegated package audit writes to both database sinks.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/reporting-off-d1-0dbb

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔎 Preview deployed: https://kody-pr-1142.kody-a99.workers.dev

Worker: kody-pr-1142
D1: kody-pr-1142-db
KV: kody-pr-1142-oauth-kv

Mocks:

• cloudflare: https://kody-pr-1142-mock-cloudflare.kody-a99.workers.dev/__mocks (kody-pr-1142-mock-cloudflare)

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/worker/src/package-registry/package-owner.ts`:
- Line 86: Update logAuditEvent in audit-log.ts to handle Promise.all dual-write
partial failures explicitly: detect when either audit sink fails, preserve the
successful-write result, and surface the failure instead of swallowing it behind
only the audit-event-failed log. Add tests covering each sink failing while the
other succeeds, including the expected retry or failure propagation behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7add499c-20b0-4aff-9b72-f688d204f797

📥 Commits

Reviewing files that changed from the base of the PR and between 38f3be9 and a91a355.

📒 Files selected for processing (11)

• packages/worker/src/mcp/capabilities/community/publish.ts
• packages/worker/src/mcp/capabilities/community/unpublish.ts
• packages/worker/src/mcp/capabilities/packages/delete-package.ts
• packages/worker/src/mcp/capabilities/packages/get-git-remote.ts
• packages/worker/src/mcp/capabilities/packages/get-package.ts
• packages/worker/src/mcp/capabilities/packages/list-packages.ts
• packages/worker/src/mcp/capabilities/packages/package-update.ts
• packages/worker/src/mcp/capabilities/packages/publish-external-push.ts
• packages/worker/src/mcp/capabilities/packages/save-package.ts
• packages/worker/src/package-registry/package-owner.ts
• packages/worker/src/package-registry/package-owner.workers.test.ts

[cursor]

@coderabbitai review

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/worker/src/audit-log.ts (1)

104-130: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Make retried audit inserts idempotent.

runD1WithRetry() can reissue the plain INSERT INTO audit_events ... after "D1_ERROR: Network connection lost" or similar transient failures. If D1 commits before the connection error reaches the Worker, the retry can create a second row because audit_events has no event-identity constraint. Add a stable audit event ID and protect the insert with INSERT OR IGNORE/ON CONFLICT; simulate a committed first write before the connection failure in the retry test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/worker/src/audit-log.ts` around lines 104 - 130, Make audit-event
writes idempotent across retries: update the audit insert in
packages/worker/src/audit-log.ts (lines 104-130) to include a stable event
identity and use INSERT OR IGNORE or an equivalent conflict-safe insert backed
by the corresponding uniqueness constraint. Update the retry scenario in
packages/worker/src/audit-log.node.test.ts (lines 75-145) to simulate the first
write committing before the connection failure, then verify the retry leaves
only one audit row.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/worker/src/audit-log.ts`:
- Around line 104-130: Make audit-event writes idempotent across retries: update
the audit insert in packages/worker/src/audit-log.ts (lines 104-130) to include
a stable event identity and use INSERT OR IGNORE or an equivalent conflict-safe
insert backed by the corresponding uniqueness constraint. Update the retry
scenario in packages/worker/src/audit-log.node.test.ts (lines 75-145) to
simulate the first write committing before the connection failure, then verify
the retry leaves only one audit row.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a1bbf7c-cfd2-483c-a511-a6136f7bb706

📥 Commits

Reviewing files that changed from the base of the PR and between a91a355 and 09e7d7d.

📒 Files selected for processing (2)

• packages/worker/src/audit-log.node.test.ts
• packages/worker/src/audit-log.ts