Skip to content

docs(pegboard): note runner config upsert split write risk#4624

Merged
MasterPtato merged 1 commit intomainfrom
04-12-docs_pegboard_note_runner_config_upsert_split_write_risk
Apr 13, 2026
Merged

docs(pegboard): note runner config upsert split write risk#4624
MasterPtato merged 1 commit intomainfrom
04-12-docs_pegboard_note_runner_config_upsert_split_write_risk

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@claude
Copy link
Copy Markdown

claude bot commented Apr 13, 2026

PR Review: docs(pegboard): note runner config upsert split write risk

This is a small but valuable documentation PR that adds a TODO comment flagging a known data consistency risk in engine/packages/pegboard/src/ops/runner_config/upsert.rs.

Summary

The comment correctly identifies a split-write atomicity gap: the pegboard_runner_config_upsert function performs two non-transactional writes — an Epoxy propose and a local UDB write — and a crash or error between them can leave state diverged.

Feedback

Comment style: The comment is accurate and concise. However, per the project's comment style guidelines, it should be written as complete sentences and avoid dashes (em dashes are explicitly disallowed per CLAUDE.md):

Write comments as normal, complete sentences. Avoid fragmented structures with parentheticals and dashes. Do not use em dashes.

Consider rephrasing to:

// TODO: This write is non-transactional. The Epoxy propose and the local UDB write
// can diverge if the process crashes or errors between them.

Tracking the follow-up: Since this is flagged as a TODO, linking to a GitHub issue (e.g., // TODO(#NNNN): ...) would make it easier to prioritize and avoid the risk being forgotten.

No code or logic concerns. The change is documentation-only and introduces no regressions.


Overall this is a good habit — surfacing non-obvious consistency risks inline is useful for future maintainers. Minor style nit on the comment phrasing.

@NathanFlurry NathanFlurry force-pushed the 04-12-docs_pegboard_note_runner_config_upsert_split_write_risk branch from b87b389 to 236286b Compare April 13, 2026 02:28
@NathanFlurry NathanFlurry force-pushed the 04-12-fix_pegboard_unpack_global_runner_config_keys_correctly branch from 41a7073 to 1c4bc6c Compare April 13, 2026 02:28
@NathanFlurry NathanFlurry marked this pull request as ready for review April 13, 2026 05:06
@NathanFlurry NathanFlurry force-pushed the 04-12-docs_pegboard_note_runner_config_upsert_split_write_risk branch from 236286b to de32947 Compare April 13, 2026 05:38
@NathanFlurry NathanFlurry force-pushed the 04-12-fix_pegboard_unpack_global_runner_config_keys_correctly branch from a481581 to f7b03f5 Compare April 13, 2026 05:50
@NathanFlurry NathanFlurry force-pushed the 04-12-docs_pegboard_note_runner_config_upsert_split_write_risk branch from de32947 to 66df461 Compare April 13, 2026 05:50
@NathanFlurry NathanFlurry force-pushed the 04-12-fix_pegboard_unpack_global_runner_config_keys_correctly branch from f7b03f5 to 839c3e4 Compare April 13, 2026 07:03
@NathanFlurry NathanFlurry force-pushed the 04-12-docs_pegboard_note_runner_config_upsert_split_write_risk branch from 66df461 to 0392214 Compare April 13, 2026 07:03
@github-actions
Copy link
Copy Markdown
Contributor

Preview packages published to npm

Install with:

npm install rivetkit@pr-4624

All packages published as 0.0.0-pr.4624.bcf7119 with tag pr-4624.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-bcf7119
docker pull rivetdev/engine:full-bcf7119
Individual packages
npm install rivetkit@pr-4624
npm install @rivetkit/react@pr-4624
npm install @rivetkit/rivetkit-native@pr-4624
npm install @rivetkit/sqlite-wasm@pr-4624
npm install @rivetkit/workflow-engine@pr-4624

Copy link
Copy Markdown
Contributor

MasterPtato commented Apr 13, 2026

Merge activity

  • Apr 13, 8:40 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 13, 8:55 PM UTC: Graphite rebased this pull request as part of a merge.
  • Apr 13, 8:56 PM UTC: @MasterPtato merged this pull request with Graphite.

@MasterPtato MasterPtato changed the base branch from 04-12-fix_pegboard_unpack_global_runner_config_keys_correctly to graphite-base/4624 April 13, 2026 20:51
@MasterPtato MasterPtato changed the base branch from graphite-base/4624 to main April 13, 2026 20:53
@MasterPtato MasterPtato force-pushed the 04-12-docs_pegboard_note_runner_config_upsert_split_write_risk branch from 8b8a9c8 to 4fd86c2 Compare April 13, 2026 20:54
@MasterPtato MasterPtato merged commit 0215b9f into main Apr 13, 2026
17 of 21 checks passed
@MasterPtato MasterPtato deleted the 04-12-docs_pegboard_note_runner_config_upsert_split_write_risk branch April 13, 2026 20:56
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.

2 participants