Skip to content

Fix codemode job metadata mutations - #571

Merged
kody-bot merged 2 commits into
mainfrom
cursor/fix-job-mutation-bind-d2e9
Jun 27, 2026
Merged

kody-bot merged 2 commits into
mainfrom
cursor/fix-job-mutation-bind-d2e9

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Jun 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Skip repo-session source publishing for metadata-only job updates such as disabling a scheduled job.
  • Keep source publishing for updates that affect job source or manifest-visible metadata.
  • Add regression coverage for codemode.job_update and codemode.job_delete through the real capability registry/service path with a D1-shaped production-style env.

Validation

  • npx vitest run packages/worker/src/jobs/service.node.test.ts packages/worker/src/mcp/run-codemode-registry.node.test.ts
  • npm run format:check
  • npm run typecheck
  • npm run lint (warnings only)
  • npm run validate

Rollout notes

  • After this is deployed, run codemode.job_update({ id: '504513c3-f29e-47f0-9ea1-402569ebef54', enabled: false }) or codemode.job_delete({ id: '504513c3-f29e-47f0-9ea1-402569ebef54' }) to disable/delete the temporary hrv-discord-reaction-poller scheduled job.
Open in Web Open in Cursor 

Summary by CodeRabbit

  • Bug Fixes
    • Job edits now only refresh source/artifact snapshots when code, name, schedule, timezone, or source selection/published commit changes.
    • Metadata-only job updates no longer trigger republishing or snapshot syncing.
    • Job deletion now consistently removes the job, its related entity source, and associated stored artifacts.
  • Improved Reliability
    • Artifact repository cleanup is now correctly scoped to the intended user when locating entity sources.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@coderabbitai

coderabbitai Bot commented Jun 27, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4e7d264c-b22b-42f3-80a0-8b4a4f6c3349

📥 Commits

Reviewing files that changed from the base of the PR and between bb0c511 and ffb2477.

📒 Files selected for processing (6)
  • packages/worker/src/jobs/service.node.test.ts
  • packages/worker/src/jobs/service.ts
  • packages/worker/src/mcp/run-codemode-registry.node.test.ts
  • packages/worker/src/repo/artifact-repo-cleanup.node.test.ts
  • packages/worker/src/repo/artifact-repo-cleanup.ts
  • packages/worker/src/repo/entity-sources.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/worker/src/mcp/run-codemode-registry.node.test.ts

📝 Walkthrough

Walkthrough

Job source lookups now use user-scoped entity-source queries. Job updates only sync source snapshots for selected field changes, and the codemode registry test adds production-shaped update and delete coverage.

Changes

Job mutation flow

Layer / File(s) Summary
User-scoped source lookup
packages/worker/src/repo/entity-sources.ts, packages/worker/src/jobs/service.ts, packages/worker/src/repo/artifact-repo-cleanup.ts, packages/worker/src/repo/artifact-repo-cleanup.node.test.ts
getEntitySourceByIdForUser is added and used by job source cleanup and artifact repo cleanup, with tests updated for the new lookup helper.
Conditional source sync
packages/worker/src/jobs/service.ts, packages/worker/src/jobs/service.node.test.ts
updateJob now calls syncArtifactSourceSnapshot only when selected update fields are present, and the params-update regression test asserts the sync path is skipped.
Codemode job update/delete test
packages/worker/src/mcp/run-codemode-registry.node.test.ts
The codemode registry test adds production-shaped DB and KV mocks, row builders, and a job mutation test that covers job_update and job_delete through mocked bindings.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • kentcdodds/kody#187: Changes updateJob snapshot syncing behavior for job updates, which this PR further gates.
  • kentcdodds/kody#331: Adds job_update and job_delete MCP flows that exercise the updated jobs service behavior.
  • kentcdodds/kody#511: Switches artifact-repo cleanup to the same user-scoped entity-source lookup used here.

Poem

A rabbit nibbled update leaves,
and hopped where user-scoped lookup breathes.
No extra sync for tiny tweak days,
just tidy jobs in gentler ways.
🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the PR’s main change around codemode job metadata mutation handling.
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.
✨ 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/fix-job-mutation-bind-d2e9

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

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

@kody-bot
kody-bot marked this pull request as ready for review June 27, 2026 06:08
@github-actions

github-actions Bot commented Jun 27, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-571.kentcdodds.workers.dev

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

Mocks:

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/jobs/service.ts`:
- Around line 925-931: The sync guard in shouldSyncJobSourceForUpdate misses
source-shape updates, so changes to sourceId or publishedCommit can bypass
syncing and leave updated with an unverified source pointer. Update
shouldSyncJobSourceForUpdate in service.ts to treat sourceId and publishedCommit
as sync-triggering fields, and make resolveUpdatedShape consistent by either
including those fields in the decision or rejecting/ignoring them before
constructing updated. Apply the same fix anywhere the update flow relies on this
predicate so source-related updates always force the matching snapshot to be
published.

In `@packages/worker/src/mcp/run-codemode-registry.node.test.ts`:
- Around line 253-257: The mock and test are allowing unscoped entity_sources
reads, which can hide cross-user access bugs. Update the D1 mock in
run-codemode-registry.node.test.ts to require userId scoping for entity_sources
lookups instead of accepting SELECT * FROM entity_sources WHERE id = ?, and
adjust the assertions around the delete/read flow to include user_id in the
lookup. Use the existing entity_sources select path and the related test case
that performs the post-delete read to ensure every read/write stays user-scoped.
- Around line 523-535: The JOB_MANAGER mock is too permissive and doesn’t verify
user-scoped routing, so this regression test can miss non-namespaced Durable
Object access. Tighten the mock in run-codemode-registry.node.test.ts by
asserting that idFromName receives a userId-prefixed name and that the syncAlarm
call is made with the expected userId. Use the JOB_MANAGER idFromName and
get().syncAlarm hooks to enforce the namespace contract so the test fails if
routing is not user-scoped.
🪄 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: ff40c5c2-a3a2-4846-8081-bc13cbf72212

📥 Commits

Reviewing files that changed from the base of the PR and between ca35ba5 and bb0c511.

📒 Files selected for processing (3)
  • packages/worker/src/jobs/service.node.test.ts
  • packages/worker/src/jobs/service.ts
  • packages/worker/src/mcp/run-codemode-registry.node.test.ts

Comment thread packages/worker/src/jobs/service.ts
Comment thread packages/worker/src/mcp/run-codemode-registry.node.test.ts Outdated
Comment thread packages/worker/src/mcp/run-codemode-registry.node.test.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@kody-bot
kody-bot merged commit f3b9deb into main Jun 27, 2026
5 checks passed
@kody-bot
kody-bot deleted the cursor/fix-job-mutation-bind-d2e9 branch June 27, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants