fix(Roseta): fixed Roseta enablement on macOS Tahoe with Podman 5.6+ - #16924
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a RosettaProvisioner injectable and integrates Rosetta enablement into Podman machine start/create flows on macOS/arm64/applehv + Podman >=5.6: determines need, optionally SSH-writes Changes
Sequence Diagram(s)sequenceDiagram
participant Extension as Extension
participant Rosetta as RosettaProvisioner
participant PodmanCLI as Podman CLI
participant VM as Machine VM
Extension->>PodmanCLI: startMachine()
PodmanCLI->>VM: machine start
VM-->>PodmanCLI: started
PodmanCLI-->>Extension: start success
Extension->>PodmanCLI: getBinaryInfo()
PodmanCLI-->>Extension: podman version
alt macOS + arm64 + podman >= 5.6 + vmType applehv
Extension->>Rosetta: provisionAndRestartForRosetta(machineName, provider, options)
Rosetta->>PodmanCLI: machine ssh "test -f /etc/containers/enable-rosetta"
PodmanCLI-->>Rosetta: file exists / not found
alt file missing
Rosetta->>PodmanCLI: machine ssh "sudo touch /etc/containers/enable-rosetta"
PodmanCLI-->>Rosetta: file created
Rosetta->>PodmanCLI: machine stop
PodmanCLI-->>VM: stop
PodmanCLI-->>Rosetta: stopped
Rosetta->>PodmanCLI: machine start (forwarded options)
PodmanCLI-->>VM: start
PodmanCLI-->>Rosetta: restarted
else file exists
PodmanCLI-->>Rosetta: no-op
end
else skip provisioning
end
Extension-->>Extension: provider.updateStatus('started')
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/podman/packages/extension/src/extension.spec.ts`:
- Around line 966-1035: Add unit tests covering the new Rosetta provisioning
paths in extension.createMachine (the --now immediate-start branch and the
stopped-creation branch) similar to the existing startMachine suite: spy/mocked
PODMAN_BINARY_MOCK.getBinaryInfo and enableRosettaInMachine to simulate (a)
no-op (file exists), (b) file-created (requires stop/start sequence), and (c)
thrown error (warning-only); assert calls to extensionApi.process.exec (using
podmanCli.getPodmanCli and the expected ['machine','stop', name] /
['machine','start', name] sequences and counts), that enableRosettaInMachine
is/isn't called appropriately, console.warn is invoked on errors, and
provider.updateStatus is eventually called with 'started' for both --now and
stopped createMachine flows.
In `@extensions/podman/packages/extension/src/extension.ts`:
- Around line 889-906: The try/catch around enableRosettaInMachine + the calls
to execPodman(['machine','stop'...]) and execPodman(['machine','start'...])
swallows errors after the VM state has already been mutated, which can leave the
VM in the wrong state while startMachine() still reports "started"; change this
to attempt to restore the original VM state on failure and surface the error
instead of silently logging it: after a failed
execPodman(['machine','start'...']) (or any error inside that try block) call
execPodman(['machine','start', machineInfo.name], machineInfo.vmType) if the VM
was stopped by your code to restore state (or attempt
execPodman(['machine','stop'...]) if you need to revert a started VM), log both
the restoration attempt result, and then rethrow the original error so callers
see the failure; apply the same change to the other identical block referenced
(around lines 2209-2235) and use the same functions enableRosettaInMachine,
execPodman and LoggerDelegator in the restoration/error handling logic.
In `@extensions/podman/packages/extension/src/utils/rosetta.spec.ts`:
- Around line 151-155: The test for enableRosettaInMachine relies on previous
state because vi.resetAllMocks() doesn't restore envAPI.isMac; make the
non-macOS scenario deterministic by explicitly setting envAPI.isMac = false in
that test (or reset env flags in the test suite beforeEach), ensuring the test
for enableRosettaInMachine (and the expect(execPodman).not.toBeCalled()) runs
with envAPI.isMac false rather than depending on arch() being unset.
In `@extensions/podman/packages/extension/src/utils/rosetta.ts`:
- Around line 42-55: Function needsRosettaEnableFile currently doesn't scope to
the AppleHV provider; update it to check the machine/provider type (e.g.,
podmanConfiguration.vmType) and return false unless it equals the AppleHV
provider constant (use APPLE_HV_VM_TYPE or the existing AppleHV identifier), so
the Rosetta-enable file path only runs for AppleHV machines; also apply the same
vmType/provider check to the other similar predicate at lines 68-74 (the
analogous function) and ensure enableRosettaInMachine/execPodman continue to
receive the provider info if needed.
- Line 49: Replace the global parseInt usage in the darwinMajor assignment with
the Number namespace: change the expression in const darwinMajor (currently
parseInt(release().split('.')[0], 10)) to use
Number.parseInt(release().split('.')[0], 10) so it complies with the
useNumberNamespace rule.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e4be6ac-4fe8-480d-be99-5777f2fbd5ec
📒 Files selected for processing (4)
extensions/podman/packages/extension/src/extension.spec.tsextensions/podman/packages/extension/src/extension.tsextensions/podman/packages/extension/src/utils/rosetta.spec.tsextensions/podman/packages/extension/src/utils/rosetta.ts
Comment on lines
+889
to
+906
| const podmanInstallation = await podmanBinary.getBinaryInfo(); | ||
| if (podmanInstallation?.version) { | ||
| try { | ||
| const fileCreated = await enableRosettaInMachine( | ||
| machineInfo.name, | ||
| machineInfo.vmType, | ||
| podmanConfiguration, | ||
| podmanInstallation.version, | ||
| ); | ||
| if (fileCreated) { | ||
| await execPodman(['machine', 'stop', machineInfo.name], machineInfo.vmType); | ||
| await execPodman(['machine', 'start', machineInfo.name], machineInfo.vmType, { | ||
| logger: new LoggerDelegator(context, logger), | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| console.warn(`Failed to set up Rosetta enable file during machine start: ${err}`); | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t swallow failures after the machine state has already changed.
Once fileCreated/setupNeeded is true, these branches start mutating VM state. If machine stop succeeds and the following machine start fails, startMachine() still reports started; likewise the temporary create-time start/stop path can return with the VM left in the wrong final state. Only downgrade pre-transition checks to warnings, or restore the original state before swallowing the error.
Also applies to: 2209-2235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/podman/packages/extension/src/extension.ts` around lines 889 -
906, The try/catch around enableRosettaInMachine + the calls to
execPodman(['machine','stop'...]) and execPodman(['machine','start'...])
swallows errors after the VM state has already been mutated, which can leave the
VM in the wrong state while startMachine() still reports "started"; change this
to attempt to restore the original VM state on failure and surface the error
instead of silently logging it: after a failed
execPodman(['machine','start'...']) (or any error inside that try block) call
execPodman(['machine','start', machineInfo.name], machineInfo.vmType) if the VM
was stopped by your code to restore state (or attempt
execPodman(['machine','stop'...]) if you need to revert a started VM), log both
the restoration attempt result, and then rethrow the original error so callers
see the failure; apply the same change to the other identical block referenced
(around lines 2209-2235) and use the same functions enableRosettaInMachine,
execPodman and LoggerDelegator in the restoration/error handling logic.
gastoner
marked this pull request as draft
April 2, 2026 05:13
gastoner
marked this pull request as ready for review
April 2, 2026 05:43
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
extensions/podman/packages/extension/src/utils/rosetta.ts (1)
53-53:⚠️ Potential issue | 🟡 MinorUse
Number.parseInthere.Biome still flags Line 53, so this helper will fail lint until the global
parseIntis replaced.🧹 Minimal fix
- const darwinMajor = parseInt(release().split('.')[0], 10); + const darwinMajor = Number.parseInt(release().split('.')[0], 10);As per coding guidelines,
**/*.{ts,tsx,js,jsx,svelte}: Follow ESLint and Biome rules for code style and formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/podman/packages/extension/src/utils/rosetta.ts` at line 53, Replace the global parseInt call with the namespaced Number.parseInt for the darwinMajor assignment: update the expression that sets const darwinMajor (currently using parseInt(release().split('.')[0], 10)) to use Number.parseInt(...) so it satisfies Biome/ESLint rules and removes the lint flag.extensions/podman/packages/extension/src/extension.ts (1)
2203-2221:⚠️ Potential issue | 🟠 MajorDon't swallow Rosetta setup errors after the VM state has changed.
This catch still hides failures after the code has already started/stopped the machine. On the
--nowpath, a failed restart fromprovisionAndRestartForRosetta()can leave the VM stopped; on the non---nowpath, a failed finalmachine stopleaves it running, yetcreateMachine()still returns success. If you want a best-effort preflight, keep the warning before anymachine start, but rethrow once this block begins mutating VM state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/podman/packages/extension/src/extension.ts` around lines 2203 - 2221, The try/catch currently swallows errors even after we've mutated VM state; change it to only "best-effort" warn before any machine start, but rethrow if any mutation occurred. Add a local boolean (e.g., mutationOccurred) and set it true immediately before calling provisionAndRestartForRosetta(...) or before execPodman(['machine','start',...])/execPodman(['machine','stop',...]) in the non-`--now` path; in the catch block, if mutationOccurred is true then rethrow the caught error (preserve original error), otherwise keep the console.warn as-is. Use the existing symbols provisionAndRestartForRosetta, needsRosettaEnableFile, execPodman and ROSETTA_ENABLE_FILE to locate the relevant calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/podman/packages/extension/src/utils/rosetta.ts`:
- Around line 111-115: The catch block around enableRosettaInMachine currently
swallows errors and returns false (same as "no-op"), so update the handler to
propagate the failure instead of masking it: either remove the try/catch so the
exception bubbles up, or rethrow a new Error with context (e.g., `throw new
Error(\`Failed to provision Rosetta enable file for ${machineName}: ${err}\`)`)
so callers like startMachine and createMachine can detect and surface the
provisioning failure rather than treating it as "nothing to do". Ensure
references to enableRosettaInMachine, startMachine, and createMachine are
preserved when adjusting control flow.
---
Duplicate comments:
In `@extensions/podman/packages/extension/src/extension.ts`:
- Around line 2203-2221: The try/catch currently swallows errors even after
we've mutated VM state; change it to only "best-effort" warn before any machine
start, but rethrow if any mutation occurred. Add a local boolean (e.g.,
mutationOccurred) and set it true immediately before calling
provisionAndRestartForRosetta(...) or before
execPodman(['machine','start',...])/execPodman(['machine','stop',...]) in the
non-`--now` path; in the catch block, if mutationOccurred is true then rethrow
the caught error (preserve original error), otherwise keep the console.warn
as-is. Use the existing symbols provisionAndRestartForRosetta,
needsRosettaEnableFile, execPodman and ROSETTA_ENABLE_FILE to locate the
relevant calls.
In `@extensions/podman/packages/extension/src/utils/rosetta.ts`:
- Line 53: Replace the global parseInt call with the namespaced Number.parseInt
for the darwinMajor assignment: update the expression that sets const
darwinMajor (currently using parseInt(release().split('.')[0], 10)) to use
Number.parseInt(...) so it satisfies Biome/ESLint rules and removes the lint
flag.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b4843b1-ba12-4737-86fa-f34cad3bb6bb
📒 Files selected for processing (4)
extensions/podman/packages/extension/src/extension.spec.tsextensions/podman/packages/extension/src/extension.tsextensions/podman/packages/extension/src/utils/rosetta.spec.tsextensions/podman/packages/extension/src/utils/rosetta.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- extensions/podman/packages/extension/src/utils/rosetta.spec.ts
- extensions/podman/packages/extension/src/extension.spec.ts
gastoner
marked this pull request as draft
April 2, 2026 07:54
gastoner
marked this pull request as ready for review
April 2, 2026 08:35
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
extensions/podman/packages/extension/src/utils/rosetta.ts (1)
111-116:⚠️ Potential issue | 🟠 MajorBubble Rosetta provisioning failures up to the caller.
Returning
falsehere makes SSH/touch failures indistinguishable from “already configured”, sostartMachine()andcreateMachine(... --now)can keep going and report success even when Rosetta is still inactive.🔧 Suggested change
try { fileCreated = await enableRosettaInMachine(machineName, vmType, podmanConfiguration, podmanVersion); } catch (err) { - console.warn(`Failed to provision Rosetta enable file: ${err}`); - return false; + throw new Error(`Failed to provision Rosetta enable file for ${machineName}: ${String(err)}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/podman/packages/extension/src/utils/rosetta.ts` around lines 111 - 116, The catch block around enableRosettaInMachine currently swallows errors and returns false, making provisioning failures indistinguishable from a no-op; change this so provisioning errors bubble to the caller (startMachine / createMachine) by removing the swallow-and-return-false behavior — either remove the try/catch or rethrow the caught error (or throw a new Error with contextual info) from the catch in the function that calls enableRosettaInMachine so callers can detect and handle real Rosetta provisioning failures.extensions/podman/packages/extension/src/extension.ts (1)
2202-2221:⚠️ Potential issue | 🟠 MajorDon't downgrade create-time Rosetta failures to warnings after mutating VM state.
This
catchwraps branches that may already have started or restarted the VM. If a laterstart/stopfails,createMachine()still resolves even though the machine can be left in the wrong final state.🔧 Suggested change
if (extensionApi.env.isMac && version) { - try { - if (params['podman.factory.machine.now']) { - await provisionAndRestartForRosetta(machineName, provider, podmanConfiguration, version, { logger }); - } else { - // Machine is stopped. Check conditions without SSH first; only start/stop - // the machine temporarily when actually needed. - const setupNeeded = await needsRosettaEnableFile(podmanConfiguration, version, provider); - if (setupNeeded) { - await execPodman(['machine', 'start', machineName], provider, { logger }); - try { - await execPodman(['machine', 'ssh', machineName, `sudo touch ${ROSETTA_ENABLE_FILE}`], provider); - } finally { - await execPodman(['machine', 'stop', machineName], provider); - } - } - } - } catch (err) { - console.warn(`Failed to set up Rosetta enable file during machine creation: ${err}`); + if (params['podman.factory.machine.now']) { + await provisionAndRestartForRosetta(machineName, provider, podmanConfiguration, version, { logger }); + } else { + const setupNeeded = await needsRosettaEnableFile(podmanConfiguration, version, provider); + if (setupNeeded) { + await execPodman(['machine', 'start', machineName], provider, { logger }); + try { + await execPodman(['machine', 'ssh', machineName, `sudo touch ${ROSETTA_ENABLE_FILE}`], provider, { + logger, + }); + } finally { + await execPodman(['machine', 'stop', machineName], provider, { logger }); + } + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/podman/packages/extension/src/extension.ts` around lines 2202 - 2221, The current try/catch around the Rosetta setup swallows errors that can leave the VM in a mutated state (started/stopped) during provisionAndRestartForRosetta, needsRosettaEnableFile, and execPodman calls; change it so errors are not downgraded to warnings—either narrow the try/catch to only non-mutating checks or, if you keep this outer try/catch, log the error (using console.warn or logger) and then rethrow it so createMachine() (or its caller) can fail and handle rollback; ensure this applies to provisionAndRestartForRosetta, needsRosettaEnableFile, and the execPodman start/stop/ssh sequences.
🧹 Nitpick comments (1)
extensions/podman/packages/extension/src/extension.spec.ts (1)
1015-1084: Pin these create-time Rosetta tests toVMTYPE.APPLEHV.
createMachineBaseParamsnever selects the Apple provider, so this suite can still pass while exercising the default macOS provider path. Please set the provider explicitly and assert that exact value is forwarded to the Rosetta helpers.🧪 Suggested change
const createMachineBaseParams = { 'podman.factory.machine.cpus': '2', 'podman.factory.machine.memory': '1048000000', 'podman.factory.machine.diskSize': '250000000000', 'podman.factory.machine.image': 'path', + 'podman.factory.machine.provider': VMTYPE.APPLEHV, }; @@ expect(provisionAndRestartForRosetta).toHaveBeenCalledWith( 'podman-machine-default', - expect.any(String), + VMTYPE.APPLEHV, podmanConfiguration, '5.7.0', expect.anything(), ); @@ - expect(needsRosettaEnableFile).toHaveBeenCalledWith(podmanConfiguration, '5.7.0', expect.any(String)); + expect(needsRosettaEnableFile).toHaveBeenCalledWith(podmanConfiguration, '5.7.0', VMTYPE.APPLEHV);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/podman/packages/extension/src/extension.spec.ts` around lines 1015 - 1084, The tests for createMachine Rosetta provisioning don't pin the VM provider so they may pass without exercising the Apple Hypervisor path; update createMachineBaseParams in this spec to include the vmType/provider key set to VMTYPE.APPLEHV (use the same symbol or constant used in production code), and adjust assertions to expect that vmType value is forwarded to needsRosettaEnableFile and provisionAndRestartForRosetta when calling extension.createMachine; ensure the tests reference createMachineBaseParams, and verify calls to needsRosettaEnableFile and provisionAndRestartForRosetta include the exact VMTYPE.APPLEHV string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@extensions/podman/packages/extension/src/extension.ts`:
- Around line 2202-2221: The current try/catch around the Rosetta setup swallows
errors that can leave the VM in a mutated state (started/stopped) during
provisionAndRestartForRosetta, needsRosettaEnableFile, and execPodman calls;
change it so errors are not downgraded to warnings—either narrow the try/catch
to only non-mutating checks or, if you keep this outer try/catch, log the error
(using console.warn or logger) and then rethrow it so createMachine() (or its
caller) can fail and handle rollback; ensure this applies to
provisionAndRestartForRosetta, needsRosettaEnableFile, and the execPodman
start/stop/ssh sequences.
In `@extensions/podman/packages/extension/src/utils/rosetta.ts`:
- Around line 111-116: The catch block around enableRosettaInMachine currently
swallows errors and returns false, making provisioning failures
indistinguishable from a no-op; change this so provisioning errors bubble to the
caller (startMachine / createMachine) by removing the swallow-and-return-false
behavior — either remove the try/catch or rethrow the caught error (or throw a
new Error with contextual info) from the catch in the function that calls
enableRosettaInMachine so callers can detect and handle real Rosetta
provisioning failures.
---
Nitpick comments:
In `@extensions/podman/packages/extension/src/extension.spec.ts`:
- Around line 1015-1084: The tests for createMachine Rosetta provisioning don't
pin the VM provider so they may pass without exercising the Apple Hypervisor
path; update createMachineBaseParams in this spec to include the vmType/provider
key set to VMTYPE.APPLEHV (use the same symbol or constant used in production
code), and adjust assertions to expect that vmType value is forwarded to
needsRosettaEnableFile and provisionAndRestartForRosetta when calling
extension.createMachine; ensure the tests reference createMachineBaseParams, and
verify calls to needsRosettaEnableFile and provisionAndRestartForRosetta include
the exact VMTYPE.APPLEHV string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32a70281-e908-43f3-b5ec-190415ae7b18
📒 Files selected for processing (4)
extensions/podman/packages/extension/src/extension.spec.tsextensions/podman/packages/extension/src/extension.tsextensions/podman/packages/extension/src/utils/rosetta.spec.tsextensions/podman/packages/extension/src/utils/rosetta.ts
benoitf
requested changes
Apr 8, 2026
benoitf
left a comment
benoitf
left a comment
Collaborator
There was a problem hiding this comment.
we've enabled inversify in this extension so we should not bring any exported function but only bring injection of objects / classes
Contributor
Author
|
@benoitf updated |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
extensions/podman/packages/extension/src/utils/rosetta.ts (1)
120-125:⚠️ Potential issue | 🟠 MajorPropagate Rosetta provisioning failures instead of returning
false.This catch makes "SSH/touch failed" indistinguishable from "already configured", so callers can continue and report a healthy machine while Rosetta is still inactive. Let this reject, or return a distinct failure state that the caller must surface.
Suggested change
try { fileCreated = await this.enableRosettaInMachine(machineName, vmType, podmanConfiguration, podmanVersion); } catch (err) { - console.warn(`Failed to provision Rosetta enable file: ${err}`); - return false; + throw new Error(`Failed to provision Rosetta enable file for ${machineName}: ${String(err)}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/podman/packages/extension/src/utils/rosetta.ts` around lines 120 - 125, The catch in the Rosetta provisioning call swallows errors and returns false, making "SSH/touch failed" indistinguishable from "already configured"; update the error handling in the block that calls enableRosettaInMachine (the try/catch around fileCreated = await this.enableRosettaInMachine(...)) to either rethrow the caught error (throw err) so callers can surface a provisioning failure, or return a distinct failure value/object (e.g., { success: false, reason: err }) and update callers accordingly; ensure you reference enableRosettaInMachine and the surrounding provisioning routine so callers can differentiate and report Rosetta provisioning failures.extensions/podman/packages/extension/src/extension.ts (1)
2206-2225:⚠️ Potential issue | 🟠 MajorDon’t downgrade create-time Rosetta setup failures to a warning after the VM state changes.
Once this block starts the machine or calls
provisionAndRestartForRosetta(), a failed restart/stop can leave the VM in the wrong final state whilecreateMachine()still resolves successfully. Re-throw after any best-effort rollback so callers don’t treat a partially configured machine as ready.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/podman/packages/extension/src/extension.ts` around lines 2206 - 2225, The catch block currently logs failures when setting up Rosetta during machine creation but swallows the error, allowing createMachine (and callers) to treat a partially-configured VM as successful; instead, after performing any best-effort rollback/cleanup (e.g., the existing finally that stops the machine after execPodman calls), re-throw the caught error so callers observe the failure. Concretely: in the try/catch around rosettaProvisioner.provisionAndRestartForRosetta and execPodman calls, replace the console.warn in the catch with a logger.error (or keep the warn) and then throw err (re-throw) so failures in rosettaProvisioner.provisionAndRestartForRosetta, execPodman(['machine', 'start'...]), or the SSH touch are propagated to callers of createMachine; ensure you don’t remove existing cleanup finally blocks (e.g., the inner finally that calls execPodman(['machine','stop',...])).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@extensions/podman/packages/extension/src/extension.ts`:
- Around line 2206-2225: The catch block currently logs failures when setting up
Rosetta during machine creation but swallows the error, allowing createMachine
(and callers) to treat a partially-configured VM as successful; instead, after
performing any best-effort rollback/cleanup (e.g., the existing finally that
stops the machine after execPodman calls), re-throw the caught error so callers
observe the failure. Concretely: in the try/catch around
rosettaProvisioner.provisionAndRestartForRosetta and execPodman calls, replace
the console.warn in the catch with a logger.error (or keep the warn) and then
throw err (re-throw) so failures in
rosettaProvisioner.provisionAndRestartForRosetta, execPodman(['machine',
'start'...]), or the SSH touch are propagated to callers of createMachine;
ensure you don’t remove existing cleanup finally blocks (e.g., the inner finally
that calls execPodman(['machine','stop',...])).
In `@extensions/podman/packages/extension/src/utils/rosetta.ts`:
- Around line 120-125: The catch in the Rosetta provisioning call swallows
errors and returns false, making "SSH/touch failed" indistinguishable from
"already configured"; update the error handling in the block that calls
enableRosettaInMachine (the try/catch around fileCreated = await
this.enableRosettaInMachine(...)) to either rethrow the caught error (throw err)
so callers can surface a provisioning failure, or return a distinct failure
value/object (e.g., { success: false, reason: err }) and update callers
accordingly; ensure you reference enableRosettaInMachine and the surrounding
provisioning routine so callers can differentiate and report Rosetta
provisioning failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38881b61-0c35-42bb-9b75-beff1d59d4ef
📒 Files selected for processing (6)
extensions/podman/packages/extension/src/extension.spec.tsextensions/podman/packages/extension/src/extension.tsextensions/podman/packages/extension/src/inject/inversify-binding.spec.tsextensions/podman/packages/extension/src/inject/inversify-binding.tsextensions/podman/packages/extension/src/utils/rosetta.spec.tsextensions/podman/packages/extension/src/utils/rosetta.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- extensions/podman/packages/extension/src/extension.spec.ts
- extensions/podman/packages/extension/src/utils/rosetta.spec.ts
benoitf
reviewed
Apr 9, 2026
| } | ||
|
|
||
| // name at the end | ||
| let machineName = 'podman-machine-default'; |
Collaborator
There was a problem hiding this comment.
I don't see why we need to get the default machine name
the command 'podman machine ssh' works without provide the default name of the machine
if podman cli decides to rename the default machine name from 'podman-machine-default' to 'default' we're screwed here
I think it should provide the name of the machine later only if it's defined, else it should assume the default naming
Contributor
Author
There was a problem hiding this comment.
Removed the default name
Contributor
Author
There was a problem hiding this comment.
Forgot to push it :D
Collaborator
There was a problem hiding this comment.
@gastoner I think it should not be empty machine name but when you call the 'podman machine ssh' the element should not be there
like you prepare the argument, if there is a custom machine name, you add the machine name to the array else you keep the arguments
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/podman/packages/extension/src/extension.ts`:
- Around line 2156-2158: The code sets machineName = '' when the param is absent
and then always passes it to podman commands, causing podman to receive an
empty-string argument instead of omitting the option; change the
command-argument construction so the machineName string is only included/spread
into the args array when machineName is truthy (non-empty). Locate uses of the
machineName variable around the podman machine command invocations (the places
that build args for "podman machine start"/"stop"/etc.) and conditionally
push/spread machineName into those args arrays (and apply the same conditional
pattern to the earlier occurrence where machineName is used) so that when
machineName === '' the argument is omitted entirely.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39f56f09-4b9a-4b4b-8123-71a75d3aca08
📒 Files selected for processing (1)
extensions/podman/packages/extension/src/extension.ts
amisskii
approved these changes
Apr 10, 2026
amisskii
left a comment
amisskii
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Functionality tested only
benoitf
approved these changes
Apr 13, 2026
podman-desktop-triager
Bot
added
domain/containers/reviewed
and removed
domain/containers/inreview
labels
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
Signed-off-by: Evzen Gasta <evzen.ml@seznam.cz>
Contributor
Author
|
Rebasing based on latest main |
gastoner
enabled auto-merge (squash)
April 13, 2026 17:45
dgolovin
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR adds a fix from https://blog.podman.io/2025/08/podman-5-6-released-rosetta-status-update/ to creation/starting of podman machine on macOS Tahoe with podman 5.6+
Screenshot / video of UI
What issues does this PR fix or reference?
Closes #16168
How to test this PR?
Create podman machine from podman desktop (production version) - applev provider
Start the machine, run
podman machine ssh "cat /proc/sys/fs/binfmt_misc/rosetta"(for podman-machine-default) - you should seecat: /proc/sys/fs/binfmt_misc/rosetta: No such file or directoryRun PD from this PR, try to stop/start the machine
run
podman machine ssh "cat /proc/sys/fs/binfmt_misc/rosetta"You should see: ±
Delete the machine, create new machine (also with appleV)
run the command again, you should see the same output