Skip to content

dl looks for devpod's ssh config where devpod writes it #421

Closed

Description

@blooop

Question

dl looks for devpod's ssh config at a hardcoded ~/.ssh/config, which devpod
never creates when DEVPOD_SSH_CONFIG is set. This is a shipped bug, not just
a prerequisite
: on any host that sets that variable, dl silently loses the pty
transport and falls back, with nothing said.

Found twice independently, by
#389 and
#390; read #390's resolution
comment for the shape it wants, since the next slice builds on this one.

The change, in rust/devlaunch-core/src/clients/ssh.rs:

  1. config_path() honours $DEVPOD_SSH_CONFIG before ~/.ssh/config.
  2. Context-option paths come only from the already-cached context options —
    never a new devpod context options round trip. The cache exists (keyed on
    devpod's own config file changing); a new trip here would spend more than the
    whole slice saves.
  3. Terminal::NoAlias splits, so "no config anywhere" is a legible state
    rather than indistinguishable from "config present, alias absent". Silence is
    what let this ship.

#390 reports a red test already exists for this. Find it, confirm it is red for
the right reason, and make it green.

Notes for the builder

  • Cut from origin/main; use your own worktree. Other agents are active.
  • Guards in rust/: cargo test --workspace, cargo clippy --locked --all-targets -- -D warnings,
    cargo fmt --check.
  • Known pre-existing flake, do not chase: aid's
    a_pasted_multi_line_prompt_arrives_whole_rather_than_leaking under full-workspace load.
  • Note for anyone measuring after this lands: scratch-scoped benchmark runs that
    set DEVPOD_SSH_CONFIG have been silently taking the non-pty path, so
    before/after numbers taken that way are not comparable across this change.

Activity

  1. self-assigned this
    on Aug 24, 2026
  2. blooop commented on Aug 24, 2026

    @blooop
    OwnerAuthor

    breadcrumb: built as #433 on fix/ssh-config-path, cut from origin/main at 57955a3.

    No red test pre-existed. #421 says #390 reports one; #390's "How it is tested" section proposes it ("red on merged main today") and nothing was ever written, on main or on research/devpod-ssh-trip. So I proved the red by hand on origin/main with a throwaway test first, since the fixed signature does not exist there:

    assertion `left == right` failed: devpod wrote the alias to
    "/tmp/.tmp3SBHzt/ssh_config"; dl looked in Some("/tmp/.tmp3SBHzt/home/.ssh/config")
    

    then wrote the permanent ones against the new shape.

    All three parts landed. ssh::config_path takes a ConfigSources and walks devpod's own order (SSH_CONFIG_INCLUDE_PATH context option, then $DEVPOD_SSH_CONFIG, then the SSH_CONFIG_PATH context option, then ~/.ssh/config), expanding a leading ~/ the way ResolveSSHConfigPath does and reading an empty value as unset. The order is read out of skevetter/devpod v0.26.1 rather than assumed: ConfigureSSHConfig prefers the include path outright, and cmd/root.go::inheritFlagsFromEnvironment is why devpod reads a variable it never names, since it gives every flag a DEVPOD_<FLAG> default and up has --ssh-config.

    The no-round-trip constraint is held by the signature, not by a test. The context-option paths come from a new already_cached_options, which reads the disk cache and nothing else; neither it nor terminal_for takes a Runner, so neither has anything to ask devpod with. A cache miss answers with devpod's defaults, which is what dl assumed unconditionally before, so a miss is never worse than the bug.

    The split is three arms, not two. NoAlias { config } (read the config, this workspace absent), ConfigMissing { looked_in } (nothing readable where devpod publishes), ConfigUnlocatable (no home directory and nothing naming a config, which XDG_CACHE_HOME makes reachable). devpod_host_configured's bool became ssh::Alias, so the collapse cannot come back by accident. Each has its own notice and each names the path dl read.

    And the e2e harness stops working around it. route_ssh_through symlinked a scratch ~/.ssh/config onto the run's config precisely so dl's hardcoded lookup would find it. That symlink is deleted: the scratch home now holds no ssh config at all, so test/e2e/test_interactive_session.py passing is what says the resolution order holds. OpenSSH still needs its -F shim, reading neither $HOME nor any devpod variable.

    Guards green in the devcontainer: cargo test --workspace, clippy --locked --all-targets -- -D warnings, fmt --check, and pytest test/ --ignore=test/e2e (411). e2e not run (no docker/devpod here). public-api.rest.txt updated for the three notice arms; nothing enters the promised api tier.

    The flake behaved as the ticket said. aid's a_pasted_multi_line_prompt_arrives_whole_rather_than_leaking failed once under full-workspace load here, and reproduces on origin/main with this branch stashed: 1 failure in 8 runs under CPU load, 0 in 3 unloaded full-workspace runs. It touches no file this branch changes.

    Recorded for whoever measures next: the note in the ticket is right and worth repeating on the PR, so it is in the body. Every scratch-scoped run sets DEVPOD_SSH_CONFIG, so all of them have silently been on the non-pty transport, and a before/after that straddles this commit is comparing two transports rather than two builds.

  3. blooop commented on Aug 24, 2026

    @blooop
    OwnerAuthor

    breadcrumb: correction to this ticket's own text, on the record. It said "#390 reports a red test already exists for this" — it does not. #390's "How it is tested" section proposed one; nothing existed on main or on research/devpod-ssh-trip. That was the manager transcribing a proposal as an artefact when it wrote this ticket, which is the same class of error the review of PR #403 caught twice in docs/performance.md. The build proved the red by hand on origin/main before writing the permanent tests, and said so rather than quietly satisfying the instruction.

  4. blooop commented on Aug 24, 2026

    @blooop
    OwnerAuthor

    breadcrumb: stage — in review, all checks green at 9c8dc90, awaiting a human merge. PR #433.

    One review round and one fix round, and the review caught a defect that would have made this worse than the bug it fixes: the PR made dl decide which ssh config to read while leaving ssh::command_args emitting no -F, so OpenSSH still resolved the alias through getpwuid's ~/.ssh/config. On exactly the hosts this ticket targets, dl <ws> -- <cmd> at a terminal would have gone from works without a pty to exit 255, could not resolve hostname. Reproduced twice: by hand on OpenSSH_9.6p1, and then through dl against a real container with the e2e shim deleted, where 6 of 7 terminal tests failed.

    The fix is the modelling one rather than the symptom: every arm of Terminal now carries a path, including the one that works (Usable { config }), Route::Terminal carries it beside the payload, and command_args takes it as its first parameter, so an invocation with no -F is not expressible and nothing re-derives the path downstream of the decision. -F is unconditional by argument, not by omission: $HOME and getpwuid's home can differ, so a conditional -F would be a guess.

    The e2e shim is deleted. It prepended -F to every invocation, which is the only reason the pty assertions ever passed against a dl emitting none. A harness must not supply the fix under test. What remains is a scratch HOME with no .ssh/config, so these tests reaching a container is itself the assertion, and a new e2e test proves the alias resolves with no shim at all.

    Merging stays human. This ticket stays open until the PR lands.

  5. blooop commented on Aug 28, 2026

    @blooop
    OwnerAuthor

    Shipped in PR #433 (merged 2026-08-25): ssh::config_path walks devpod's four-candidate order via ConfigSources, the context-option paths come from the cache-only already_cached_options, and NoAlias split into NoAlias / ConfigMissing / ConfigUnlocatable. The review round also made -F unconditional via Terminal::Usable { config }.

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

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions