Repository navigation
Conversation
Integration test reportCommit: 3f03a6d
Top 6 slowest tests (at least 2 minutes):
|
rugpanov
left a comment
There was a problem hiding this comment.
[nice to have] The Tests section lists only unit tests. These tests do not show that sshd SetEnv puts ~/.agent-shim/bin on PATH on real serverless compute. They also do not show that os.Executable() returns the uploaded binary path there. Add the result of a manual run on serverless with a freshly built binary, for both -- 'claude -p "..."' and a terminal in --ide. As an alternative, add a command -v claude check to the serverless acceptance test that runs with CLOUD_ENV.
| // put the Databricks context where the agent reads it, returning any extra argv. | ||
| func injectAgentContext(ctx context.Context, home string, agent agentSpec) ([]string, error) { | ||
| func injectAgentContext(ctx context.Context, home string, agent agentlauncher.AgentSpec) ([]string, error) { | ||
| systemContext := agentSystemContext(env.Get(ctx, workspaceHomeEnv)) |
There was a problem hiding this comment.
[blocking] agentSystemContext (line 815) always tells the agent Your working directory is <wsHome>. Only the interactive shell changes into that directory. For databricks ssh connect -- claude -p ..., sshd starts the command in the OS home directory, which does not persist. The same text tells the agent that only /Workspace and /Volumes persist. So the agent can write files with relative paths, think that they persist, and lose them when the compute stops. Use os.Getwd() for the working directory, and name DATABRICKS_WORKSPACE_HOME as the place where files persist.
| @@ -0,0 +1 @@ | |||
| * `databricks ssh connect` on serverless now makes `claude` and `codex` available in IDE sessions (`--ide`) and to non-interactive commands (e.g. `databricks ssh connect --name my-conn -- claude -p "..."`). ([#7123](https://github.com/databricks/cli/pull/7123)) | |||
There was a problem hiding this comment.
[nice to have] ssh joins the remote command arguments with spaces, and the remote shell then splits them again. So -- claude -p "summarize the table" sends three separate arguments to claude, not one prompt. The remote shell also expands characters such as ? or $ in the prompt. Quote the full remote command in the example, for example -- 'claude -p "..."', as acceptance/ssh/connect-serverless-cpu/script does.
|
Commit: Regression test reportSelected 1 changed tests (limit=20, 0 not selected) 1 tests: regression=1 unreleased=0 coverage=0 failing=0
TestAccept/ssh/agent-launchers: branch ✅ | main (270b2d0) ❌ | latest (v1.22.1) ❌main (270b2d0): latest (v1.22.1): same output as main (270b2d0) |
anton-107
left a comment
There was a problem hiding this comment.
Leaving one finding inline. I consider it a blocker for merging this PR.
| // involved. Appended (not prepended) so the launchers never shadow a real | ||
| // binary of the same name earlier on PATH. | ||
| if opts.Serverless && parts[0] == "PATH" { | ||
| value += ":" + path.Join(homeDir, agentlauncher.BinDir) |
There was a problem hiding this comment.
[blocking] Preserve the launcher directory through IDE login-shell initialization.
SetEnv adds the directory to the initial SSH environment, but Cursor resolves the remote shell environment with -i -l -c and merges the resulting PATH into the environment used by extension hosts and terminals that inherit the shell environment. On serverless, /etc/profile rebuilds PATH from scratch—the existing comment in client.go:955–961 documents this exact behavior. The seeded .bashrc restores only the Python directory, so the agent launcher directory is lost. Adding it only here therefore leaves the advertised --ide support incomplete.
To reproduce on fresh serverless compute, compare:
databricks ssh connect --name pr7123 -- 'command -v claude codex'
databricks ssh connect --name pr7123 -- 'bash -ilc "command -v claude codex"'The direct command should find both launchers; the login-shell invocation should lose them. Also connect with --ide=cursor and check command -v claude codex in a new terminal. Please ensure the launcher directory is restored after login-profile processing as well as supplied by SetEnv.
Verification: the affected unit tests and the new local SSH acceptance test pass. I inspected Cursor's remote environment-resolution code, but have not run this reproduction on live serverless compute. The new acceptance test avoids login startup files, so it does not cover this path.
There was a problem hiding this comment.
On serverless, /etc/profile rebuilds PATH from scratch—the existing comment in client.go:955–961 documents this exact behavior.
This is not true in my testing. It seems that comment is out of date or incorrect
databricks ssh connect --name pr7123 -- 'bash -ilc "command -v claude codex"'
This correctly finds claude and codex for me. Cursor as well works as expected for me
Changes
Remove the interactive-only restriction for agent shims by putting the agent shims on
PATHviaSetEnvon the server-sidesshd(rather than addingexport PATH="..."to the SSH command sent by the client)Why
To allow agents to be used in more places
Tests
added unit tests