Working directory auto cd#9
Conversation
📝 WalkthroughWalkthroughThis PR adds a workspace cd-on-login feature: a new ChangesWorkspace cd-on-login feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BaseImageBuilder
participant Dockerfile
participant SSHClient
participant Container
BaseImageBuilder->>Dockerfile: write /etc/profile.d/workspace-cd.sh
BaseImageBuilder->>Dockerfile: chmod +x workspace-cd.sh
SSHClient->>Container: connect over SSH
SSHClient->>Container: run "bash -l -c pwd"
Container-->>SSHClient: working directory output
Container-->>SSHClient: fallback to home dir if /workspace is missing
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
internal/docker/integration_lifecycle_test.go (1)
62-91: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPurge test can leak the container if an earlier assertion fails.
TestPurgeRemovesContainersAndImagesintentionally skips registeringcleanup(line 70) so the manual stop/remove is itself part of the test. Ifrequire.NotNilat line 78 (or any earlier require) fails,t.FailNow()aborts before the container is stopped/removed, leaking it for subsequent runs.Consider registering a
t.Cleanupsafety net (force stop+remove, ignoring errors) in addition to the explicit purge-logic assertions — it won't interfere with the "container is gone" check since removing an already-removed container is a no-op.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/docker/integration_lifecycle_test.go` around lines 62 - 91, The purge test in TestPurgeRemovesContainersAndImages can leak a container if any earlier require fails before the manual stop/remove path runs. Add a t.Cleanup safety net right after startContainerFromSharedImage that forcefully stops and removes the container using docker.StopContainer and docker.RemoveContainer, ignoring cleanup errors, while keeping the explicit purge assertions in the test body so the container lifecycle is still validated.internal/docker/integration_main_test.go (3)
85-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated "resolve user strategy + build base/instance image" boilerplate across files.
This same ~15-line pattern (
FindConflictingUser→ strategy/conflictingUser →NewBaseImageBuilder/NewInstanceImageBuilder) is repeated near-verbatim inintegration_lifecycle_test.go(TestTwoLayerBuildCycle), all three tests inintegration_network_test.go, andintegration_ssh_test.go(buildAndGetFingerprint). Consider extracting a shared helper here (e.g.resolveUserStrategy(ctx, client, info)and/orbuildBaseAndInstanceImages(...)) that all the other integration test files can reuse.Also applies to: 100-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/docker/integration_main_test.go` around lines 85 - 92, The duplicated user-strategy and image-builder setup logic should be extracted into shared helpers instead of being repeated across integration tests. Create a reusable helper around the FindConflictingUser call and strategy/conflictingUser selection (for example resolveUserStrategy(ctx, client, info)), and consider a second helper for the NewBaseImageBuilder/NewInstanceImageBuilder sequence so TestTwoLayerBuildCycle, the integration_network tests, buildAndGetFingerprint in integration_ssh_test, and this test can all call the same utility.
59-64: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLazy shared-image init isn't safe against future parallelism.
sharedImageTag != ""guards a package-level singleton without a mutex/sync.Once. It's safe today only because no test callst.Parallel(). If that changes later, this becomes a data race (and could trigger duplicate/racy image builds).♻️ Suggested fix using sync.Once
+var buildOnce sync.Once + func buildSharedImage(t *testing.T) { t.Helper() - - if sharedImageTag != "" { - return // already built - } - - ctx := context.Background() - ... + buildOnce.Do(func() { buildSharedImageOnce(t) }) +} + +func buildSharedImageOnce(t *testing.T) { + ctx := context.Background() + ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/docker/integration_main_test.go` around lines 59 - 64, The package-level shared-image initialization in buildSharedImage is guarded only by sharedImageTag, which is unsafe if tests ever run in parallel. Update the singleton initialization to use a sync.Once-based guard (or equivalent mutex protection) around buildSharedImage so the shared image is built exactly once without races, and keep the existing sharedImageTag check only as the cached result inside that protected path.
190-199: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
findFreePorthas a check-then-use race (TOCTOU).The listener is closed immediately after probing, and the port isn't reserved until the container is actually started later. Another process (or a concurrently-running test binary) could grab the port in between, causing intermittent "address already in use" flakes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/docker/integration_main_test.go` around lines 190 - 199, The findFreePort helper in integration_main_test.go has a TOCTOU race because it probes a port with net.Listen and closes it before the container uses it. Fix this by changing the flow around findFreePort and the container startup so the chosen port stays reserved until the service is ready, or eliminate the separate probe-and-reuse pattern by letting the container/runtime bind an available port directly. Refer to findFreePort and the later container start path that consumes its result when applying the change.internal/docker/integration_network_test.go (1)
27-351: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared setup to remove ~90 lines of triplicated boilerplate.
TestHostNetworkModeSSHReachable,TestHostNetworkCanReachHostService, andTestBridgeModeSSHReachableeach repeat the identical "generate keys → resolve user strategy → build base image → build instance image → create+start container" sequence, differing only inhostNetworkOffand a name prefix. Extracting a helper (e.g.buildAndStartNetworkContainer(t, namePrefix string, hostNetworkOff bool) (containerName string, sshPort int, client *docker.Client)) inintegration_main_test.gowould cut duplication significantly and reduce the chance of the three copies drifting out of sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/docker/integration_network_test.go` around lines 27 - 351, The three network integration tests duplicate the same setup flow, so extract the shared “generate keys → resolve user strategy → build base image → build instance image → create/start container” logic into a helper such as buildAndStartNetworkContainer used by TestHostNetworkModeSSHReachable, TestHostNetworkCanReachHostService, and TestBridgeModeSSHReachable. Keep the per-test differences (name prefix, hostNetworkOff, and the host-service connectivity assertion) in the test bodies, and reuse the helper’s outputs like containerName, sshPort, and client to avoid the repeated boilerplate drifting apart.internal/docker/integration_ssh_test.go (1)
424-436: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winOverly broad stderr assertion risks unrelated flakes.
require.Empty(t, stderr.String(), ...)asserts that no profile.d script emits anything to stderr during the login shell, not just the workspace-cd script under test. Since/etc/profile.d/dbus-keyring.sh(D-Bus/gnome-keyring startup) also runs duringbash -l, any warning it emits (e.g., DBus launch issues in the container environment) would fail this test for reasons unrelated to the workspace-cd fallback behavior being validated (Req 27.3).Consider scoping this check to just the workspace script, e.g. exec
sh /etc/profile.d/workspace-cd.shdirectly and check its own stderr, rather than asserting on the full login-shell stderr.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/docker/integration_ssh_test.go` around lines 424 - 436, The stderr assertion in the login-shell flow is too broad because it captures output from all profile.d scripts, not just the workspace-cd behavior being tested. Update the test around session.Run("bash -l -c pwd") to avoid requiring stderr.String() to be empty for the whole shell, and instead scope validation to the workspace script itself by invoking workspace-cd.sh directly and checking only its stderr. Keep the existing fallback checks in integration_ssh_test.go focused on pwd output and info.HomeDir.internal/docker/integration_misc_test.go (1)
28-67: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDocker build-cache may make this test flaky/false-negative.
specdoesn't setNoCache: true. If a prior run already built an image with the sameFROM <base>+RUN sleep 300instruction (even under a different tag, since Docker's build cache is keyed on instruction+parent-layer, not the target tag), a subsequent build can hit the cached layer and finish well undertestBuildTimeout, causingrequire.Errorto fail for reasons unrelated to the timeout logic under test.♻️ Suggested fix
spec := docker.ContainerSpec{ Name: containerName, ImageTag: imageTag, Dockerfile: hangingDockerfile, Labels: map[string]string{"bac.managed": "true"}, + NoCache: true, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/docker/integration_misc_test.go` around lines 28 - 67, The timeout test in TestBuildImageTimeoutEnforced can be a false negative because Docker may reuse a cached layer for the hanging Dockerfile. Update the ContainerSpec used for docker.BuildImageWithTimeout to disable build cache (for example via the NoCache field) so the RUN sleep 300 step is always executed, keeping the test focused on timeout enforcement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/docker/integration_misc_test.go`:
- Around line 70-101: TestAFindConflictingUserPullsImageIfAbsent currently
depends on test ordering and TestMain to ensure the base image is absent, so it
may miss the intended code path if other tests pull it first. Update this test
to call testutil.EnsureBaseImageAbsent() at the start before creating the Docker
client and invoking docker.FindConflictingUser, so the “image absent” scenario
is enforced regardless of package order.
In `@internal/docker/integration_ssh_test.go`:
- Around line 71-131: The SSH fingerprint check in buildAndGetFingerprint is
tautological because it returns the captured hostKeyPub input instead of
verifying the rebuilt artifact. Update buildAndGetFingerprint and the final
require.Equal assertion to extract the actual SSH host key from each rebuilt
image/container, using the existing docker.BuildImage flow plus a real readback
path such as SSH HostKeyCallback or docker.ExecInContainer against the instance
built by NewInstanceImageBuilder, then compare those observed fingerprints
rather than the originally generated variable.
---
Nitpick comments:
In `@internal/docker/integration_lifecycle_test.go`:
- Around line 62-91: The purge test in TestPurgeRemovesContainersAndImages can
leak a container if any earlier require fails before the manual stop/remove path
runs. Add a t.Cleanup safety net right after startContainerFromSharedImage that
forcefully stops and removes the container using docker.StopContainer and
docker.RemoveContainer, ignoring cleanup errors, while keeping the explicit
purge assertions in the test body so the container lifecycle is still validated.
In `@internal/docker/integration_main_test.go`:
- Around line 85-92: The duplicated user-strategy and image-builder setup logic
should be extracted into shared helpers instead of being repeated across
integration tests. Create a reusable helper around the FindConflictingUser call
and strategy/conflictingUser selection (for example resolveUserStrategy(ctx,
client, info)), and consider a second helper for the
NewBaseImageBuilder/NewInstanceImageBuilder sequence so TestTwoLayerBuildCycle,
the integration_network tests, buildAndGetFingerprint in integration_ssh_test,
and this test can all call the same utility.
- Around line 59-64: The package-level shared-image initialization in
buildSharedImage is guarded only by sharedImageTag, which is unsafe if tests
ever run in parallel. Update the singleton initialization to use a
sync.Once-based guard (or equivalent mutex protection) around buildSharedImage
so the shared image is built exactly once without races, and keep the existing
sharedImageTag check only as the cached result inside that protected path.
- Around line 190-199: The findFreePort helper in integration_main_test.go has a
TOCTOU race because it probes a port with net.Listen and closes it before the
container uses it. Fix this by changing the flow around findFreePort and the
container startup so the chosen port stays reserved until the service is ready,
or eliminate the separate probe-and-reuse pattern by letting the
container/runtime bind an available port directly. Refer to findFreePort and the
later container start path that consumes its result when applying the change.
In `@internal/docker/integration_misc_test.go`:
- Around line 28-67: The timeout test in TestBuildImageTimeoutEnforced can be a
false negative because Docker may reuse a cached layer for the hanging
Dockerfile. Update the ContainerSpec used for docker.BuildImageWithTimeout to
disable build cache (for example via the NoCache field) so the RUN sleep 300
step is always executed, keeping the test focused on timeout enforcement.
In `@internal/docker/integration_network_test.go`:
- Around line 27-351: The three network integration tests duplicate the same
setup flow, so extract the shared “generate keys → resolve user strategy → build
base image → build instance image → create/start container” logic into a helper
such as buildAndStartNetworkContainer used by TestHostNetworkModeSSHReachable,
TestHostNetworkCanReachHostService, and TestBridgeModeSSHReachable. Keep the
per-test differences (name prefix, hostNetworkOff, and the host-service
connectivity assertion) in the test bodies, and reuse the helper’s outputs like
containerName, sshPort, and client to avoid the repeated boilerplate drifting
apart.
In `@internal/docker/integration_ssh_test.go`:
- Around line 424-436: The stderr assertion in the login-shell flow is too broad
because it captures output from all profile.d scripts, not just the workspace-cd
behavior being tested. Update the test around session.Run("bash -l -c pwd") to
avoid requiring stderr.String() to be empty for the whole shell, and instead
scope validation to the workspace script itself by invoking workspace-cd.sh
directly and checking only its stderr. Keep the existing fallback checks in
integration_ssh_test.go focused on pwd output and info.HomeDir.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 779e5163-e43d-4667-a56e-39e70cad6e4e
📒 Files selected for processing (7)
internal/docker/integration_container_test.gointernal/docker/integration_lifecycle_test.gointernal/docker/integration_main_test.gointernal/docker/integration_misc_test.gointernal/docker/integration_network_test.gointernal/docker/integration_ssh_test.gointernal/docker/integration_test.go
💤 Files with no reviewable changes (1)
- internal/docker/integration_test.go
| // TestAFindConflictingUserPullsImageIfAbsent | ||
| // Validates: Req 10a.1 — FindConflictingUser must succeed even when the base | ||
| // image is not present in the local Docker image store. | ||
| // | ||
| // Named with "A" prefix so Go's alphabetical test ordering runs this first. | ||
| // The base image is guaranteed absent by TestMain's call to | ||
| // EnsureBaseImageAbsent(), so this test simply calls FindConflictingUser and | ||
| // asserts it succeeds (pulling the image automatically). | ||
| // ---------------------------------------------------------------------------- | ||
|
|
||
| func TestAFindConflictingUserPullsImageIfAbsent(t *testing.T) { | ||
| if _, err := exec.LookPath("docker"); err != nil { | ||
| t.Skip("docker not available") | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
|
|
||
| client, err := docker.NewClient() | ||
| require.NoError(t, err, "connecting to Docker daemon") | ||
|
|
||
| info, err := hostinfo.Current() | ||
| require.NoError(t, err) | ||
|
|
||
| result, err := docker.FindConflictingUser(ctx, client, info.UID, info.GID) | ||
| require.NoError(t, err, | ||
| "FindConflictingUser must succeed even when the base image is not cached locally") | ||
| _ = result | ||
|
|
||
| _, _, err = client.ImageInspectWithRaw(ctx, constants.BaseContainerImage) | ||
| require.NoError(t, err, | ||
| "base image should be present locally after FindConflictingUser pulls it") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm current cross-file test ordering assumption for the integration package
fd -e go -g '*_test.go' internal/docker | sortRepository: koudis/bootstrap-ai-coding
Length of output: 594
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the integration test setup and any explicit ordering dependencies.
ast-grep outline internal/docker/integration_misc_test.go --view expanded || true
printf '\n--- TestMain / setup references ---\n'
rg -n "EnsureBaseImageAbsent|TestMain|startContainerFromSharedImage|buildSharedImage|FindConflictingUser|RunTests|Test[A-Z]" internal/docker/*.goRepository: koudis/bootstrap-ai-coding
Length of output: 19588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- internal/docker/integration_main_test.go ---'
sed -n '1,220p' internal/docker/integration_main_test.go
printf '\n%s\n' '--- internal/docker/integration_container_test.go ---'
sed -n '1,170p' internal/docker/integration_container_test.go
printf '\n%s\n' '--- internal/docker/integration_lifecycle_test.go ---'
sed -n '1,210p' internal/docker/integration_lifecycle_test.goRepository: koudis/bootstrap-ai-coding
Length of output: 18862
🌐 Web query:
Go testing package order of execution top-level tests file order alphabetical filenames order guarantee
💡 Result:
In Go, there is no guarantee regarding the execution order of top-level tests [1]. The Go testing framework does not specify a required order, and it is considered a best practice for unit tests to be independent of each other [2]. While some observers may note that tests often execute in a consistent order (such as alphabetically by filename) under specific local conditions, this behavior is not guaranteed by the language specification or the testing tool [1]. Relying on any specific execution order is discouraged. To manage execution order, you should avoid dependencies between tests. If you require specific control over test execution, you can use the following mechanisms: 1. Shuffle Flag: You can use the -shuffle flag (e.g., -shuffle on) with the go test command to randomize the execution order [3][2]. This is useful for identifying hidden dependencies between tests, as it forces them to run in a non-deterministic sequence. 2. Subtests: For related tests that require a specific sequence, you can group them within a single test function using subtests (t.Run) [4][5][6]. Subtests execute in the order they are defined within the parent test function. 3. Test Suites or Frameworks: If complex setup and teardown order are required, consider using standard testing patterns or established third-party testing frameworks that provide explicit lifecycle management. In summary, the execution order of top-level tests is undefined, and you should design your test suite to be resilient to any order of execution [1][2].
Citations:
- 1: https://stackoverflow.com/questions/31201858/how-to-run-golang-tests-sequentially
- 2: https://dev.to/gkampitakis/go-test-supports-shuffling-tests-38e
- 3: https://pkg.go.dev/cmd/go/internal/test
- 4: https://go.dev/src/testing/testing.go?m=text
- 5: https://github.com/golang/go/blob/master/src/testing/testing.go
- 6: https://pkg.go.dev/testing
Make this test independent of package order
Go test does not guarantee top-level test order, so the A prefix/file name won’t reliably make this run before other integration tests. startContainerFromSharedImage can pull the base image first, which means this no longer covers the “image absent” path. Call testutil.EnsureBaseImageAbsent() at the start of this test instead of relying on TestMain alone.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/docker/integration_misc_test.go` around lines 70 - 101,
TestAFindConflictingUserPullsImageIfAbsent currently depends on test ordering
and TestMain to ensure the base image is absent, so it may miss the intended
code path if other tests pull it first. Update this test to call
testutil.EnsureBaseImageAbsent() at the start before creating the Docker client
and invoking docker.FindConflictingUser, so the “image absent” scenario is
enforced regardless of package order.
| buildAndGetFingerprint := func() string { | ||
| t.Helper() | ||
|
|
||
| strategy := docker.UserStrategyCreate | ||
| conflictingUser := "" | ||
| conflictingImageUser, err := docker.FindConflictingUser(ctx, client, info.UID, info.GID) | ||
| require.NoError(t, err, "checking base image for UID/GID conflicts") | ||
| if conflictingImageUser != nil { | ||
| strategy = docker.UserStrategyRename | ||
| conflictingUser = conflictingImageUser.Username | ||
| } | ||
|
|
||
| builder := docker.NewBaseImageBuilder( | ||
| info, | ||
| strategy, conflictingUser, | ||
| "", | ||
| ) | ||
|
|
||
| // Build base image | ||
| baseSpec := docker.ContainerSpec{ | ||
| Name: containerName, | ||
| ImageTag: constants.BaseImageTag, | ||
| Dockerfile: builder.Build(), | ||
| Labels: map[string]string{"bac.managed": "true"}, | ||
| HostInfo: info, | ||
| } | ||
|
|
||
| _, err = docker.BuildImage(ctx, client, baseSpec, false) | ||
| require.NoError(t, err, "building base image") | ||
|
|
||
| // Build instance image | ||
| instanceBuilder := docker.NewInstanceImageBuilder( | ||
| info, | ||
| userPubKey, | ||
| hostKeyPriv, hostKeyPub, | ||
| 2222, false, | ||
| ) | ||
| instanceBuilder.Finalize() | ||
| spec := docker.ContainerSpec{ | ||
| Name: containerName, | ||
| ImageTag: imageTag, | ||
| Dockerfile: instanceBuilder.Build(), | ||
| Mounts: []docker.Mount{ | ||
| {HostPath: projectDir, ContainerPath: constants.WorkspaceMountPath}, | ||
| }, | ||
| SSHPort: port, | ||
| Labels: map[string]string{"bac.managed": "true"}, | ||
| HostInfo: info, | ||
| } | ||
|
|
||
| _, err = docker.BuildImage(ctx, client, spec, false) | ||
| require.NoError(t, err, "building instance image") | ||
|
|
||
| return hostKeyPub | ||
| } | ||
|
|
||
| fingerprint1 := buildAndGetFingerprint() | ||
| fingerprint2 := buildAndGetFingerprint() | ||
|
|
||
| require.Equal(t, fingerprint1, fingerprint2, | ||
| "SSH host key fingerprint must be stable across rebuilds") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Test is tautological — it can never fail regardless of actual rebuild behavior.
buildAndGetFingerprint closes over the single hostKeyPub value generated once at line 40, and returns that exact same variable both times it's called (line 124). fingerprint1 and fingerprint2 are therefore always the identical string by construction — the require.Equal at line 130 provides no real signal about whether the built image/container actually persists or re-derives the SSH host key correctly across rebuilds; it only checks that a variable equals itself.
To genuinely validate Req 13.3, the test needs to read back the host key that's actually baked into (or presented by) each rebuilt artifact — e.g. dial SSH against a container started from each build and capture the presented host key via a custom HostKeyCallback, or docker.ExecInContainer + cat/hash the host key file inside the container — and compare those extracted values instead of the input parameter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/docker/integration_ssh_test.go` around lines 71 - 131, The SSH
fingerprint check in buildAndGetFingerprint is tautological because it returns
the captured hostKeyPub input instead of verifying the rebuilt artifact. Update
buildAndGetFingerprint and the final require.Equal assertion to extract the
actual SSH host key from each rebuilt image/container, using the existing
docker.BuildImage flow plus a real readback path such as SSH HostKeyCallback or
docker.ExecInContainer against the instance built by NewInstanceImageBuilder,
then compare those observed fingerprints rather than the originally generated
variable.
1 participant
Summary by CodeRabbit
/workspaceby default (login-time working directory setup)./workspaceisn’t available, SSH login continues gracefully (workspace switch is skipped without breaking the session).