Skip to content

Add opt-in client/server mode to the rdx executable - #869

Open
paracycle wants to merge 11 commits into
uk_cli_refactorfrom
uk-add-client-server-for-cli
Open

Add opt-in client/server mode to the rdx executable#869
paracycle wants to merge 11 commits into
uk_cli_refactorfrom
uk-add-client-server-for-cli

Conversation

@paracycle

@paracycle paracycle commented Jun 18, 2026

Copy link
Copy Markdown
Member

Goal

rdx query indexes and resolves the workspace on every call. This PR adds an optional resident server per workspace. The server holds an indexed and resolved Rubydex::Graph in memory. Repeated calls of rdx query --server then skip that cost.

Without --server the command runs inline, as before. The command also runs inline on a platform without fork or UNIX sockets.

This PR builds on #971. Please review and merge #971 first.

This PR is the first step of a larger change. It adds the runtime, the IPC, and a server for the read-only query path. It contains only Ruby code. It changes no Rust or C code.

CLI surface

  • rdx query <CYPHER> [--server] [--format ...] sends the query to the resident server.
  • rdx server <start|stop|restart|status> [--no-detach] manages the server for the workspace.
  • DISABLE_RDX_SERVER forces inline execution.
  • RDX_SERVER_DIR overrides the runtime directory.

CLI changes

#971 gives each subcommand its own file. This PR follows that structure:

  • lib/rubydex/cli/command/server.rb adds Rubydex::CLI::Command::Server. The class declares command "server", so the command appears in rdx help automatically.
  • lib/rubydex/cli/command/query.rb gains the --server option. Two private methods separate the server path from the inline path.
  • Command#parse_options! accepts an optional banner. The server command uses it to list its actions for rdx server --help.

Server design

  • lib/rubydex/server/cache.rb holds a runtime directory per workspace. An app id keys the directory. The id hashes the workspace path, the Ruby version, the rubydex version, and a fingerprint of the native extension. A gem upgrade therefore forces a new server. The class manages the pid, token, socket, and version files with 0700 and 0600 permissions.
  • lib/rubydex/server/request.rb frames JSON messages with a length prefix. It applies an IO.select timeout to the handshake read.
  • lib/rubydex/server/core.rb accepts connections on a UNIX socket. It checks the version line, compares the token in constant time, and dispatches query, status, and stop under a mutex. It also checks an mtime manifest on each request. It reindexes the changed files, deletes the removed files, and resolves the graph.
  • lib/rubydex/server/client.rb starts the server if it is absent. It uses fork and Process.daemon, an flock for a single start, and a poll for readiness. It also handles the version handshake, the cold and warm recovery, and a graceful stop with a SIGTERM fallback.
  • lib/rubydex/server/commands.rb implements the four actions of rdx server.

The socket binds and connects through a relative name from its directory. Long runtime paths therefore stay below the sockaddr_un limit of about 104 bytes.

Next steps

This PR keeps a small scope. Later PRs can add these features:

  • A warm rdx console session from the resident graph. An IRB session needs the terminal of the client. This requires a worker process per session, the transfer of the client file descriptors, and signal forwarding.
  • A background file watcher for a fresh graph. A watcher thread applies incremental updates and replaces the staleness check of this PR. The server can then also stop itself after an idle timeout.

Tests

  • test/server/cache_test.rb covers the app id, the file permissions, the version compare, and the cleanup of stale files.
  • test/server/request_test.rb covers the message framing.
  • test/server/integration_test.rb spawns a real server subprocess. It covers a warm query, staleness after an add and a delete, equality with inline output, start, status, stop, and restart. The POSIX assertions skip on Windows.
  • test/cli_test.rb now also covers the server command for the help output and for a bad option.

@paracycle
paracycle requested a review from a team as a code owner June 18, 2026 23:20
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch 2 times, most recently from 6967765 to 3bf6c65 Compare June 18, 2026 23:37
@paracycle
paracycle force-pushed the uk_add_cypher_query_engine branch 2 times, most recently from 49ed15d to 13f37ec Compare June 19, 2026 00:22
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch 2 times, most recently from 59e9e4b to 7a54996 Compare June 19, 2026 00:30
@paracycle
paracycle force-pushed the uk_add_cypher_query_engine branch 2 times, most recently from 2e6a202 to bc2a231 Compare June 23, 2026 21:16
@paracycle
paracycle force-pushed the uk_add_cypher_query_engine branch 3 times, most recently from 85bbbba to cc553f6 Compare July 3, 2026 21:56
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch 4 times, most recently from f282639 to 173d2e6 Compare July 8, 2026 16:28
@paracycle
paracycle changed the base branch from uk_add_cypher_query_engine to uk_query_object_results July 8, 2026 16:28
@paracycle
paracycle force-pushed the uk_query_object_results branch from cc69ca7 to d68b816 Compare July 8, 2026 16:40
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 173d2e6 to 5cb5692 Compare July 8, 2026 16:42
@paracycle
paracycle force-pushed the uk_query_object_results branch from d68b816 to 51b4f05 Compare July 8, 2026 18:26
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 5cb5692 to d319c14 Compare July 8, 2026 18:31
@paracycle
paracycle force-pushed the uk_query_object_results branch from 51b4f05 to 2bb59e0 Compare July 8, 2026 20:05
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from d319c14 to 632c830 Compare July 8, 2026 20:07
@paracycle
paracycle force-pushed the uk_query_object_results branch from 2bb59e0 to 1de091c Compare July 8, 2026 20:19
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 632c830 to 519a264 Compare July 8, 2026 20:22
@paracycle
paracycle force-pushed the uk_query_object_results branch from 1de091c to ae8fcbd Compare July 9, 2026 20:13
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 519a264 to fbb1b8b Compare July 9, 2026 20:17
@paracycle
paracycle force-pushed the uk_query_object_results branch from ae8fcbd to 9e80825 Compare July 9, 2026 20:56
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from fbb1b8b to 861101d Compare July 9, 2026 21:02
@paracycle
paracycle force-pushed the uk_query_object_results branch from 9e80825 to 5fb40e7 Compare July 9, 2026 21:17
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 861101d to 6d7bd1f Compare July 9, 2026 21:18
@paracycle
paracycle force-pushed the uk_query_object_results branch from 5fb40e7 to 933bf3d Compare July 9, 2026 22:39
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 6d7bd1f to 7d42698 Compare July 9, 2026 22:40
@paracycle
paracycle force-pushed the uk_query_object_results branch from 933bf3d to a22a461 Compare July 16, 2026 20:18
@paracycle
paracycle force-pushed the uk_query_object_results branch 3 times, most recently from bdddecc to 6865c54 Compare July 29, 2026 17:24
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 7d42698 to 2d397bd Compare July 29, 2026 17:30
Base automatically changed from uk_query_object_results to main July 29, 2026 18:24
Comment thread lib/rubydex/cli.rb
@@ -0,0 +1,255 @@
# frozen_string_literal: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to split the cli right now? I think if we were to split it I'd try to have each command in it's own file.

Comment thread lib/rubydex/server.rb Outdated
# keeps the built `Rubydex::Graph` in memory. Subsequent commands (currently `--query`) run against
# the already-built graph over a UNIX domain socket, making follow-up queries effectively instant.
#
# See `tmp/rdx-server-plan/README.md` for the full design.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Comment thread lib/rubydex/server.rb Outdated
PROTOCOL = 1

# How long the client waits for the server's version line / handshake before giving up.
HANDSHAKE_TIMEOUT = 10.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in Client?

Comment thread lib/rubydex/server.rb Outdated
HANDSHAKE_TIMEOUT = 10.0

# How long the client waits for a freshly spawned server to become ready (socket to appear).
BOOT_TIMEOUT = 120.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in Client?

Comment thread lib/rubydex/server.rb Outdated
class Error < StandardError; end

# Raised when a read from the server exceeds its timeout.
class ServerReadTimeout < Error; end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Client?

Comment thread lib/rubydex/server/commands.rb Outdated
return 0
end

Client.request(cache, { "command" => "status" }, stdout: stdout, stderr: stderr)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't that start a server? So checking the status always starts a server?

Comment thread lib/rubydex/server/commands.rb Outdated

#: (Cache cache, ?stdout: IO, ?stderr: IO) -> Integer
def status(cache, stdout: $stdout, stderr: $stderr)
unless cache.server_alive? && File.socket?(cache.socket_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also check version compatibility?

end

Client.ensure_server(cache, detach: detach)
# With --no-detach the server runs in the foreground and only returns here once stopped.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means we log that the server started before it's true?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need to log again? It's done in core already?

def status(cache, stdout: $stdout, stderr: $stderr)
unless cache.server_alive? && File.socket?(cache.socket_path)
stdout.puts("rdx server: not running for #{cache.workspace_path}")
return 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we return something else than 0 here?

Comment thread lib/rubydex/server/commands.rb Outdated
Comment on lines +5 to +6
# Implementations for the server-control CLI flags (`--start-server`, `--stop-server`,
# `--restart-server`, `--server-status`). Each returns a process exit status.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You eman rdx server start|stop|restart|status?

Introduce a resident, per-workspace server that keeps an indexed +
resolved Rubydex::Graph in memory so repeated `--query` invocations skip
the index/resolve cost. Server mode is opt-in via `--server`; without it
(or on platforms lacking fork/UNIX sockets) `rdx` runs inline as before.

- lib/rubydex/server/cache.rb: per-workspace runtime dir keyed by an
  app-id hashed from the workspace, Ruby/rubydex version and a native-ext
  fingerprint; pid/token/socket/version files with restrictive perms.
- lib/rubydex/server/request.rb: length-prefixed JSON framing plus an
  IO.select handshake read timeout.
- lib/rubydex/server/core.rb: UNIX socket accept loop with a version-line
  gate, constant-time token auth, mutex-guarded dispatch and per-request
  mtime-manifest staleness (reindex changed / delete removed / resolve).
- lib/rubydex/server/client.rb: start-if-absent (fork + Process.daemon,
  flock single-start, spawn-and-poll readiness), version handshake,
  cold/warm recovery, graceful stop with SIGTERM fallback.
- lib/rubydex/server/commands.rb: --start/stop/restart/server-status.
- exe/rdx: new --[no-]server, --start/stop/restart-server,
  --server-status and --no-detach flags; queries route to the server when
  opted in, everything else stays inline.

Binds/connects the socket via a relative name from its directory so long
runtime paths stay under the sockaddr_un limit. Adds unit tests for the
cache and framing plus integration tests that spawn a real server.
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from b6059d2 to ff7506e Compare July 31, 2026 20:09
@paracycle
paracycle changed the base branch from main to uk_cli_refactor July 31, 2026 20:09
paracycle added 10 commits July 31, 2026 23:54
@Morriar asked for several small changes in the server code. This commit
applies them. It changes no behaviour of the server protocol.

- `Server::Cache` becomes `Server::State`. The class owns the runtime
  state of a server, and not a cache.
- The app id now includes `PROTOCOL`. A protocol change therefore maps to
  a new runtime directory. The constant had no use before.
- `HANDSHAKE_TIMEOUT` and `BOOT_TIMEOUT` move from `Server` into
  `Client`. Only the client reads them.
- `ServerReadTimeout` becomes `Request::ReadTimeout`. `Request` raises it,
  so the error belongs there and not in `Client`.
- `Rubydex::Progress.with_timer` replaces the two copies of the timer in
  `Server` and in `CLI::Command`.
- `Client.monotonic` goes away. The four call sites now use
  `Process.clock_gettime` directly, like the rest of the code.
- The comment of `Commands` named the old `--start-server` flags. It now
  names the `rdx server` actions.
- The comment of `Client` said that the server saves time. The client
  saves the time.
- The comment of `Server` pointed to `tmp/rdx-server-plan/README.md`.

`Server.disabled?` stays. It implements `DISABLE_RDX_SERVER`, which the
pull request documents and the query command uses.
The app id keys the runtime directory of a server. It includes
`PROTOCOL`, so a protocol change maps a workspace to a new directory and
a new client never reaches a server with an older wire format.

The existing tests cover the workspace path only, so this rule could
regress without a failure.
@Morriar found several ways for a peer to block the server or to pass a
bad frame. This commit closes them.

Reads now have a deadline and a size limit:

- Every read takes a deadline. A peer that stops in the middle of a frame
  raises `Request::ReadTimeout`. Before, `socket.read(length)` blocked
  forever when the length promised more bytes than the peer sent.
- `MAX_PAYLOAD_BYTES` bounds one payload, and `MAX_LINE_BYTES` bounds one
  line. Before, the reader accepted any length.
- A length line must hold only decimal digits. `String#to_i` accepted
  `"100garbage"` and returned `100`.
- The reader rejects a payload that is not a JSON object.
- The reader rejects a truncated frame. A close after a partial read
  raises `Request::MalformedFrame`. Before, the reader returned the
  partial body, so a peer could declare 100 bytes, send `{}`, close, and
  the caller accepted that valid JSON.

`read` and `read_line` now share one byte loop, so the two public methods
no longer duplicate the read logic.

`Core#handle` rescues `MalformedFrame`, `ReadTimeout` and
`JSON::ParserError`. It logs the error, answers with status 1 and keeps
the accept loop alive. Before, a `JSON::ParserError` left the loop.

`Client#request` treats a response without an integer status as an error.
Before, `response["status"] || 0` reported success for a broken response.
The previous commit gave every read one deadline for the whole frame. That
deadline broke the client, because the server sends nothing until the
query finishes. A query longer than 10 seconds failed, and so did a large
result over a slow connection.

An inactivity deadline alone is wrong for the server. `Core#serve` handles
one client at a time, so a client that sends one byte every 9 seconds
would hold the accept loop and lock out every other client.

The two directions therefore have separate readers:

- `read_request` bounds the whole frame with an absolute deadline. The
  server uses it, and a client that drips bytes now loses the connection
  after 10 seconds.
- `read_response` has no total deadline. It waits for the first byte
  without a limit, then bounds the gap between two chunks. A slow but
  steady transfer succeeds, and a server that dies mid-answer still
  releases the client.

`read_response` also accepts an optional `total_timeout`. The stop path
passes it, because a stop needs no work from the server.

The two directions carry different size caps. `MAX_REQUEST_BYTES` is 1 MiB
and bounds untrusted input. `MAX_RESPONSE_BYTES` is 512 MiB, because a
query result is far larger than the query that asked for it.

The module now raises only `ReadTimeout` and `MalformedFrame`. A body that
is not JSON becomes a `MalformedFrame`, so no caller rescues
`JSON::ParserError`. The message keeps only the first 120 characters of
the parser's complaint, which quotes the payload.

`Client#request` rescues both errors and closes the socket in an `ensure`
block. A server that dies in the middle of its answer now reports:

    rdx server: the peer closed the connection after 15 of 500 bytes

A server that answers with invalid JSON reports:

    rdx server: the payload is not valid JSON: unexpected token 'not' ...

Before, both raised a backtrace at the user.

Known limit: one bad client can still stall the others for the length of
the request deadline, because the server is single-threaded. A concurrent
accept loop is a separate change.
`IO#read_nonblock` allocates its `maxlen` before it reads any byte. The
reader passed the remaining frame length straight through, so a peer that
declared 300 MiB and sent one byte cost the reader 300 MiB at once. I
measured the resident size in both states:

- before: RSS 19 MiB -> 319 MiB for one received byte
- after: no change, and the read ends at its deadline

Each read now asks for `CHUNK_BYTES`, which is 64 KiB, or for the rest of
the frame when that is smaller. The frame cap still bounds the buffer that
the reads fill, so the size limits do not change. A large transfer is
unaffected: a 40 MiB response round trips in 0.13 seconds.

A new test records the `maxlen` of every read and asserts that none
exceeds one chunk. Against the old code it reports:

    Expected 196621 to be <= 65536.
@Morriar found ten problems in the server lifecycle. A process id read from
a file drove all of them: the client signalled it, the liveness check
trusted it, and a cleanup deleted files under a live server.

The lock now answers the question "does a server own this workspace". The
kernel releases a lock when the holder dies, so a recycled process id can
never look alive. Spring uses the same rule for the same reason.

The runtime directory holds three files:

- `lock` is empty, and no code reads or writes it. The server holds an
  exclusive lock on it for its whole life. The file is never deleted,
  because a lock belongs to an inode: unlinking the path would let a
  second process create a new file, lock that, and start a second server.
- `state.json` replaces the `pid`, `token` and `version` files. It holds
  the pid, the token, the version and the start time, and a rename
  replaces it in one step, so no reader sees a torn record.
- `socket` is unchanged.

The two files stay separate on purpose. On Windows `flock` becomes
`LockFileEx` over the whole file, and an exclusive lock there denies every
other process read access to that range. A record inside the locked file
would be unreadable exactly while a server runs. A test asserts the split
and that the lock file stays empty.

Stop, status and cleanup follow from the lock:

- The client sends no signal. An authenticated `stop` request over the
  socket is the only stop, and the client waits for the lock to go free.
- `clean!` takes the lock before it deletes the socket, so a live or a
  starting server never loses it.
- `status` uses a new `Client.probe`, which connects, hands over one
  budget and never starts, stops or restarts anything. The command that
  diagnoses a wedged server must not change it.
- A wedged server is reported, and never waited on. `status`, `stop` and
  `restart` all print the recorded pid and start time and exit 1.

The recorded pid is a display value for a human. The lock proves a live
owner; the pid says which process wrote the record at the last boot. The
CLI never signals it.

`Rubydex::Server::Error` now reaches the user as one sentence instead of a
backtrace. The rescue tests the constant with `defined?`, because
`rubydex/server` is loaded only when a command needs it.

Two problems that the live tests caught, both mine:

- A boot crash waited out the whole 120 second timeout. The lock leaves no
  trace, so the old evidence that a boot had started was gone. The server
  now writes its record before the slow index, and the client compares it
  with the record it saw before the spawn. A crash fails in one second.
- A second client that arrived while the first server was still indexing
  got an error instead of waiting. A held lock means "starting or
  running", so `start` now waits for that server rather than forking one
  that could not take the lock. `test/server/client_test.rb` covers both.
The previous commit made `rdx server status` report a wedged server
instead of hanging on it. Only a temporary patch proved that, and no test
referenced `Client.probe` or `Commands.status`. A later change could route
status back through the mutating request path, and every test would still
pass.

`test/server/client_test.rb` now covers the two shapes a wedge takes. A
server is single-threaded, so one stuck query keeps it inside `handle`:

- It never accepts again, so a new client waits in the listen backlog and
  never receives the version line. The handshake budget ends that wait.
- Or it accepts and sends its version line, and then answers nothing. Only
  the response budget ends that one.

Each test asserts three things: the exit status is 1, the report carries
the recorded pid and start time, and the command started, stopped or
restarted nothing. The last point matters most. Diagnosing a wedged server
must not change it, because a new server cannot take the lock from a live
one, so a repair attempt only burns another timeout and loses the report.

`Commands.status` takes a `timeout` keyword, so a test can pin the wedged
path in a third of a second instead of ten. Both assertions run inside
`Timeout.timeout`, so a lost budget fails the test instead of hanging CI.

Each test was checked against the change it guards:

- Response budget removed -> `Timeout::Error: execution expired`
- Handshake budget removed -> "status must not wait beyond its budget"
- Status routed through `Client.request` -> "the rdx server ... is not
  answering", raised where the report belongs

The stand-in server now names its modes: `bind`, `bind_after`, `accept`,
`greet` and `die_after`. The old `ready_after: nil` meant "never binds",
which reads like "ready at once". 30 runs across 30 seeds are stable.
@Morriar found five problems in how the server handles failure. Behind all
of them is one rule: a resident server belongs to every client, so one bad
request, one unreadable file or one bug must not take it away from the
rest.

`handle` now rescues `StandardError` per connection. It logs the class,
the message and the backtrace, answers with status 1, and the accept loop
lives on. Before, an unexpected error reached `run`, which re-raised and
stopped the server.

`handle_query` checks the shape of the two fields a caller controls. JSON
can carry a number, an array or an object in either of them, and the
extension answers that with a `TypeError` that would be logged as a fault
of this server. Only an absent `query_format` means the default, because
`|| "table"` also swallowed a `false`.

The rescue around the query no longer covers the refresh. An
`ArgumentError` from a refresh is a server fault, and reporting it as a
bad query blamed the caller for it.

`refresh_if_stale` acts on the errors that `index_all` returns. A file
that failed keeps its previous mtime, so the next request retries it.
Recording the new one called a file the server never read "fresh" for the
rest of its life.

The same bug existed at boot, one layer down. `build_graph` discarded the
result of `index_workspace`, so `Core#run` marked every file fresh. It now
returns the errors, and it drops the roots that do not exist before
indexing: `workspace_paths` names gem directories that an install may not
have, and eight phantom errors would otherwise hide the one real failure.

Attribution is the hard part, because the indexer reports opaque messages
for a whole batch. The graph cannot answer it either: a failed update
leaves the previous document in place, and mapping a path to a document
URI is the parity question that Group D still owns. So a failing batch is
halved until each failure sits alone. A batch indexes far faster per file
than single calls do, so this beats asking file by file. Measured on 1081
files with one unreadable among them:

- splitting:  21 calls in 75ms
- one by one: 1081 calls in 311ms

`collect_files` rescues per entry, so a file that vanished mid-walk no
longer ends the walk of its directory. `EACCES` on a directory records it
as unreadable and its files keep their entries; without that, one `chmod`
made a whole subtree look deleted and erased it from the graph. `ENOENT`
still means the directory is gone. The walk still streams with
`each_child`, because materialising a directory listing is not affordable
on a hyper-scale workspace.

On the last comment, the early return after the unauthorized answer: it
was necessary, since without it the request would still be dispatched. The
branch is now an `if`/`else`, so the structure says that rather than
relying on the reader to notice the `return`.

`test/server/core_test.rb` covers each of these. Every test was checked
against the change it guards.
The manifest walk decides what the server re-indexes, so it has to see the
workspace the way the indexer does. It did not, and each difference cost
something.

`File.directory?` follows symlinks. The Rust walker asks a `DirEntry` for
its type, which does not, and `collect_files_does_not_follow_symlinked_
directories` pins that. So a link that points at an ancestor made the walk
descend into the same subtree again and again. One `ln -s .. sub/loop`
recorded a single file 32 times, under paths up to 301 characters, until
the platform refused. Every one of those phantom entries was missing from
the graph, so each would look changed on every request for the life of the
server.

The rule differs by depth, and both halves matter:

- `Graph#workspace_paths` uses `File.directory?` on the workspace's own
  children, so a symlinked directory there becomes an explicit root, and
  the Rust walker does traverse an explicit root. The walk follows it too,
  or those files would sit in the graph and never be refreshed.
- Below that, a symlink is never followed.

A symlink to a file is still recorded at its own path, which is what
`collect_files_indexes_symlinked_files_at_their_own_path` describes.

A self-referential symlink is a third case. `lstat` calls it a plain file,
and `File.mtime` then follows it and raises `ELOOP`, which no rescue
caught. One such file made every query fail.

The per-entry rescue therefore names the errors that describe one path:
`ENOENT`, `EACCES`, `ELOOP`, `ENAMETOOLONG` and `ENOTDIR`. A resource
failure such as `EMFILE` or `EIO` is deliberately absent. Swallowing one of
those would make the walk skip every entry, and the refresh would read the
empty result as "every file was deleted" and erase the graph. Those travel
on instead, and the request fails without the manifest moving.

The list resolves through `Errno.const_defined?`, because the constants a
build defines are platform-dependent and this file still loads on Windows.
A test pins the full resolution on POSIX, so a misspelled name cannot hide
behind the lookup, and a second test pins the two names that exist
everywhere.
POSIX separates reading a directory from searching it. At mode `0400`
`Dir.each_child` lists the names and every `lstat` is refused, so the walk
recorded nothing under that directory while its outer rescue never fired.
The refresh then read the gap as "every file below here was deleted" and
removed the subtree from the graph.

This is the same failure the previous commit fixed for a directory that
cannot be read at all, reached through a different door.

A per-entry `EACCES` now records that path. The entry is still there, and
it may be a file or a whole subtree, so the caller keeps whatever it
already knew rather than treating the silence as a deletion. The other
path errors keep their old meaning: the entry really is unusable, and it
leaves the manifest.

`under_any?` matches a path that equals a recorded prefix as well as one
beneath it, because the thing that could not be read may be a single file
and not only a directory above it. Both halves are load-bearing, and
removing either one fails the new test.
@paracycle
paracycle force-pushed the uk-add-client-server-for-cli branch from 679504a to b8f407e Compare July 31, 2026 23:13
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.

2 participants