fix(backend): validate Orca's composite worktree id on teardown - #1406
Open
birbprophet wants to merge 1 commit into
Open
fix(backend): validate Orca's composite worktree id on teardown#1406birbprophet wants to merge 1 commit into
birbprophet wants to merge 1 commit into
Conversation
Orca's native worktree id is the composite "<repo-uuid>::<absolute-path>", and its repo uuid is identical across every worktree of one repository, so the path segment is the only discriminator. Endpoint validation checked that value with the shared atom charset, which rejects ':' and '/', so every Orca task refused teardown as malformed even though its metadata was correct. Orca worktree ids now get their own shape check that accepts exactly the composite Orca issues and rejects an empty value, a missing separator, a non-uuid segment, a relative path, whitespace, newlines, and shell metacharacters. The shared atom validator keeps its current strictness for tmux, herdr, zellij, cmux, and for Orca's terminal handle, and teardown's existing id/path resolution and mismatch refusals are unchanged. Orca fixtures now carry the composite id shape Orca actually issues rather than a bare atom it never returns.
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
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.
The defect
The Orca runtime backend could not tear down any task it spawned.
Every attempt refused with:
Orca's native worktree id is a composite,
<repo-uuid>::<absolute-path>.bin/fm-spawn.shrecords that id faithfully intostate/<id>.metaasorca_worktree_id=, andbin/backends/orca.shhands it back to Orca as--worktree "id:<value>", both correct.The Orca branch of
fm_backend_validate_task_endpointthen validated that value withfm_backend_endpoint_atom_valid, which rejects any character outside[A-Za-z0-9._@%+-].The composite contains
:and/, so validation always failed and teardown refused correct metadata.Reproduced against real task
smoke-b3, whose metadata was intact.The fix
fm_backend_endpoint_orca_worktree_id_validvalidates the id shape Orca actually issues: a uuid-shaped segment, the literal::separator, and an absolute path drawn from a charset that excludes whitespace, newlines, and every shell metacharacter, so a validated id cannot escape the quoted argument it is passed as.Scope of the change is deliberately narrow:
fm_backend_endpoint_atom_validis untouched, so tmux, herdr, zellij, and cmux validation keep exactly their current strictness.terminal=keeps atom validation; real handles are simple atoms such asterm_ad314371-e22d-4ac8-a541-546a8eb29ca7, confirmed against live metadata before relying on it.worktree=path, and refuses rather than deleting anything on any mismatch.bin/fm-spawn.shis unchanged; the id it records was already correct.Empirical basis
Verified against the live CLI rather than assumed, with Orca 1.4.162:
All 200 listed worktrees shared a single
repoId, and everyidequalled<repoId>::<path>:orca --helpdocuments the same composite as the selector formid:<repo-id>::<path>.Because the repo uuid never varies within a repository, the path segment is the only discriminator and a bare uuid identifies nothing.
docs/verification/runtime-backends.mdrecords this evidence anddocs/orca-backend.mddocuments the metadata shape.Test evidence
New coverage in
tests/fm-backend-orca.test.sh:test_endpoint_validation_pins_orca_worktree_id_shapepins the real composite as valid and pins each rejected shape as invalid: empty, no separator, single-colon separator, empty uuid segment, non-uuid segment, short uuid, empty path, relative path, embedded space, embedded newline,;,$(...), a quote, a pipe, and a second::.test_teardown_accepts_native_composite_orca_worktree_iddrivesbin/fm-teardown.shend to end with the composite id and asserts the id reachesorca worktree rmunchanged.test_teardown_refuses_malformed_orca_worktree_id_before_mutationasserts a malformed id still refuses before any runtime call and preserves task metadata.Red before, green after:
Mutation proof - each mutation applied to the fixed code turned the suite red on exactly the value it newly admitted:
not ok - Orca worktree id shape check accepted '3bbb3ee8-…-af5118987e86::'not ok - Orca worktree id shape check accepted 'not-a-uuid::/tmp/wt'not ok - Orca worktree id shape check accepted '3bbb3ee8-…-af5118987e86::/tmp/wt other'Real-world proof:
bin/fm-teardown.sh smoke-b3succeeded with this change, removing that task's record and releasing its Orca worktree while preserving the scout report.Fixture correction
Existing Orca teardown fixtures recorded bare ids such as
wt-teardown, a shape Orca never issues, which is why the gap survived a suite that otherwise covers Orca cleanup well.They now carry the composite shape, and
tests/fm-teardown-endpoint-safety.test.shpins the same real shape as its valid Orca record.Checks run
bin/fm-lint.sh,bin/fm-doc-audience-check.sh, a syntax check over the canonical shell surface,tests/fm-backend.test.sh,tests/fm-teardown.test.sh,tests/fm-teardown-endpoint-safety.test.sh, and the fullbackend-dispatchfamily all pass.Pre-existing failure found, not addressed here
tests/fm-backend-orca.test.sh'stest_spawn_releases_orca_resources_when_metadata_write_failsfails on this branch's base, independently of this change, verified by stashing the change and re-running.The cause is real but environment-dependent.
macOS system bash 3.2 does not apply
errexitto a failed redirection on a compound command:So when
bin/fm-spawn.sh's metadata write at line 1648 fails on macOS, spawn continues, launches the harness, and reports success, leaving an Orca terminal and worktree with no task record.CI does not surface this because the Orca suite is gated on the Orca binary being present.
Left untouched as out of scope for this fix; it deserves its own change.