fix(driver-vm): resolve lifecycle requests on sandbox_id alone

[letv1nnn]

Summary

stop_sandbox, start_sandbox, and delete_sandbox on the VM driver preferred the supplied sandbox_id but fell back to matching on sandbox_name whenever that id was absent from the registry. Because sandbox names are unique per workspace rather than globally, that fallback could resolve to a same-named sandbox in a different workspace. Lifecycle resolution now treats a supplied id as authoritative.

Related Issue

Fixes #3254

Changes

• stop_sandbox, start_sandbox, and delete_sandbox select the resolution branch on whether the caller supplied a sandbox_id, not on whether that id happened to be present. A supplied id resolves on its own or reports absence.
• Resolution moves into a shared resolve_record_id helper, replacing the same logic copy-pasted across the three methods.
• A name-only request matching more than one sandbox is rejected with FailedPrecondition instead of resolving by HashMap iteration order.
• Test helper insert_named_record sets a record's id, name, and workspace independently.

get_sandbox is unchanged: it already gates its name fallback on sandbox_id.is_empty(), and it is read-only.

Testing

• mise run pre-commit passes
• Unit tests added/updated
• E2E tests added/updated (if applicable)

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[johntmyers]

Is this unique to the vm driver or are others already using ID? I thought we were standardizing on workspace+sandbox_name for sandbox management. @drew can you weigh in?

[letv1nnn]

hey @johntmyers, this is not unique to vm driver, docker (#3240) and mxc (#3278) drivers had the same weakness and got the same fix: sandbox_id wins, and a name-only match backs off instead of guessing.
Workspace+sandbox_name isn't available here because StopSandboxRequest/StartSandboxRequest/DeleteSandboxRequest in compute_driver.proto don't carry a workspace field, that's why K8s/Podman can key on workspace+name (they resolve through platform labels), but the registry-based drivers (docker/vm/mxc) can't without a proto change.

[drew]

We have this ticket #3050 to standardize how we reference sandboxes across the product. Lets prefer that ticket and associated pr. We should try and prefer workspace+name rather than id I think.

[letv1nnn]

Thanks @drew, I read through #3272. My understanding is that it standardizes the public gateway RPCs on workspace + name while keeping sandbox IDs internal at the compute-driver boundary, and it doesn't touch compute_driver.proto or the driver crates. This PR sits below that line, so I think it's complementary rather than competing, but please correct me if I've read the intent wrong.

Two options, happy with either:

1. Land as-is. Same id-authoritative shape as fix(driver-docker): scope pending sandbox matching by id and workspace #3240 and fix(driver-mxc): resolve StopSandbox by sandbox_id #3278, so all three registry drivers behave consistently. Ambiguous name-only requests return FailedPrecondition.
2. Extend to workspace + name at the driver boundary. Add workspace to StopSandboxRequest/StartSandboxRequest/DeleteSandboxRequest in compute_driver.proto and have the driver resolve on (workspace, name), with sandbox_id as a consistency check. That makes the driver mirror feat(api)!: unify sandbox references across gateway RPCs #3050's model instead of diverging from it, but it's an additive proto change touching five driver crates, so I'd suggest it as a follow-up to feat(api)!: unify sandbox references across gateway RPCs #3050 rather than folded in here.

I'd lean toward 1 now and 2 as the follow-up, mainly so docker/vm/mxc don't sit in three different states while #3050 is in flight.

[github-actions]

Label test:e2e applied, but pull-request/3305 does not exist yet. A maintainer needs to comment /ok to test 7d1c9b86b28e0ed2c202175e64028ba19c63671c to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

[drew]

/ok to test 7d1c9b8

[drew]

gator-agent

PR Review Status

@drew, I checked your concern about preferring workspace plus sandbox name against the merged #3272 change and current main. The public gateway RPCs now use workspace and name, while the internal compute-driver lifecycle requests deliberately retain a stable sandbox ID plus runtime name, so @letv1nnn's narrow VM-driver fix is complementary rather than competing. The independent code review found no blocking defects.

Blocking findings:

• No blocking findings remain

Carried findings:

• None

Non-blocking suggestions:

• None

Gator metadata

• Validation: Concentrated VM-driver correctness and cross-workspace data-loss fix linked to #3254; the merged public API standardization does not remove the internal ID-based driver boundary.
• Docs: Fern docs are not needed because this corrects internal driver resolution without changing the supported user-facing workflow.
• Checks: Current-head Branch Checks are queued, Helm Lint is running, and required pipeline monitoring is active.
• E2E: test:e2e applied; /ok to test created pull-request/3305, and Branch E2E Checks are running.
• Head SHA: 7d1c9b86b28e0ed2c202175e64028ba19c63671c
• Base SHA: 5b9daab9351b1e053f9a5e0ce4c899f5d3f674b0
• Merge base SHA: bcf96e4900004e07253d72332a6619f7e2c4e7d7
• Patch ID: cf9176aee04b78990c599f462943c3b5f7be34c7
• Gator payload: 9
• Review mode: initial
• Previous reviewed SHA: none
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:watch-pipeline