feat: stabilize AWS desktop VNC leases#14
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3870d5e45b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func bootstrapAWSWindowsDesktop(ctx context.Context, cfg Config, target SSHTarget, publicKey string, stderr io.Writer) error { | ||
| if cfg.Provider != "aws" || cfg.TargetOS != targetWindows || cfg.WindowsMode != windowsModeNormal { | ||
| return waitForSSH(ctx, &target, stderr) |
There was a problem hiding this comment.
Preserve resolved SSH port from bootstrap helper
bootstrapAWSWindowsDesktop takes target by value, but it calls waitForSSH/waitForSSHReady, which can rewrite target.Port when a fallback port is the one that actually works. Because callers pass a local target and this helper does not return the updated struct, AWS/coordinator acquire flows now drop that resolved port and keep using the original (possibly dead) port for subsequent SSH/VNC commands. This regresses setups that rely on ssh.fallbackPorts or non-default port failover.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee58fbc064
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if isWindowsNativeTarget(target) { | ||
| return `$ErrorActionPreference = "Stop" | ||
| $base = "C:\ProgramData\crabbox" | ||
| $password = Get-Content -Raw -LiteralPath (Join-Path $base "windows.password") |
There was a problem hiding this comment.
Add fallback when
windows.password is absent
The new Windows screenshot path unconditionally reads C:\ProgramData\crabbox\windows.password before creating the scheduled task, so reused AWS Windows leases created by older bootstrap logic (which only wrote vnc.password) now fail with a path-not-found error instead of producing a screenshot. This regression hits existing kept/reclaimed leases after upgrading the CLI; the command should fall back to the legacy password file or otherwise handle missing windows.password gracefully.
Useful? React with 👍 / 👎.
No description provided.