chore(frontseat-api): GH-201 reserve removed fields, drop dead ExecutionMode - #229
chore(frontseat-api): GH-201 reserve removed fields, drop dead ExecutionMode#229jbadeau wants to merge 3 commits into
Conversation
85ce016 to
e1e8993
Compare
842ff91 to
38b929b
Compare
References to specific downstream products in the OSS surface (proto comments, API READMEs, the remote-only-execution design doc) become product-neutral: "IDE integrations" and "aggregating consumers". Regenerating also catches up pre-existing codegen drift: the committed TypeScript gen still contained the removed relations/query/order_by fields and predates protoc-gen-es 2.13.0, and bytestream.connect.go lagged its source proto's header comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ionMode Reserve the tags of three removed fields so future reuse cannot corrupt journaled/replayed OperationEvents: ActionCompleted 13 (sandbox_violations), plugin TaskConfig 9, CheckReleaseGuardResponse 9. Remove the ExecutionMode enum and BuildRequest.execution_mode (reserved too): builds are remote-only and nothing outside generated code references them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
38b929b to
e344977
Compare
e1e8993 to
6579662
Compare
There was a problem hiding this comment.
Approving — the four reservations are right, ExecutionMode is dead.
Two inline notes on the protos: TaskConfig and CheckReleaseGuardResponse need no reserved name (neither ever had a field 9), and BuildRequest.sandbox_mode = 9 is a fifth removed field still unreserved and untracked in #201.
Stale docs: docs/design/remote-only-execution.md still describes ExecutionMode as a live enum with LOCAL/REMOTE/AUTO meanings, and still points at ActionCompleted.sandbox_violations. This PR already edits that file, so both fold into the same pass.
ExecutionMode is an exported symbol of the in-repo TypeScript package @frontseat/api. That package is unpublished — version 0.0.0, absent from npm and GitHub Packages — and sibling repos consume it by path per docs/design/cross-repo-api-adoption.md, so the removal is wire-safe and source-breaking for those consumers next time they pick up the generated package.
This review was published with assistance from Claude.
| google.protobuf.Struct options = 6; | ||
| LifecycleBinding lifecycle = 7; | ||
| repeated string workspace_inputs = 8; | ||
| reserved 9; |
There was a problem hiding this comment.
The number alone is the complete statement here — there is no identifier to reserve. TaskConfig has never had a field 9: every revision goes straight from repeated string workspace_inputs = 8; to CacheConfig cache = 10;, and CheckReleaseGuardResponse carries the same original 8→10 gap.
BuildRequest and ActionCompleted reserve both number and name (reserved 6; reserved "execution_mode";) because those fields really existed. These two never did.
Worth a short comment on both saying the tag was never assigned, so a future reader does not go hunting for a removed field that never existed.
There was a problem hiding this comment.
Agreed — number-only is already what's committed (reserved 9; with no name on both), since neither TaskConfig nor CheckReleaseGuardResponse ever had a field 9. The clarifying comment you asked for rides with the same frontseat generate pass as the sandbox_mode reservation (see the other thread): it's a .proto edit, so it moves the .frontseat-gen.lock fingerprint, which I can't reproduce reliably offline. Tracked, not dropped.
Generated by Claude Code
| reserved 6; | ||
| reserved "execution_mode"; | ||
| CacheMode cache_mode = 7; | ||
| int32 max_workers = 8; |
There was a problem hiding this comment.
BuildRequest is missing one: sandbox_mode = 9 (TYPE_STRING) was removed without a reserved, and it is not on the #201 checklist — the one hole left open in the message this PR is otherwise closing.
| int32 max_workers = 8; | |
| int32 max_workers = 8; | |
| reserved 9; | |
| reserved "sandbox_mode"; |
No live hazard: sandbox_mode never shipped in a release, and BuildRequest is never journaled (only OperationEvent is), so tag 9 cannot mis-parse anything in the field. Worth reserving purely for consistency with the execution_mode = 6 reservation this PR already makes in the same message.
Note that reserved is encoded into the descriptor as reserved_range/reserved_name — visible in this PR own .pb.go hunk — so the generated Go and TS, and .frontseat-gen.lock, need regenerating in the same commit.
There was a problem hiding this comment.
Landed the stale-docs half of this in 2f6cb4c: remote-only-execution.md §5.1 no longer describes ExecutionMode as a live LOCAL/REMOTE/AUTO enum, §5.4 no longer points at ActionCompleted.sandbox_violations as a live field, and §10 lists ActionCompleted as 1..12 (field 13 reserved) — all consistent with the reservations this PR already makes.
The sandbox_mode = 9 reservation I did not push here, deliberately. Reserving it edits frontseat.proto, which regenerates .pb.go + the TS package and the .frontseat-gen.lock fingerprint — and that lock is computed by the frontseat sandbox (buf:generate-check) with staged paths I can't reproduce outside the daemon/grid (a pristine find | sha256sum reproduction doesn't match the committed lock, so hand-writing it would just redden the check on an approved PR). It wants a real frontseat generate run. I can do the proto edit + regen the moment I'm in an environment with the daemon, or hand it back to you to run frontseat generate — whichever you prefer. Flagging so the sandbox_mode hole stays tracked and isn't silently dropped.
Generated by Claude Code
… fields The design doc still described machinery this PR's proto reservations removed: - Section 5.1 described ExecutionMode as a live enum with LOCAL/REMOTE/AUTO meanings; builds are remote-only and BuildRequest.execution_mode + the ExecutionMode enum are removed (tags reserved). State that instead. - Section 5.4 pointed at ActionCompleted.sandbox_violations as the live "why it failed" signal; that field is removed and its tag reserved, so describe the surviving mechanism (violations surfaced through the action's failure output). - Section 10 listed ActionCompleted "fields 1..13"; field 13 (sandbox_violations) is reserved, so it is 1..12. Docs only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYAgCD9HZHZWEuH4vF17p1
|
We'll run it — you're blocked on a fingerprint only the daemon can produce, and we have one. It'll be a follow-up once this lands rather than a push onto your branch, carrying Confirmed the number-only reservations are right, incidentally: One thing blocking it: the branch conflicts because This comment was published with assistance from Claude. |
3 participants
Proto-hygiene checklist item of #201, stacked on #228 (both regenerate the same files; merge #228 first, then retarget this to main).
reservedfor three removed fields whose tag reuse would corrupt journaled/replayed OperationEvents:ActionCompletedfield 13 (sandbox_violations), pluginTaskConfigfield 9,CheckReleaseGuardResponsefield 9.ExecutionModeenum andBuildRequest.execution_mode— builds are remote-only and there are zero non-generated references (verified by grep and a fullgo buildof the workspace).🤖 Generated with Claude Code