feat(workbench): caller-pinned expected_generation for put_file - #491
Merged
Conversation
The SDK publish surface accepts a caller-supplied expected_generation CAS, but the workbench put_file tool only offered replace=true, which stats the CURRENT generation and replaces against it. That protects the window inside one call and nothing else: a caller who read generation 1 while a competitor advanced to 2 would replace to 3 and destroy the competitor's write, reproduced live against a real stack. The stale caller had no way to say what it observed - the frozen tool schema rejected the field outright - while the workspace-path rename/remove surface already takes --expected-generation, so the platform had the concept everywhere except the one publish verb coordination cares about. put_file now accepts an optional expected_generation (integer >= 1, schema-validated on every routed surface and re-checked in the facade for direct handler callers): replace=true plus the pin builds the publish condition from the caller's observation and a stale pin surfaces as the typed Conflict with the current generation in the details; replace=true alone keeps the legacy self-stat semantics byte for byte; the pin without replace, and a zero pin, are typed InvalidArguments so the never-upsert contract holds. The collect CLI wrapper gains --expected-generation (requiring --replace, with a dedicated error sentence). Live verification on a real stack: the exact lost-update sequence now conflicts with the competitor's write preserved, a refreshed pin applies, and unpinned replace behaves unchanged. Signed-off-by: wchwawa <wch19961116@gmail.com>
wchwawa
force-pushed
the
feat/put-file-expected-generation
branch
from
August 25, 2026 04:37
4403201 to
f515fd0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Gap
The SDK publish surface takes a caller-supplied `expected_generation` CAS, but the workbench `put_file` tool only offered `replace=true`, which stats the current generation and replaces against it. That protects the window inside one call and nothing else. Reproduced live against a real stack: caller reads generation 1, a competitor CAS-advances to 2, the stale caller runs `replace=true` - rc 0, generation 3, competitor's write destroyed. The frozen tool schema rejected the field outright, while `workspace-path rename/remove` already take `--expected-generation`: the platform had the concept everywhere except the one publish verb coordination cares about. (#486 adds CLI qualification tooling but does not close this; the two do not conflict.)
Fix
`workbench_put_file` accepts optional `expected_generation` (integer >= 1):
Designs considered: caller-supplied pin (chosen, matches the SDK and the workspace-path precedent); CLI-side read-retry loop (still races, rejected); forbidding CLI replace (breaks legitimate uses, rejected).
Verification
Workspace clippy clean with -D warnings; 213 nokv/nokv-agent tests pass including the new conditional-CAS matrix (matching pin succeeds, stale pin conflicts with the competitor's bytes preserved, invalid shapes fail closed, schema-bypass zero pin re-checked). Live on a real stack: the exact lost-update sequence now conflicts, a refreshed pin applies at generation 3, and unpinned replace behaves unchanged.