Skip to content

import <agent> hangs forever on a non-regular destination (all adapters) #242

Description

@spxrogers

Found during review of #240, which fixes the same defect class for status / diff / reconcile's drift walk. This one is in the adapter Ingest paths and is not fixed by that PR.

os.ReadFile on a FIFO does not fail — it blocks in the open — so the read's own error path never runs and the command never returns.

Measured

Built at d4fb233 (i.e. with #240's gate in place), clean apply, destination replaced by a 0600 FIFO, neutral cwd:

import claude              rc=124  *** HUNG ***
import claude --dry-run    rc=124  *** HUNG ***

Reproduced with a FIFO at a whole-file destination (~/.claude/skills/demo/SKILL.md) and at a key-merge one (~/.claude.json).

Note agentsync import with no agent is cobra.ExactArgs(1) and fails instantly with accepts 1 arg(s), received 0 — a selector is required to reach the bug, and forgetting one is how an earlier test of this silently measured nothing.

doctor is exposed through the same path

doctor performs no destination read of its own, but its plugin check reaches one: checkPluginsundeclaredNativePlugins (internal/cli/plugin_drift.go) → claude.IngestPlugins → a bare os.ReadFile(p.Settings) at internal/adapter/claude/ingest_plugins.go:35. settings.json is a managed destination (claude renders /hooks and /lspServers into it).

A reviewer measured doctor hanging at rc=124 with a FIFO there, wedged mid-"Plugins". A second fixture exited rc=1 instead, because an unrelated issue was reported before the plugin check ran — so reachability depends on what else doctor finds first, but the unguarded read is there by inspection and doctor should be treated as in scope for this issue rather than immune.

Where

Two distinct read paths, both unguarded:

  1. internal/adapter/readoptional.go:25ReadFileOptional's os.ReadFile, reached from claude.(*Adapter).Ingestcli.importRun. 27 call sites across the adapters.
  2. 31 direct os.ReadFile calls in the adapter packages that do not go through ReadFileOptional — e.g. internal/adapter/claude/ingest.go:82, ingest_plugins.go:35, and the equivalents in codex, cursor, gemini, generic, windsurf, roo, cline, continuedev.

So this is a sweep, not a one-liner — which is why it was split out of #240 rather than folded in.

Why ReadFileOptional is the natural home for most of it

Its existing contract already distinguishes the three cases:

  • (data, true, nil) when the file exists and is readable;
  • (nil, false, nil) when the file is absent — the only legitimate "component not present" case;
  • (nil, false, err) for any OTHER read error (permission, EISDIR from a directory sitting where a file is expected, transient I/O), which callers MUST surface.

A FIFO is squarely the third case: present, but not readable as a file. It should return an error there, exactly as a directory already does via EISDIR. Adding a shape check inside ReadFileOptional extends an existing, documented contract rather than inventing a new one, and covers 27 sites at once. The 31 direct reads then need routing through it (or through an equivalent gate) individually.

Test note

internal/cli/dest_fifo_e2e_unix_test.go carries a ready-made import claude row, currently t.Skipped citing this issue — deleting the skip: string inherits the assertion. It was verified to fail with the correct BLOCKED diagnostic on both destination shapes when un-skipped.

Any regression test must be timeout-bounded and must pass an agent selector. This class does not fail, it hangs.

One trap for whoever does this work: an abandoned goroutine parked in open(2) never reaches its defer, so heldLocks[home] stays above zero and later in-process rows silently skip the flock (internal/cli/lock.go). That is harmless while the rows are skipped, and becomes live the moment they are not.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions