fix(qemu): force TCG for arm64 CHR on all Apple Silicon, add --accel override

[mobileskyfi]

Refixes #97. The FEAT_SSBS=0 → TCG fallback merged in #98 keyed on the wrong axis. It is still unreleased (last tag v0.4.5), so this corrects it in place rather than deprecating it.

SSBS was a coincident marker, not the mechanism

M4 is the first Apple chip to report hw.optional.arm.FEAT_SSBS = 0, which is why the reporter's diagnostics correlated. But Linux 5.6 treats SSBS as an optional mitigation and boots fine without it — cortex-a53 and neoverse-n1 don't implement it either. SSBS-absence cannot produce No working init found.

Actual root cause: the CHR image requires AArch32, Apple Silicon has none

Current arm64 CHR images pair an AArch64 kernel with a 32-bit ARM userspace:

CHR image	/init type
7.20.8 arm64	ELF 32-bit LSB ARM, EABI5, static
7.22.1 arm64	ELF 32-bit LSB ARM, EABI5, static
7.23beta5 arm64	ELF 32-bit LSB ARM, EABI5, static

The 7.22.1 system package holds 101 more ARM32 executables and 18 ARM32 shared objects; the only AArch64 executables are kexec and vmcore-dmesg. An AArch64 /init alone would not fix it — the failure would just move later.

Apple Silicon implements no AArch32 at any exception level (ID_AA64PFR0_EL1 is AArch64-only for EL0/EL1), and QEMU under HVF passes that hardware register straight through — hvf_arch_init_vcpu() re-reads the live vCPU register and edits only the GIC bit, so the -cpu model is inert. The guest kernel never sets ARM64_HAS_32BIT_EL0, compat_elf_check_arch() rejects the EM_ARM /init with -ENOEXEC, the initramfs has no fallback init, and Linux panics at t≈0.076 s.

Confirmed by the guest's own panic-time capability bitmap, decoded against Linux 5.6 cpucaps.h (ARM64_HAS_32BIT_EL0 == 13):

Guest	Bitmap	Caps	ARM64_HAS_32BIT_EL0
M4, -accel hvf -cpu host — panics	0x20012,28000230	8	absent
-accel tcg -cpu cortex-a710 — boots	0x20013,28402230	11	present

The failing set is a strict subset; the only other differences are ARM64_SVE and ARM64_HAS_STAGE2_FWB, neither of which participates in execve().

Consequences for the fix

1. Scope is every Apple Silicon generation, not M4+. No Apple CPU since 2020 implements AArch32, so the SSBS predicate left M1/M2/M3 on HVF and panicking — a live bug, not merely a narrow gate.
2. A QEMU version floor is not a valid restore signal. Hypervisor.framework exposes only hv_vcpu_config_get_feature_reg() — there is no setter — so no macOS VMM can present a feature the silicon lacks (UTM's fork behaves identically). The deferred getQemuVersion() guard is dropped, not postponed.
3. The restore signal is the guest artifact: a future arm64 CHR whose appended /init and required system-package executables/shared objects are all AArch64, confirmed by a real HVF boot. That check is mechanical and could become a release-time guard.
4. Standard server ARM (Ampere, Graviton) does implement AArch32 EL0, which is why arm64 CHR runs there under KVM. This is an Apple-Silicon-plus-CHR-artifact interaction, not a RouterOS-on-ARM defect.

Full chain, evidence table, and reproduction: docs/m4-hvf-arm64-investigation.md. Reported upstream to MikroTik (request: ship an ARM32-free arm64 userspace, or document that the image requires AArch32 EL0 despite its AArch64 kernel).

Second bug found while fixing this: x86 CHR was broken on Apple Silicon

Not a slowdown — a hard failure, pre-existing and unrelated to SSBS. detectAccel("x86") returned hvf whenever kern.hv_support=1, including on Apple Silicon, so qemu-system-x86_64 was launched with -accel hvf on an arm64 host. HVF is compiled into a QEMU binary only when the emulated target matches the physical host, so that binary has no hvf accelerator at all and QEMU exits.

Verified by the mirror-image test on an Intel Mac, where the same asymmetry is observable locally:

$ qemu-system-x86_64 -accel help     # target matches host
Accelerators supported in QEMU binary:
tcg
hvf
$ qemu-system-aarch64 -accel help    # target does not match host
Accelerators supported in QEMU binary:
tcg

detectAccel now selects TCG for both guest arches on Apple Silicon, and isAppleSiliconHost() uses sysctl.proc_translated so a Rosetta process (which reports process.arch === "x64") is still recognized as an arm64 host. isCrossArchEmulation() became symmetric as a result, so x86-on-arm64 finally gets the cross-arch timeout factor.

Repo docs already disagreed with the code here — qemu.instructions.md said x86 CHR "must use accel=tcg" on Apple Silicon while DESIGN.md's table claimed HVF. The table was wrong; both now match the code.

Changes

• detectAccel("arm64") returns tcg on all macOS hosts. hostLacksSsbs()/ssbsTcgWarning() → isAppleSiliconHost()/accelNote().
• New escape hatch — --accel <auto|tcg|hvf|kvm> on start/add, plus an accel setting and QUICKCHR_ACCEL env var (precedence: flag > env > quickchr.env > auto, matching every other setting). Anything but auto is passed to QEMU verbatim and bypasses detection entirely, so HVF can be tested against a future AArch64-only image with no code change. Forcing --accel hvf for an arm64 guest on Apple Silicon still prints the panic caveat rather than obeying silently.
• doctor marks the acceleration row when an override is in effect (otherwise the row reads as a capability report it isn't), and the launch note names the tier that set the accelerator (--accel / QUICKCHR_ACCEL / quickchr.env) — a stale accel=tcg in the settings file is otherwise an unexplained slowdown.
• integration.yml's header claimed macos-arm64 → arm64 CHR HVF and printed Accel hint: HVF (expected). Both were false and are corrected — see Verification.
• Docs corrected: DESIGN.md fix(ci): kill orphaned QEMU processes on macOS-x86 step timeout to prevent runner death #10, CHANGELOG.md (replaced the unreleased SSBS entry rather than stacking a correction on it), MANUAL.md, .github/instructions/qemu.instructions.md, .github/copilot-instructions.md, and the investigation doc's implications section.

One structural change worth a look

Wiring platform.ts to settings.ts created an import cycle (settings → cache → state → network → platform) that loaded eagerly on every CLI start and pushed cli-settings.test.ts past its 5 s timeout. The quickchr.env file tier is extracted into a leaf module src/lib/settings-file.ts (node:fs + paths.ts only); settings.ts re-exports it, so the public API is unchanged. platform.ts imports the leaf, not settings.ts.

Verification

Local (Intel x86_64 Mac):

• bun run check clean — Biome, tsc --noEmit, markdownlint, cspell, examples, shellcheck.
• bun test test/unit/ — 719 pass, 18 skip, 0 fail.
• Real x86 CHR booted and answered REST under both auto (-accel hvf) and --accel tcg (-accel tcg,tb-size=256), confirming the override reaches QEMU's argv and the launch path is intact.
• The Apple Silicon branch is exercised by mocking process.platform/process.arch.
• QUICKCHR_INTEGRATION=1 bun test over start-stop, library-api, forward-cli, settings-secure-login-cli — 14 pass, 0 fail (326 s, real CHR boots).

CI cannot validate the HVF path, and its docs claimed otherwise. The macos-arm64 gating leg runs on hosted macos-15 runners that are themselves VMs (Apple M1 (Virtual)) reporting kern.hv_support=0; its own platform log reads quickchr detectAccel(arm64): tcg (run 29669706663). So that leg has never exercised arm64 HVF, and a green macOS run is not evidence about #97 either way. The workflow header said it was HVF; that's fixed in this PR so the next reader isn't misled.

Grounding boundary, unchanged and important: this is not reproduced locally — dev and CI hosts are Intel x86_64, and the failure needs Apple Silicon. The M4 evidence is the reporter's (tikoci/mikropkl#11), and the exact Failed to execute /init (error -8) line remains unobserved. What's verified here is accelerator selection, the override, and the x86 boot path. A reviewer on Apple Silicon confirming that --accel hvf on an arm64 guest still panics — and that the default TCG path boots — would close the last gap.

Refs #97. Downstream: tikoci/mikropkl#11.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added accelerator controls through the --accel option, QUICKCHR_ACCEL, and the accel setting.
  • Supported modes include automatic selection, TCG, HVF, and KVM, with clear override precedence.
  • Added status messaging when acceleration is explicitly configured.

• Bug Fixes

  • Apple Silicon now reliably uses TCG for CHR guests where HVF is incompatible, improving boot reliability.

• Documentation

  • Updated manuals, design guidance, changelog, and troubleshooting information for acceleration behavior and overrides.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3031203d-c9ef-4a97-81ed-bd4c65032155

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR adds accelerator overrides through CLI, settings, and environment variables; forces TCG for macOS Apple Silicon guests; replaces SSBS-based detection; updates launch diagnostics, tests, documentation, and integration guidance.

Changes

Accelerator policy

Layer / File(s)	Summary
Accelerator setting and settings-file contract src/lib/types.ts, src/lib/settings-file.ts, src/lib/settings.ts, test/unit/*	Adds validated accelerator modes, settings-file parsing, the managed accel setting, and updated settings output tests.
Override resolution and Apple Silicon detection src/lib/platform.ts, test/unit/platform.test.ts	Adds override precedence and Apple Silicon detection; macOS auto-selection uses TCG while explicit modes bypass detection.
CLI and launch-path integration src/cli/index.ts, src/lib/quickchr.ts, test/unit/cli-start-flag-resolution.test.ts	Applies --accel to add and start, updates help text, emits accelerator notes, and reports forced modes in diagnostics.
Policy documentation and validation .github/*, .github/workflows/*, CHANGELOG.md, DESIGN.md, MANUAL.md, docs/*, test/integration/*, project-words.txt	Documents accelerator precedence, Apple Silicon TCG behavior, boot timeout guidance, and the implemented mitigation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• tikoci/quickchr#60 — Provides the settings framework extended with the accelerator setting.
• tikoci/quickchr#98 — Contains the superseded SSBS-based fallback replaced by this policy.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main change: forcing TCG for arm64 CHR on Apple Silicon and adding an accel override.
Linked Issues check	✅ Passed	The PR implements #97 by defaulting arm64 CHR on Apple Silicon to TCG and adding explicit accel overrides for testing.
Out of Scope Changes check	✅ Passed	The changes stay within accelerator selection, settings, docs, and tests; no unrelated functionality was added.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/arm64-hvf-apple-silicon-aarch32

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR updates quickchr’s QEMU acceleration selection to avoid HVF for arm64 CHR on Apple Silicon (forcing TCG by default), and introduces a process-wide accelerator override (--accel, QUICKCHR_ACCEL, and quickchr.env setting) so users can explicitly bypass detection when needed (e.g., to test future images).

Changes:

• Force detectAccel("arm64") to return tcg on macOS, and replace SSBS-based messaging with an Apple-Silicon/AArch32 explanation via accelNote().
• Add an accelerator override chain (flag > env > settings file > auto) with validation (AccelMode / parseAccelMode) and CLI wiring (--accel).
• Break out quickchr.env file-tier logic into a leaf module (src/lib/settings-file.ts) to avoid import cycles and startup cost.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/lib/platform.ts	Adds accel override resolution + new accelNote(), forces arm64-on-macOS to TCG, updates host logic helpers.
src/cli/index.ts	Adds --accel flag handling (applyAccelFlag) and help text updates.
src/lib/types.ts	Introduces AccelMode, ACCEL_MODES, and parseAccelMode() validator.
src/lib/settings.ts	Adds the accel managed setting and re-exports file-tier helpers from the new leaf module.
src/lib/settings-file.ts	New leaf module for quickchr.env path/read/parse to avoid platform/settings import cycles.
src/lib/quickchr.ts	Switches to accelNote() and annotates doctor output when accel is forced.
test/unit/platform.test.ts	Updates acceleration tests for the new Apple Silicon behavior + override semantics.
test/unit/settings.test.ts	Updates expectations for the new 6th settings key (accel).
test/unit/cli-settings.test.ts	Updates CLI settings print/JSON expectations to include accel.
MANUAL.md	Documents the new accel setting and its precedence/behavior.
DESIGN.md	Updates decision #10 to the AArch32 userspace root cause and documents the override.
CHANGELOG.md	Adds --accel/setting/env and replaces the unreleased SSBS-based entry with the widened fix.
docs/m4-hvf-arm64-investigation.md	Updates implications section to reflect implemented fix/override and corrected root cause.
.github/instructions/qemu.instructions.md	Updates acceleration rules to reflect Apple Silicon arm64 TCG + override chain.
.github/copilot-instructions.md	Updates project QEMU rules summary to include the Apple Silicon arm64 TCG rule + override.
project-words.txt	Adds new vocabulary (pfr, vmm) for cSpell.

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/copilot-instructions.md:
- Line 46: Update the arm64 macOS acceleration guidance in the documentation to
state that automatic selection uses TCG with “-cpu cortex-a710,” rather than
claiming it always does. Preserve the documented behavior that explicit --accel
or QUICKCHR_ACCEL values, including hvf or kvm, override the automatic policy.

In `@MANUAL.md`:
- Around line 401-412: Update the managed-key count reference near the
settings/environment-variable documentation from five to six, keeping the
surrounding explanation unchanged.

In `@src/lib/platform.ts`:
- Around line 200-205: Update accelNote in src/lib/platform.ts (lines 200-205)
and the doctor suffix in src/lib/quickchr.ts (lines 2008-2015) to use consistent
source-neutral wording such as “configured override,” or propagate resolver
provenance so messages accurately cover flag, environment, and quickchr.env
overrides under the existing flag > environment > quickchr.env precedence.

In `@src/lib/settings-file.ts`:
- Around line 13-31: Replace Node filesystem and path usage in
src/lib/settings-file.ts lines 13-31 and src/lib/settings.ts lines 16-17 with
Bun filesystem APIs and Bun path helpers, preserving existing settings behavior.
Update the atomic write flow in src/lib/settings.ts lines 182-212 to use Bun
equivalents for directory creation, writing, copying, syncing, closing, and
removal, and wrap any write, copy, fsync, or rename failure in QuickCHRError.

In `@src/lib/settings.ts`:
- Around line 105-109: Update the managed-key list used by the quickchr settings
help in the CLI entrypoint to include accel, documenting its supported AccelMode
values and the builtin default "auto". Use the existing settings metadata or
formatting conventions in the managed-key list rather than changing the settings
definition in parseAccelMode or the accel configuration.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: faf93995-d5e3-4e87-9993-aa2fd9967cc5

📥 Commits

Reviewing files that changed from the base of the PR and between 3e456d3 and 09059b8.

📒 Files selected for processing (16)

• .github/copilot-instructions.md
• .github/instructions/qemu.instructions.md
• CHANGELOG.md
• DESIGN.md
• MANUAL.md
• docs/m4-hvf-arm64-investigation.md
• project-words.txt
• src/cli/index.ts
• src/lib/platform.ts
• src/lib/quickchr.ts
• src/lib/settings-file.ts
• src/lib/settings.ts
• src/lib/types.ts
• test/unit/cli-settings.test.ts
• test/unit/platform.test.ts
• test/unit/settings.test.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (3)

• GitHub Check: copilot-pull-request-reviewer
• GitHub Check: Unit Tests (windows-latest)
• GitHub Check: Analyze (javascript-typescript)

🧰 Additional context used

📓 Path-based instructions (6)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

In Bun-based TypeScript code, use Bun.spawn(), Bun.write(), Bun.sleep(), bun:test, and ESM imports with .ts extensions.

**/*.{ts,tsx}: Use Bun APIs and bun:test; do not use Node.js APIs or CommonJS. Use ESM with .ts extensions in imports.
For ARM64 virt machines, never use if=virtio for drives; use explicit -device virtio-blk-pci,drive=drive0.
When using HVF acceleration, use -cpu host, not cortex-a710.
Arm64 guests on macOS must use TCG with -cpu cortex-a710; --accel and QUICKCHR_ACCEL override this for testing.
UEFI pflash code and vars units must be identical in size.
QGA is x86-only; do not expect the guest agent to start for arm64 CHR.
Use Biome 2.x for linting, with tabs for indentation, and avoid unnecessary comments on obvious code.
Throw QuickCHRError(code, message, installHint?) for errors.

Files:

• test/unit/settings.test.ts
• src/lib/types.ts
• src/lib/settings-file.ts
• test/unit/cli-settings.test.ts
• src/lib/quickchr.ts
• src/lib/settings.ts
• src/lib/platform.ts
• test/unit/platform.test.ts
• src/cli/index.ts

test/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not turn a red integration test green by broadening timeouts, skipping it, or platform-gating it before reproducing and root-causing the failure.

Files:

• test/unit/settings.test.ts
• test/unit/cli-settings.test.ts
• test/unit/platform.test.ts

test/unit/**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Unit tests must be fast and must not require QEMU.

Files:

• test/unit/settings.test.ts
• test/unit/cli-settings.test.ts
• test/unit/platform.test.ts

src/**

📄 CodeRabbit inference engine (CLAUDE.md)

Follow the rules in general.instructions.md for files under src/**, including layer boundaries, error-code usage, port layout, and the RouterOS “expired admin” caveat.

Files:

• src/lib/types.ts
• src/lib/settings-file.ts
• src/lib/quickchr.ts
• src/lib/settings.ts
• src/lib/platform.ts
• src/cli/index.ts

src/lib/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Keep src/lib/ as pure library code: do not import from src/cli/ and do not call process.exit() there.

Keep src/lib/ as pure library modules: no CLI dependencies and no process.exit().

Files:

• src/lib/types.ts
• src/lib/settings-file.ts
• src/lib/quickchr.ts
• src/lib/settings.ts
• src/lib/platform.ts

src/cli/**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Keep src/cli/ as a thin CLI wrapper over the library.

Files:

• src/cli/index.ts

🧠 Learnings (1)

📓 Common learnings

Learnt from: CR
Repo: tikoci/quickchr

Timestamp: 2026-07-27T01:43:38.312Z
Learning: Keep `SKILL.md` and `references/quickchr-api.md` aligned with the repository, and update the relevant shared skill when QEMU/CHR behavior changes.

🔇 Additional comments (12)

.github/copilot-instructions.md (1)

46-46: 📐 Maintainability & Code Quality

Update the paired shared skill documentation.

This QEMU/CHR policy change also needs the corresponding routeros-qemu-chr / routeros-quickchr skill updates, or a linked follow-up if those live in a separate repository. Based on learnings, keep SKILL.md and references/quickchr-api.md aligned when QEMU/CHR behavior changes.

Sources: Coding guidelines, Learnings

CHANGELOG.md (1)

11-20: LGTM!

Also applies to: 22-42

docs/m4-hvf-arm64-investigation.md (1)

576-615: LGTM!

project-words.txt (1)

275-275: LGTM!

Also applies to: 430-430

.github/instructions/qemu.instructions.md (1)

37-39: 🎯 Functional Correctness

No documentation change needed.

.github/instructions/qemu.instructions.md documents Apple Silicon x86 CHR as TCG, while DESIGN.md documents Apple Silicon x86 CHR as HVF, matching the current src/lib/platform.ts:251 behavior that selects hvf for any x86 guest on macOS.

			> Likely an incorrect or invalid review comment.

src/lib/types.ts (1)

16-36: LGTM!

src/lib/settings.ts (1)

23-25: LGTM!

Also applies to: 161-180

test/unit/settings.test.ts (1)

296-299: LGTM!

test/unit/cli-settings.test.ts (1)

51-69: LGTM!

src/lib/platform.ts (1)

219-268: LGTM!

test/unit/platform.test.ts (1)

1-1: LGTM!

Also applies to: 12-16, 64-73, 388-504

src/cli/index.ts (1)

160-173: LGTM!

Also applies to: 553-553, 1194-1195, 2741-2743, 2826-2828

[mobileskyfi]

Independent review at e7091bf:

The AArch32 root-cause chain in docs/m4-hvf-arm64-investigation.md supports the core policy: current arm64 CHR must auto-select TCG on Apple Silicon, while an explicit accelerator value must bypass detection for future-image testing.

I pushed the corrections that were clear from the current code and docs:

• Detect the physical Apple Silicon host under Rosetta via the documented sysctl.proc_translated probe. This fixes the warning path and a larger latent bug: x86 CHR was still auto-selecting impossible x86/HVF on Apple Silicon. Both guest architectures now auto-select TCG there; Intel x86 keeps HVF.
• Correct cross-architecture timeout classification for x86-on-arm64.
• Make repeated --accel values use the last value and reject a missing value instead of silently ignoring it.
• Use source-neutral configured-override wording for flag, environment, and quickchr.env tiers.
• Align MANUAL.md, DESIGN.md, the QEMU instructions, settings help, managed-key counts, changelog, and integration-test comments. The manual previously still told native Apple Silicon users they would get arm64/HVF.
• Add focused unit coverage for native Apple Silicon, Rosetta, Intel macOS, repeated/missing flags, and settings help.

Validation on the Intel maintainer host:

• bun run check: clean
• bun test test/unit/: 722 pass, 18 platform skips, 0 fail
• live CHR start-stop under forced TCG: pass (36.9 s)
• live CHR start-stop under Intel auto/HVF: pass (24.1 s)

Release boundary: I would not publish the new release until one native Apple Silicon run confirms the actual quickchr path: default arm64 start emits -accel tcg,tb-size=256 and boots, while --accel hvf reaches QEMU (and is expected to retain the current-image panic caveat). The artifact/source evidence is strong, but this repo's own Apple Silicon launch path still has no live run.

Larger follow-ups, not folded into this PR:

1. Update tikoci/routeros-skills before or with the release. The public routeros-qemu-chr acceleration example still allows matching arm64/HVF and does not record the CHR AArch32 artifact constraint; routeros-quickchr also carries the older acceleration boundary.
2. Decide whether default-arch=auto under Rosetta should mean the physical arm64 host instead of the translated process's x86 architecture. This PR now chooses the correct x86/TCG accelerator, but changing the default guest architecture also touches wizard/default semantics, binary/firmware resolution, and needs a real Apple Silicon test.
3. Rewrite or close issue #97 with the corrected AArch32 explanation when this lands; its body still leads with the superseded SSBS theory, while the correction currently lives in a later comment.

All seven bot threads were reviewed, replied to, and resolved. Six findings were fixed. I declined the requested wholesale Node-to-Bun settings I/O rewrite: the synchronous atomic-write implementation predates this split, node:fs/node:path are supported and used throughout this Bun repo, and replacing the fsync/rename sequence is a separate behavioral refactor rather than a correction to this PR.

[mobileskyfi]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/unit/platform.test.ts`:
- Around line 494-508: Add the same spawnSync mock used by the “false on an
Intel Mac” isAppleSiliconHost test before the darwin/x64 accelNote("arm64",
"tcg") assertion, ensuring the sysctl probe reports a non-translated Intel host
and the test remains environment-independent.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d48f037d-76e0-4a9f-96ed-842940efc2fe

📥 Commits

Reviewing files that changed from the base of the PR and between 3e456d3 and 195dbb6.

📒 Files selected for processing (19)

• .github/copilot-instructions.md
• .github/instructions/qemu.instructions.md
• .github/workflows/integration.yml
• CHANGELOG.md
• DESIGN.md
• MANUAL.md
• docs/m4-hvf-arm64-investigation.md
• project-words.txt
• src/cli/index.ts
• src/lib/platform.ts
• src/lib/quickchr.ts
• src/lib/settings-file.ts
• src/lib/settings.ts
• src/lib/types.ts
• test/integration/start-stop.test.ts
• test/unit/cli-settings.test.ts
• test/unit/cli-start-flag-resolution.test.ts
• test/unit/platform.test.ts
• test/unit/settings.test.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (8)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

In Bun-based TypeScript code, use Bun.spawn(), Bun.write(), Bun.sleep(), bun:test, and ESM imports with .ts extensions.

**/*.{ts,tsx}: Use Bun APIs and bun:test rather than Node.js APIs, CommonJS, or other test frameworks; use .ts extensions in ESM imports.
For ARM64 (virt) QEMU machines, never use if=virtio for drives; use an explicit -device virtio-blk-pci,drive=drive0 device instead.
When using HVF acceleration, use -cpu host, not cortex-a710.
For arm64 guests on macOS, automatically select TCG with -cpu cortex-a710; --accel and QUICKCHR_ACCEL must override this behavior for testing.
UEFI pflash code and vars units must be identical in size.
QGA is x86-only; do not start or configure the guest agent for arm64 CHR guests.
Use tabs for indentation and avoid unnecessary comments on obvious code.
Throw QuickCHRError(code, message, installHint?) for errors.

Files:

• test/unit/settings.test.ts
• test/unit/cli-settings.test.ts
• test/integration/start-stop.test.ts
• src/lib/types.ts
• src/lib/settings-file.ts
• src/lib/quickchr.ts
• src/lib/platform.ts
• src/cli/index.ts
• src/lib/settings.ts
• test/unit/cli-start-flag-resolution.test.ts
• test/unit/platform.test.ts

test/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not turn a red integration test green by broadening timeouts, skipping it, or platform-gating it before reproducing and root-causing the failure.

Files:

• test/unit/settings.test.ts
• test/unit/cli-settings.test.ts
• test/integration/start-stop.test.ts
• test/unit/cli-start-flag-resolution.test.ts
• test/unit/platform.test.ts

test/unit/**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Unit tests must be fast and must not require QEMU.

Files:

• test/unit/settings.test.ts
• test/unit/cli-settings.test.ts
• test/unit/cli-start-flag-resolution.test.ts
• test/unit/platform.test.ts

**/*.{md,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Keep the routeros-qemu-chr and routeros-quickchr shared skills synchronized with quickchr behavior; keep SKILL.md and references/quickchr-api.md aligned with the repository.

Files:

• test/unit/settings.test.ts
• test/unit/cli-settings.test.ts
• test/integration/start-stop.test.ts
• src/lib/types.ts
• src/lib/settings-file.ts
• CHANGELOG.md
• docs/m4-hvf-arm64-investigation.md
• src/lib/quickchr.ts
• MANUAL.md
• src/lib/platform.ts
• src/cli/index.ts
• DESIGN.md
• src/lib/settings.ts
• test/unit/cli-start-flag-resolution.test.ts
• test/unit/platform.test.ts

test/integration/**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Integration tests require QEMU and must be guarded by QUICKCHR_INTEGRATION=1.

Files:

• test/integration/start-stop.test.ts

src/**

📄 CodeRabbit inference engine (CLAUDE.md)

Follow the rules in general.instructions.md for files under src/**, including layer boundaries, error-code usage, port layout, and the RouterOS “expired admin” caveat.

Files:

• src/lib/types.ts
• src/lib/settings-file.ts
• src/lib/quickchr.ts
• src/lib/platform.ts
• src/cli/index.ts
• src/lib/settings.ts

src/lib/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Keep src/lib/ as pure library code: do not import from src/cli/ and do not call process.exit() there.

Keep src/lib/ modules pure: do not add CLI dependencies or call process.exit().

Files:

• src/lib/types.ts
• src/lib/settings-file.ts
• src/lib/quickchr.ts
• src/lib/platform.ts
• src/lib/settings.ts

src/cli/**/*.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Keep src/cli/ as a thin wrapper over the library.

Files:

• src/cli/index.ts

🧠 Learnings (1)

📚 Learning: 2026-07-27T03:48:25.618Z

Learnt from: mobileskyfi
Repo: tikoci/quickchr PR: 99
File: src/lib/settings-file.ts:13-31
Timestamp: 2026-07-27T03:48:25.618Z
Learning: For TypeScript files in src/lib/settings*.ts (notably src/lib/settings.ts and src/lib/settings-file.ts), treat the existing settings subsystem contract as intentional: it uses synchronous node:fs/node:path with the established atomic fsync/rename write sequence. When reviewing changes—especially the extraction of read-only quickchr.env handling into src/lib/settings-file.ts—avoid requesting a Bun-native filesystem conversion as part of that extraction, unless a dedicated behavioral refactor is explicitly in scope. Keep the extraction focused on breaking the import cycle (leaf-module structure), not changing IO semantics.

Applied to files:

• src/lib/settings-file.ts
• src/lib/settings.ts

🔇 Additional comments (19)

.github/copilot-instructions.md (1)

46-46: LGTM!

.github/instructions/qemu.instructions.md (1)

30-39: LGTM!

Also applies to: 92-98

.github/workflows/integration.yml (1)

25-36: LGTM!

Also applies to: 313-316

CHANGELOG.md (1)

11-21: LGTM!

Also applies to: 22-43, 44-47

DESIGN.md (1)

57-63: LGTM!

Also applies to: 111-134

MANUAL.md (1)

401-412: LGTM!

Also applies to: 421-429, 932-945, 1009-1009, 1021-1021

docs/m4-hvf-arm64-investigation.md (1)

576-615: LGTM!

project-words.txt (1)

275-275: LGTM!

Also applies to: 430-430

test/integration/start-stop.test.ts (1)

9-9: LGTM!

Also applies to: 36-37, 78-78

src/lib/types.ts (1)

16-37: LGTM!

src/lib/settings-file.ts (1)

1-52: LGTM! Extraction correctly preserves the existing file-tier contract as a leaf module.

Based on learnings, the synchronous node:fs/node:path implementation here is an established, intentional contract for this extraction — not flagging a Bun-native I/O conversion.

Source: Learnings

src/lib/settings.ts (1)

16-25: LGTM!

Also applies to: 100-110, 126-126, 140-141, 161-213

test/unit/cli-settings.test.ts (1)

51-76: LGTM!

test/unit/settings.test.ts (1)

296-299: LGTM!

src/lib/platform.ts (1)

6-10: LGTM! Override precedence, Apple Silicon detection, and the arm64/x86 TCG fallback logic on macOS are correct and consistent with the documented policy.

Also applies to: 117-121, 142-261, 279-305

test/unit/platform.test.ts (1)

1-1: LGTM! Setup/teardown correctly pins the accel override and Apple Silicon cache per test, and the new detectAccel/isAppleSiliconHost/override suites all match the implementation.

Also applies to: 12-17, 65-76, 328-339, 371-404, 417-482

src/cli/index.ts (1)

160-179: LGTM!

Also applies to: 558-558, 1199-1200, 2746-2748, 2831-2833, 3088-3089

src/lib/quickchr.ts (1)

29-29: LGTM!

Also applies to: 1529-1530, 1843-1844, 2008-2015

test/unit/cli-start-flag-resolution.test.ts (1)

4-19: LGTM!

Also applies to: 30-30, 140-152

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.