-
Notifications
You must be signed in to change notification settings - Fork 0
feat(fm-backend): extend legacy-metadata self-repair to zellij and cmux backends #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f931fb2
0dc4fd0
8c8b31c
9fad788
6ae45ce
6091c3e
a2ddaa3
3cf7da5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -483,10 +483,6 @@ fm_backend_validate_task_endpoint() { # <meta-file> <task-id> | |
| fi | ||
| ;; | ||
| zellij) | ||
| [ "$binding" = "$id" ] || { | ||
| echo "REFUSED: legacy Zellij endpoint metadata for task $id lacks an exact task binding; preserving task state." >&2 | ||
| return 1 | ||
| } | ||
| recorded_session=$(fm_backend_meta_exact_value "$meta" zellij_session) || recorded_session= | ||
| tab=$(fm_backend_meta_exact_value "$meta" zellij_tab_id) || tab= | ||
| pane=$(fm_backend_meta_exact_value "$meta" zellij_pane_id) || pane= | ||
|
|
@@ -497,8 +493,29 @@ fm_backend_validate_task_endpoint() { # <meta-file> <task-id> | |
| echo "REFUSED: Zellij endpoint metadata for task $id is malformed or inconsistent; preserving task state." >&2 | ||
| return 1 | ||
| fi | ||
| if [ "$binding" = "$id" ]; then | ||
| : | ||
| else | ||
| fm_backend_source zellij || return 1 | ||
| if fm_backend_zellij_pane_verifies_task "$recorded_session" "$tab" "$pane" "$id" 2>/dev/null; then | ||
| echo "zellij endpoint self-repair: appending endpoint_task_id=$id to metadata for legacy task $id" >&2 | ||
| printf 'endpoint_task_id=%s\n' "$id" >> "$meta" || return 1 | ||
|
Comment on lines
+500
to
+502
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win Preserve metadata record boundaries during repair. At Line 502 and Line 556, a metadata file accepted without a final LF causes
Proposed fix+fm_backend_append_endpoint_task_id() { # <meta-file> <task-id>
+ local meta=$1 id=$2 last_byte
+ last_byte=$(tail -c 1 "$meta") || return 1
+ [ -z "$last_byte" ] || printf '\n' >> "$meta" || return 1
+ printf 'endpoint_task_id=%s\n' "$id" >> "$meta"
+}
+
- printf 'endpoint_task_id=%s\n' "$id" >> "$meta" || return 1
+ fm_backend_append_endpoint_task_id "$meta" "$id" || return 1📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| else | ||
| echo "REFUSED: legacy Zellij endpoint metadata for task $id lacks an exact task binding; pane verification failed or pane does not belong to this task; preserving task state." >&2 | ||
| return 1 | ||
| fi | ||
| fi | ||
| ;; | ||
| orca) | ||
| # No legacy-metadata self-repair here, unlike herdr/zellij/cmux: Orca | ||
| # has no verified live-identity-check primitive to prove a recorded | ||
| # terminal/worktree still belongs to this exact task (see | ||
| # bin/backends/orca.sh's fm_backend_orca_json_get header - only | ||
| # worktree-id, worktree-path, terminal-handle, worktree-terminal-handle, | ||
| # and repo-id fields are verified real shapes; a task-owning label or | ||
| # title readback is not). Refusing unconditionally when the binding is | ||
| # absent is the correct, intended behavior here, not a gap: the brief's | ||
| # own rule is "refuse ONLY when it cannot be verified". | ||
| [ "$binding" = "$id" ] || { | ||
| echo "REFUSED: legacy Orca endpoint metadata for task $id lacks an exact task binding; preserving task state." >&2 | ||
| return 1 | ||
|
|
@@ -522,10 +539,6 @@ fm_backend_validate_task_endpoint() { # <meta-file> <task-id> | |
| window=$terminal | ||
| ;; | ||
| cmux) | ||
| [ "$binding" = "$id" ] || { | ||
| echo "REFUSED: legacy cmux endpoint metadata for task $id lacks an exact task binding; preserving task state." >&2 | ||
| return 1 | ||
| } | ||
| workspace=$(fm_backend_meta_exact_value "$meta" cmux_workspace_id) || workspace= | ||
| surface=$(fm_backend_meta_exact_value "$meta" cmux_surface_id) || surface= | ||
| if [ -z "$workspace" ] || [ -z "$surface" ] || [ "$window" != "$workspace:$surface" ] \ | ||
|
|
@@ -534,6 +547,18 @@ fm_backend_validate_task_endpoint() { # <meta-file> <task-id> | |
| echo "REFUSED: cmux endpoint metadata for task $id is malformed or inconsistent; preserving task state." >&2 | ||
| return 1 | ||
| fi | ||
| if [ "$binding" = "$id" ]; then | ||
| : | ||
| else | ||
| fm_backend_source cmux || return 1 | ||
| if fm_backend_cmux_surface_verifies_task "$workspace" "$surface" "$id" 2>/dev/null; then | ||
| echo "cmux endpoint self-repair: appending endpoint_task_id=$id to metadata for legacy task $id" >&2 | ||
| printf 'endpoint_task_id=%s\n' "$id" >> "$meta" || return 1 | ||
| else | ||
| echo "REFUSED: legacy cmux endpoint metadata for task $id lacks an exact task binding; surface verification failed or surface does not belong to this task; preserving task state." >&2 | ||
| return 1 | ||
| fi | ||
| fi | ||
| ;; | ||
| esac | ||
| # shellcheck disable=SC2034 # Output globals are consumed by sourcing callers. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,20 @@ Firstmate does not attempt to close the macOS window because cmux's socket canno | |
| Real tests share the captain's running app rather than creating an isolated cmux session. | ||
| `tests/cmux-test-safety.sh` permits cleanup only for an exact currently listed `fm-test-` workspace and never enumerates and closes unrelated workspaces or relaunches the app. | ||
|
|
||
| ## Endpoint metadata | ||
|
|
||
| ```text | ||
| backend=cmux | ||
| window=<workspace-uuid>:<surface-uuid> | ||
| cmux_workspace_id=<workspace-uuid> | ||
| cmux_surface_id=<surface-uuid> | ||
| ``` | ||
|
|
||
| The UUID pair is the active endpoint authority within one app run. | ||
| Workspace UUIDs are not stable across an app relaunch, so recovery searches by the scoped title and then resolves the current surface id. | ||
|
Comment on lines
+113
to
+123
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Keep one authoritative endpoint-metadata schema. The new sections duplicate endpoint fields and identity rules already documented earlier in each file. Remove the repeated schema and rules, or replace them with a short link to the existing section. Keep the legacy self-repair paragraph.
📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| Legacy cmux metadata written before `endpoint_task_id=` existed self-repairs at teardown validation time instead of refusing outright: `fm_backend_validate_task_endpoint` verifies the live surface still exists within its recorded workspace and that the workspace still carries the task's label via `fm_backend_cmux_surface_verifies_task`, and only when both the surface membership and the workspace's title are confirmed to match (using `fm_backend_cmux_workspace_matches_label`'s scoped/bare-with-ambiguity rules), appends `endpoint_task_id=<id>` to the metadata file so the task tears down with no manual editing. When the live surface or workspace does not match - for example because the surface was recycled to a different workspace or the workspace was renamed to a different task's label - validation refuses without mutating the metadata, preserving the wrong-surface safety guarantee. | ||
|
|
||
| ## Active limits | ||
|
|
||
| - cmux is experimental, macOS-only, GUI-first, and requires the app running. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: trillium/firstmate
Length of output: 16134
🌐 Web query:
Claude Code MCP project .mcp.json command enableAllProjectMcpServers documentation💡 Result:
In Claude Code, enableAllProjectMcpServers is a configuration setting used to automatically approve and enable all MCP servers defined in a project's.mcp.json file, bypassing the individual per-server prompt [1][2][3]. Key points regarding its usage and limitations: 1. Purpose and Usage: By setting "enableAllProjectMcpServers": true in your Claude Code settings (typically in ~/.claude/settings.json), you authorize all project-scoped servers to run without needing to manually approve them each time [2][3]. 2. Important Security Constraint: As of Claude Code v2.1.196, this setting is ignored if the workspace folder is considered "untrusted" [4][1]. Even if "enableAllProjectMcpServers": true is committed to a project's.claude/settings.json file, Claude Code will not honor it until you have explicitly trusted the workspace by running the claude command in that folder and accepting the trust dialog [4][1]. This prevents a potential security risk where a cloned, untrusted repository could automatically execute its own configured servers [4][1]. 3. Recommended Alternative: For better security control, it is often recommended to use the enabledMcpjsonServers setting instead [1]. This allows you to explicitly list only the specific servers you trust by name, rather than using a blanket true setting that enables every server defined in the.mcp.json file [1]. 4. Troubleshooting: If your.mcp.json servers are stuck at "Pending approval," it is frequently because the workspace trust gate has not been cleared [4][1]. Additionally, note that entries in disabledMcpjsonServers will override enableAllProjectMcpServers, meaning a server explicitly disabled will not run regardless of your global "enable all" setting [1].
Citations:
Security Misconfiguration (CWE-829): Inclusion of Functionality from Untrusted Control Sphere
Reachability: Internal
Do not persist
enableAllProjectMcpServers=trueon account bootstrap.bin/claude-account.shwrites this global approval into$CLAUDE_CONFIG_DIR/.claude.json, anddocs/configuration.mddocuments it as intentional. Add MCP approval requirements behind an explicit captain opt-in for a known project/server, or keep Claude Code’s default per-server approval. Updatedocs/configuration.mdand the account bootstrap tests with the replacement behavior.🤖 Prompt for AI Agents