Skip to content

fix(backend): validate Orca's composite worktree id on teardown - #1406

Open
birbprophet wants to merge 1 commit into
kunchenguid:mainfrom
birbprophet:fm/fix-orca-teardown
Open

fix(backend): validate Orca's composite worktree id on teardown#1406
birbprophet wants to merge 1 commit into
kunchenguid:mainfrom
birbprophet:fm/fix-orca-teardown

Conversation

@birbprophet

Copy link
Copy Markdown

The defect

The Orca runtime backend could not tear down any task it spawned.
Every attempt refused with:

REFUSED: Orca endpoint metadata for task <id> is malformed or inconsistent; preserving task state.

Orca's native worktree id is a composite, <repo-uuid>::<absolute-path>.
bin/fm-spawn.sh records that id faithfully into state/<id>.meta as orca_worktree_id=, and bin/backends/orca.sh hands it back to Orca as --worktree "id:<value>", both correct.
The Orca branch of fm_backend_validate_task_endpoint then validated that value with fm_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_valid validates 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_valid is untouched, so tmux, herdr, zellij, and cmux validation keep exactly their current strictness.
  • Orca's terminal= keeps atom validation; real handles are simple atoms such as term_ad314371-e22d-4ac8-a541-546a8eb29ca7, confirmed against live metadata before relying on it.
  • Teardown's existing safety is unchanged: it still resolves the recorded worktree id through Orca, verifies the resolved path matches the recorded worktree= path, and refuses rather than deleting anything on any mismatch.
  • bin/fm-spawn.sh is unchanged; the id it records was already correct.

Empirical basis

Verified against the live CLI rather than assumed, with Orca 1.4.162:

orca worktree list --json

All 200 listed worktrees shared a single repoId, and every id equalled <repoId>::<path>:

3bbb3ee8-c628-4390-992c-af5118987e86::/Users/benjamintang/Documents/GitHub/diffusal
3bbb3ee8-c628-4390-992c-af5118987e86::/private/tmp/diffusal-wt-base-main

orca --help documents the same composite as the selector form id:<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.md records this evidence and docs/orca-backend.md documents the metadata shape.

Test evidence

New coverage in tests/fm-backend-orca.test.sh:

  • test_endpoint_validation_pins_orca_worktree_id_shape pins 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_id drives bin/fm-teardown.sh end to end with the composite id and asserts the id reaches orca worktree rm unchanged.
  • test_teardown_refuses_malformed_orca_worktree_id_before_mutation asserts a malformed id still refuses before any runtime call and preserves task metadata.

Red before, green after:

# against the unfixed code
not ok - Orca teardown must accept the composite worktree id Orca actually issues
REFUSED: Orca endpoint metadata for task orcacompositez1 is malformed or inconsistent; preserving task state.: expected exit 0, got 1

# after the fix
ok - Orca endpoint identity: accepts the native composite worktree id and rejects every looser shape
ok - fm-teardown.sh backend=orca: tears down a task recorded with Orca's native composite worktree id
ok - fm-teardown.sh backend=orca: refuses a malformed worktree id before any runtime call

Mutation proof - each mutation applied to the fixed code turned the suite red on exactly the value it newly admitted:

Mutation Failure
Drop the absolute-path requirement not ok - Orca worktree id shape check accepted '3bbb3ee8-…-af5118987e86::'
Accept any non-empty first segment instead of a uuid not ok - Orca worktree id shape check accepted 'not-a-uuid::/tmp/wt'
Drop the path charset check not ok - Orca worktree id shape check accepted '3bbb3ee8-…-af5118987e86::/tmp/wt other'

Real-world proof: bin/fm-teardown.sh smoke-b3 succeeded 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.sh pins 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 full backend-dispatch family all pass.

Pre-existing failure found, not addressed here

tests/fm-backend-orca.test.sh's test_spawn_releases_orca_resources_when_metadata_write_fails fails 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 errexit to a failed redirection on a compound command:

$ bash --version | head -1
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin25)
$ mkdir -p /tmp/dirtest.meta
$ bash -c 'set -eu; { echo hi; } > /tmp/dirtest.meta; echo "CONTINUED past failed redirect"; exit 0'
bash: /tmp/dirtest.meta: Is a directory
CONTINUED past failed redirect

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.

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.
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.

1 participant