Skip to content

Commit d94b8bd

Browse files
cailmdaleyclaude
andcommitted
One canonical fiber id (realpath → enclosing .felt → slug) across /fibers and /state
The /fibers list and /state runtime keys disagreed on a fiber's id whenever the file was reached through a symlinked store. /fibers (FiberDocuments) emitted felt's *traversal* id — loom walking through `loom/.felt/shapepipe → project/.felt` yields `shapepipe/review-ngmix-v2-pr740` — while the runtime keyed the same fiber by the project-relative slug `review-ngmix-v2-pr740` (realpath against the project .felt). The kanban joins card→runtime by id, so a *running* worker went unmatched: the card sat in Drafts and any drag to In-flight snapped back. Unify on one rule, everywhere an id is assigned: canonical id = realpath(fiber file) → enclosing .felt → store-relative slug Because realpath resolves every symlink, the resolved path holds exactly one .felt segment — the store where the file physically lives — so the rule is host-invariant for git-synced stores (loom) and store-prefix-free for project- resident fibers, with no per-store identity. The Elixir mirror of Portolan's canonicalStoreRelativeId. - New Shuttle.FiberId owns the rule (ref_from_path/1, canonical_id/1, canonical_host_path/1, resolve_realpath/1). The poller's three private copies (fiber_ref_from_path, fiber_id_from_tail, resolve_realpath, canonical_host_path) collapse into it (-110 lines). - FiberDocuments canonicalizes each entry's `id` from the realpath of its md file; `felt_store` + `path` stay store-relative so remote clients still open the file by path. /fibers ids now equal /state runtime keys. - Fix a latent realpath bug surfaced by the new tests: the "never revisit a symlink" loop guard mis-flagged benign re-traversal of a shared prefix symlink (macOS `/tmp → /private/tmp`) as a loop. Replaced with OS-style hop-counting. - Fix FiberDocuments folding felt's stderr into stdout: a stray non-fiber .md (SPEC.md) made `felt ls` warn on stderr, corrupting the JSON and 500ing the whole /fibers endpoint. Parse stdout only. project_dir reverts to being only the worker's cwd; it never participates in id derivation. Tests pin the rule across loom-resident, project-resident-via-symlink, and portolan (symlink-into-loom-subpath) topologies. mix test green (241). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6aec5e1 commit d94b8bd

5 files changed

Lines changed: 379 additions & 125 deletions

File tree

lib/shuttle/fiber_documents.ex

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,13 @@ defmodule Shuttle.FiberDocuments do
4646
args = ["ls", "-s", "all", "-j"]
4747
args = if with_body?, do: args ++ ["--body"], else: args
4848

49-
case System.cmd("felt", args, cd: store, stderr_to_stdout: true) do
49+
# Do NOT fold stderr into stdout: felt prints `warning: failed to parse …`
50+
# for stray non-fiber `.md` files (SPEC.md, README.md) to stderr while still
51+
# emitting valid JSON on stdout and exiting 0. Capturing stderr would prepend
52+
# those warnings to the JSON and break Jason.decode for the whole store —
53+
# 500ing the entire /fibers endpoint. Felt's warnings land in the daemon log
54+
# instead; only stdout is parsed.
55+
case System.cmd("felt", args, cd: store) do
5056
{output, 0} ->
5157
decode_store(store, output)
5258

@@ -65,13 +71,21 @@ defmodule Shuttle.FiberDocuments do
6571
end
6672

6773
defp entry_for(store, %{"id" => id} = fiber) when is_binary(id) and id != "" do
74+
# `path` (and thus the physical file location) is derived from felt's
75+
# traversal id *before* we overwrite `id` — it stays store-relative so
76+
# remote clients open the file via `felt_store` + `path`. The card's logical
77+
# `id`, by contrast, is canonicalized: realpath → enclosing .felt → slug,
78+
# the one rule shared with /state runtime keying. For a fiber physically
79+
# rooted in this store the two coincide; for a symlink-traversed fiber (loom
80+
# walking through `loom/.felt/shapepipe → project/.felt`) the canonical id
81+
# drops the store prefix to match the project-relative runtime key.
6882
path = relative_felt_path(fiber)
6983
full_path = Path.join([store, ".felt", path])
7084

7185
entry = %{
7286
felt_store: store,
7387
path: path,
74-
fiber: fiber
88+
fiber: Map.put(fiber, "id", canonical_id(full_path, id))
7589
}
7690

7791
report_path = full_path |> Path.dirname() |> Path.join("report.html")
@@ -85,6 +99,13 @@ defmodule Shuttle.FiberDocuments do
8599

86100
defp entry_for(_store, _fiber), do: []
87101

102+
defp canonical_id(full_path, fallback) do
103+
case Shuttle.FiberId.canonical_id(full_path) do
104+
{:ok, id} -> id
105+
{:error, _} -> fallback
106+
end
107+
end
108+
88109
defp relative_felt_path(%{"id" => id, "entry_point" => true}) do
89110
"#{Path.basename(id)}.md"
90111
end

lib/shuttle/fiber_id.ex

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
defmodule Shuttle.FiberId do
2+
@moduledoc """
3+
The one canonical fiber-id rule, used everywhere Shuttle assigns an id.
4+
5+
canonical id = realpath(fiber file) → enclosing .felt → store-relative slug
6+
7+
Because `realpath` resolves every symlink, the resulting absolute path
8+
contains exactly one `.felt` segment — the store where the file is
9+
*physically* rooted. The slug below it is the id `felt ls` reports when run
10+
from inside that store. The same string therefore names the fiber on every
11+
daemon surface (`/api/v1/fibers` cards, `/api/v1/state` runtime keys,
12+
tmux/lifecycle keying), and is host-invariant for git-synced stores like
13+
loom — so cross-host dedup falls out for free, with no per-store identity.
14+
15+
This is the Elixir mirror of Portolan's `canonicalStoreRelativeId`
16+
(`server/src/canonicalFiberRef.ts`); the two implementations must agree.
17+
18+
## Symlink topologies it resolves uniformly
19+
20+
* **Loom-resident** (`loom/.felt/ai-futures/portolan/X`) → `ai-futures/portolan/X`.
21+
A project whose `.felt` symlinks *into* a loom subpath (portolan shape:
22+
`project/.felt → loom/.felt/ai-futures/portolan`) resolves to the same
23+
loom-relative slug, because realpath collapses the symlink onto loom.
24+
* **Project-resident** (shapepipe shape: `loom/.felt/shapepipe → project/.felt`)
25+
→ the project-relative slug `X`, because realpath collapses the loom
26+
symlink onto the project's own `.felt`, which is the physically-enclosing
27+
store.
28+
29+
In both cases the id is keyed against the store where the file physically
30+
lives; `project_dir` never participates — it is only the worker's cwd.
31+
"""
32+
33+
@typedoc "Canonical `(host, id)` for a fiber file: the directory holding the owning `.felt/`, and the store-relative slug."
34+
@type ref :: %{host: String.t(), id: String.t()}
35+
36+
@doc """
37+
Canonical `(host, id)` for an on-disk fiber path.
38+
39+
`host` is the directory that physically contains the owning `.felt/`; `id`
40+
is the store-relative slug. Returns `{:error, reason}` when the resolved path
41+
sits under no `.felt/` directory (`:no_felt_store`) or isn't a fiber-container
42+
`.md` file (`:not_markdown`, `:unexpected_layout`, `:empty_tail`).
43+
"""
44+
@spec ref_from_path(String.t()) :: {:ok, ref()} | {:error, term()}
45+
def ref_from_path(path) do
46+
resolved =
47+
case resolve_realpath(path) do
48+
{:ok, real} -> real
49+
{:error, _} -> Path.expand(path)
50+
end
51+
52+
segments = Path.split(resolved)
53+
54+
case felt_index(segments) do
55+
nil ->
56+
{:error, :no_felt_store}
57+
58+
felt_idx ->
59+
tail = Enum.drop(segments, felt_idx + 1)
60+
61+
with {:ok, fiber_id} <- id_from_tail(tail) do
62+
host =
63+
case Enum.take(segments, felt_idx) do
64+
[] -> "/"
65+
parts -> Path.join(parts)
66+
end
67+
68+
{:ok, %{host: host, id: fiber_id}}
69+
end
70+
end
71+
end
72+
73+
@doc """
74+
Canonical store-relative id for an on-disk fiber path — the `id` of
75+
`ref_from_path/1` without the host. Returns `{:error, reason}` on the same
76+
conditions.
77+
"""
78+
@spec canonical_id(String.t()) :: {:ok, String.t()} | {:error, term()}
79+
def canonical_id(path) do
80+
with {:ok, %{id: id}} <- ref_from_path(path), do: {:ok, id}
81+
end
82+
83+
@doc """
84+
Realpath of a felt store root (resolves symlinks; falls back to `Path.expand/1`).
85+
Used to compare a candidate fiber's owning host against a configured store.
86+
"""
87+
@spec canonical_host_path(String.t()) :: String.t()
88+
def canonical_host_path(host) do
89+
case resolve_realpath(host) do
90+
{:ok, resolved} -> resolved
91+
{:error, _} -> Path.expand(host)
92+
end
93+
end
94+
95+
# POSIX SYMLOOP_MAX is 8; Linux allows 40. We cap symlink *hops* rather than
96+
# forbidding revisits, because a non-cyclic resolution legitimately re-traverses
97+
# a shared prefix symlink (e.g. macOS `/tmp → /private/tmp`) on more than one
98+
# pass — a "never revisit" guard mis-flags that as a loop. A genuine A→B→A
99+
# cycle still blows past the cap.
100+
@max_symlink_hops 40
101+
102+
@doc """
103+
Pure-Elixir `realpath`: resolves every symlink along an expanded path,
104+
segment by segment, capping total symlink hops to guard against cyclic links.
105+
Returns the fully resolved absolute path.
106+
"""
107+
@spec resolve_realpath(String.t()) :: {:ok, String.t()} | {:error, term()}
108+
def resolve_realpath(path) do
109+
expanded = Path.expand(path)
110+
111+
case Path.split(expanded) do
112+
["/" | rest] -> resolve_realpath_segments("/", rest, 0)
113+
[first | rest] -> resolve_realpath_segments(first, rest, 0)
114+
[] -> {:error, :empty_path}
115+
end
116+
end
117+
118+
# The enclosing `.felt` segment. After realpath there is exactly one, so the
119+
# last-occurrence scan is unambiguous; it mirrors Portolan's `lastIndexOf`.
120+
defp felt_index(segments) do
121+
segments
122+
|> Enum.with_index()
123+
|> Enum.reduce(nil, fn
124+
{".felt", idx}, _acc -> idx
125+
_, acc -> acc
126+
end)
127+
end
128+
129+
defp id_from_tail([]), do: {:error, :empty_tail}
130+
131+
defp id_from_tail([file]) do
132+
if String.ends_with?(file, ".md") do
133+
{:ok, String.replace_suffix(file, ".md", "")}
134+
else
135+
{:error, :not_markdown}
136+
end
137+
end
138+
139+
defp id_from_tail(tail) do
140+
file = List.last(tail)
141+
parent = Enum.at(tail, -2)
142+
143+
cond do
144+
not String.ends_with?(file, ".md") ->
145+
{:error, :not_markdown}
146+
147+
String.replace_suffix(file, ".md", "") != parent ->
148+
{:error, :unexpected_layout}
149+
150+
true ->
151+
{:ok, tail |> Enum.take(length(tail) - 1) |> Path.join()}
152+
end
153+
end
154+
155+
defp resolve_realpath_segments(current, [], _hops), do: {:ok, current}
156+
157+
defp resolve_realpath_segments(_current, _segments, hops) when hops > @max_symlink_hops,
158+
do: {:error, :symlink_loop}
159+
160+
defp resolve_realpath_segments(current, [segment | rest], hops) do
161+
candidate = Path.join(current, segment)
162+
163+
case :file.read_link(String.to_charlist(candidate)) do
164+
{:ok, target} ->
165+
target_path = List.to_string(target)
166+
167+
expanded_target =
168+
case Path.type(target_path) do
169+
:absolute -> Path.expand(target_path)
170+
_ -> Path.expand(target_path, Path.dirname(candidate))
171+
end
172+
173+
case Path.split(expanded_target) do
174+
["/" | target_rest] ->
175+
resolve_realpath_segments("/", target_rest ++ rest, hops + 1)
176+
177+
[first | target_rest] ->
178+
resolve_realpath_segments(first, target_rest ++ rest, hops + 1)
179+
180+
[] ->
181+
{:error, :empty_target}
182+
end
183+
184+
{:error, _} ->
185+
resolve_realpath_segments(candidate, rest, hops)
186+
end
187+
end
188+
end

0 commit comments

Comments
 (0)