Skip to content

fix(ac): finish delete when container stop errors but the container stopped anyway - #1225

Merged
cdalar merged 1 commit into
mainfrom
fix/ac-destroy-stop-internal-error
Oct 22, 2026
Merged

cdalar merged 1 commit into
mainfrom
fix/ac-destroy-stop-internal-error

Conversation

@cdalar

@cdalar cdalar commented Oct 22, 2026

Copy link
Copy Markdown
Owner

Summary

onctl delete on the ac provider failed with:

✘ Cannot destroy VM: claude-onctl
failed to stop container "claude-onctl": container stop: Error: internalError: "stop failed for one or more containers claude-onctl"

while onctl ls right after showed the container as stopped. container stop (0.1.0) sometimes reports an internalError for a container it did stop; Destroy took the exit code at face value and never ran container rm, leaving the sandbox behind.

Destroy now re-reads the container's state after a failed stop and continues with the delete when it is stopped. A stop that leaves the container running still aborts the delete as before.

Test plan

  • New TestAC_Destroy_StopFailsButStopped: stop errors, container reads stopped on the next list, rm runs and the container is gone. Fails on main, passes here.
  • Existing TestAC_Destroy_StopFails still proves a stop that leaves the container running never reaches rm.
  • go vet ./pkg/cloud/ and go test ./pkg/cloud/ pass.

πŸ€– Generated with Claude Code

https://claude.ai/code/session_01HJvsvJw8cZfuUwPFRQXV5k

`container stop` (0.1.0) occasionally exits with "internalError: stop
failed for one or more containers" for a container it did stop. Destroy
treated that as fatal and never ran `container rm`, so `onctl delete`
reported "Cannot destroy VM" and left the sandbox behind as stopped.

Re-read the container's state after a failed stop and carry on with the
delete when it is stopped; still fail when it is not.

Claude-Session: https://claude.ai/code/session_01HJvsvJw8cZfuUwPFRQXV5k
@cdalar
cdalar force-pushed the fix/ac-destroy-stop-internal-error branch from 5fdcd32 to 5997948 Compare October 22, 2026 22:18
@codecov

codecov Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 46.73%. Comparing base (8de477b) to head (5997948).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1225      +/-   ##
==========================================
+ Coverage   46.72%   46.73%   +0.01%     
==========================================
  Files          75       75              
  Lines        8783     8785       +2     
==========================================
+ Hits         4104     4106       +2     
  Misses       4679     4679              

β˜” View full report in Codecov by Harness.
πŸ“’ Have feedback on the report? Share it here.

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cdalar
cdalar merged commit f32ed6b into main Oct 22, 2026
14 checks passed
@cdalar
cdalar deleted the fix/ac-destroy-stop-internal-error branch October 22, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

⌂ 捒网址