Skip to content

Fix Apple Container status parsing across versions - #1214

Merged
penso merged 1 commit into
mainfrom
cat-mambo
Aug 19, 2026
Merged

Fix Apple Container status parsing across versions#1214
penso merged 1 commit into
mainfrom
cat-mambo

Conversation

@penso

@penso penso commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • replace raw JSON substring matching with a typed Apple Container status decoder
  • accept both pre-1.x scalar status values and 1.x nested status.state values
  • use the shared decoder for readiness, lifecycle inspection, failed-removal classification, and container listing
  • add regression coverage for both schemas, whitespace, stopped/exited states, unknown states, and malformed output

Fixes #1185.

Validation

Completed

  • cargo test -p moltis-tools sandbox::tests::apple -- --nocapture
  • ./scripts/check-file-size.sh
  • just release-preflight

Remaining

  • ./scripts/local-validate.sh <PR_NUMBER>
  • Manual runtime verification with Apple Container pre-1.x and 1.x installations

Manual QA

  1. Configure [tools.exec.sandbox] with backend = "apple-container" and mode = "all".
  2. Run a sandboxed command such as pwd using Apple Container 1.x and confirm the container reaches exec-ready state without being removed and recreated.
  3. Repeat with a pre-1.x Apple Container installation and confirm scalar status output remains supported.
  4. Open the sandbox container list and verify running, stopped, and exited states render correctly on either runtime version.

Apple Container changed inspect status from a scalar in pre-1.x releases to a nested state object in 1.x. Lifecycle checks still searched minified JSON for the scalar form, while list rendering only handled the nested form, so either runtime could be misclassified.\n\nDecode both schemas through one typed parser and use the resulting run-state enum for readiness, inspection, failed-removal classification, and list output. Add regression coverage for both schemas, formatting differences, terminal states, and malformed output.\n\nFixes #1185
@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces brittle Apple Container status substring checks with a typed decoder shared across readiness, lifecycle inspection, removal handling, and container listing.

  • Supports legacy scalar status values and current nested status.state values.
  • Preserves distinct running, stopped, exited, and unknown states.
  • Adds regression coverage for both schemas, whitespace, malformed output, missing status, and unknown values.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code failure established.

The shared decoder handles both status schemas described by the change, its callers consistently interpret the resulting states, and the added tests cover the relevant successful, stopped, exited, unknown, empty, and malformed cases.

Important Files Changed

Filename Overview
crates/tools/src/sandbox/containers.rs Introduces the shared typed status decoder and applies it consistently to listing, removal classification, and inspect parsing without an established defect.
crates/tools/src/sandbox/apple.rs Updates readiness and lifecycle inspection to consume typed running, stopped, and exited states.
crates/tools/src/sandbox/tests/apple.rs Adds focused regression tests for legacy and current status schemas, malformed output, missing fields, and unknown states.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  CLI["Apple Container JSON output"] --> Decoder["Shared typed status decoder"]
  Decoder --> Legacy["Legacy: status = running"]
  Decoder --> Current["Current: status.state = running"]
  Legacy --> State["ContainerRunState"]
  Current --> State
  State --> Ready["Readiness polling"]
  State --> Inspect["Lifecycle inspection"]
  State --> Remove["Removal classification"]
  State --> List["Container listing"]
Loading

Reviews (1): Last reviewed commit: "fix(sandbox): parse Apple Container stat..." | Re-trigger Greptile

@penso
penso merged commit a92821f into main Aug 19, 2026
22 of 32 checks passed
@penso
penso deleted the cat-mambo branch August 19, 2026 04:07
@codspeed-hq

codspeed-hq Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 10.19%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 38 untouched benchmarks
⏩ 9 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
session_history_path[2026-02-09T12:00:00Z] 7.9 µs 7.2 µs +10.19%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cat-mambo (5a2ca90) with main (d739b6f)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

[Bug]: Apple Container 1.x sandbox starts but Moltis treats it as not running

1 participant