feat(mac): native .app bundle (Apple Silicon + macOS 26+) — C1+A#269
Merged
feat(mac): native .app bundle (Apple Silicon + macOS 26+) — C1+A#269
Conversation
…/20260414, container CLI to 0.12.0 pkg; wire Sparkle 2.6.0 binaryTarget
Local SPM resolve hangs on the Sparkle-for-Swift-Package-Manager.zip download in some environments; SparkleBridge already gates on canImport(Sparkle) so the launcher works without it. The build pipeline re-adds the .binaryTarget at release time (URL + pinned checksum kept in the file header for reference).
- build.sh: skip sparkle-sign step when no EdDSA private key is present - assemble_bundle.sh: substitute empty SU_PUBLIC_ED_KEY when ed_public.pem missing; copy container libexec plugins into bundle - build_frontend.sh: build from desktop/ (not frontend/) using npm ci/run build
Five interlocking fixes uncovered by the first end-to-end smoke build: * App.swift installs DispatchSource SIGTERM/SIGINT handlers that route to NSApp.terminate(nil), so shell-driven kills (and macOS shutdown) run applicationWillTerminate and reap the embedded uvicorn child instead of orphaning it. * ServerProcess.stop() is now synchronous so it can run on the main thread during applicationWillTerminate without deadlocking on a Task that needs the main actor. * App.swift sets PYTHONDONTWRITEBYTECODE=1 on the embedded server so .pyc files don't get written next to bundled sources and invalidate the codesign seal. * tinyagentos/__main__.py honours TAOS_DATA_DIR by passing it to create_app() and seeding the directory from the bundled data/ skeleton on first run, so user state lives in ~/Library/Application Support/taOS instead of inside the .app bundle. * assemble_bundle.sh ships the data/ and app-catalog/ skeletons into the bundle so the seed step has something to copy from. Build pipeline cleanups: * sign.sh: empty-array expansion guard so set -u + bash 3.2 can sign unsigned bundles (no Dev ID). * fetch_container_cli.sh: rename payload extraction dir to extracted/ to avoid colliding with the pkg's Payload file on case-insensitive APFS. * build_frontend.sh: copy from static/desktop/ (Vite outDir) instead of the non-existent desktop/dist/. Also: ignore mac/launcher/.build/ (SwiftPM intermediate artefacts).
📝 WalkthroughWalkthroughThis pull request adds comprehensive macOS support to taOS, including a Swift launcher application with Sparkle auto-updates, an end-to-end build pipeline (compilation, signing, notarization, DMG packaging), an Apple container backend for macOS-native container management, and extensive test coverage for all new components. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Launcher as macOS Launcher<br/>(Swift)
participant StatusBar as Status Bar Menu
participant Server as Python Backend<br/>(FastAPI)
participant Container as Apple Container<br/>CLI
participant WebView as Web View<br/>(WKWebView)
participant Sparkle as Sparkle<br/>Updates
User->>Launcher: Launch taOS.app
Launcher->>Launcher: Read env vars (TAOS_HOST/PORT)
Launcher->>Server: start() via Process
Launcher->>Server: Poll /api/health (async)
Server-->>Container: Initialize backend
Server-->>Launcher: 200 OK
Launcher->>StatusBar: Install menu (green)
Launcher->>WebView: showWindow()
WebView->>Server: Load http://127.0.0.1:port
Server-->>WebView: HTML/JS content
Launcher->>Sparkle: startAutomaticUpdates()
Sparkle->>Sparkle: Check feed (background)
User->>StatusBar: Toggle fullscreen
Launcher->>WebView: Load / vs /mobile
User->>StatusBar: Check for Updates
Sparkle->>Sparkle: User-triggered check
User->>StatusBar: Quit
Launcher->>Server: SIGTERM
Server->>Container: stop() graceful
Launcher->>Launcher: Cleanup & exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
tests/test_main_entry.py-26-42 (1)
26-42:⚠️ Potential issue | 🟡 MinorMake this fallback test deterministic.
This assertion currently depends on whatever
PROJECT_DIR/data/config.yamlcontains in the repo. If that file changes, the entrypoint can still be correct and this test will start failing. Stubload_config()here so the fallback behavior is isolated from repository data.Suggested fix
with patch("uvicorn.run", side_effect=fake_run), \ - patch.object(m, "create_app", return_value=object()): + patch.object(m, "create_app", return_value=object()), \ + patch.object( + m, + "load_config", + return_value=type("Cfg", (), {"server": {"host": "0.0.0.0", "port": 6969}})(), + ): m.main()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_main_entry.py` around lines 26 - 42, The test test_main_falls_back_to_config_when_env_unset should stub the configuration loader so it doesn't depend on PROJECT_DIR/data/config.yaml; patch tinyagentos.__main__.load_config (or import m.load_config) to return a dict/config object with host "0.0.0.0" and port 6969 before calling m.main(), keep the existing patches for uvicorn.run and m.create_app, and assert captured values remain "0.0.0.0" and 6969 to make the fallback deterministic.tests/test_app_apple_backend_wiring.py-17-20 (1)
17-20:⚠️ Potential issue | 🟡 MinorReset the global backend singleton after this smoke test.
get_backend()reads module-level state, and this test leaves_active_backendset toAppleContainerBackendafter the lifespan exits. That makes later tests order-dependent if they expect no backend to be configured yet.Suggested fix
async def test_create_app_installs_apple_backend_on_darwin(tmp_path, monkeypatch): monkeypatch.setenv("TAOS_CONTAINER_BIN", "/usr/local/bin/container") + from tinyagentos.containers import backend as backend_module + monkeypatch.setattr(backend_module, "_active_backend", None) with patch("tinyagentos.containers.backend.detect_runtime", return_value="apple"): from tinyagentos.app import create_app from tinyagentos.containers.backend import get_backend from tinyagentos.containers.apple_backend import AppleContainerBackend🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_app_apple_backend_wiring.py` around lines 17 - 20, The test leaves the module-level backend singleton set; after using create_app and asserting get_backend() is an AppleContainerBackend, clear the global backend by resetting the module's _active_backend to None (or call any provided reset/clear function) so later tests don't see a configured backend; update the test around create_app / async with app.router.lifespan_context(app) to clear _active_backend after the context exits (referencing create_app, get_backend, AppleContainerBackend, and _active_backend).mac/launcher/Sources/taOSLauncher/ServerProcess.swift-35-46 (1)
35-46:⚠️ Potential issue | 🟡 MinorClose the log handle when process launch fails.
If
proc.run()throws on line 45, the file handle opened on lines 35-37 is left open and stored onself.logHandle. This leaks the file descriptor and leaves stale state for any retry path.Suggested fix
let handle = try FileHandle(forWritingTo: logFile) try handle.seekToEnd() - self.logHandle = handle let proc = Process() proc.executableURL = executable proc.arguments = arguments proc.environment = env proc.standardOutput = handle proc.standardError = handle - try proc.run() + do { + try proc.run() + } catch { + try? handle.close() + throw error + } + self.logHandle = handle self.process = proc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/launcher/Sources/taOSLauncher/ServerProcess.swift` around lines 35 - 46, The FileHandle opened and assigned to self.logHandle before launching the Process can be leaked if proc.run() throws; wrap the Process launch so that if proc.run() fails you close the FileHandle (handle.closeFile() or try handle.close()) and clear self.logHandle (set to nil) before rethrowing or returning; update the block around FileHandle/Process creation (references: FileHandle, self.logHandle, proc, proc.run(), proc.standardOutput/proc.standardError) to ensure the handle is always closed on error.mac/README.md-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorClarify that
.dmg.sigis conditional.Line 11 currently reads as if the signature file is always produced. Since signing can be skipped, document
.dmg.sigas conditional on Sparkle signing credentials.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/README.md` at line 11, Update the README sentence that lists build artifacts so it clarifies the signature file is conditional: replace the current line that reads "Produces `dist/taOS.app`, `dist/taOS-0.1.0.dmg`, and `dist/taOS-0.1.0.dmg.sig`" with a version stating that `dist/taOS.app` and `dist/taOS-0.1.0.dmg` are always produced, and that `dist/taOS-0.1.0.dmg.sig` is produced only when Sparkle signing credentials (or signing is enabled) are provided; mention Sparkle signing credentials explicitly to link the condition to the signing step.mac/build/generate_icon_placeholder.sh-17-24 (1)
17-24:⚠️ Potential issue | 🟡 MinorAdd source-file preflight validation before resize loop.
Line 17 validates argument presence but not source validity. Failing early on missing/unreadable input gives clearer errors than mid-loop
sipsfailures.Proposed fix
[[ -n "$SRC" && -n "$OUT" ]] || { echo "all args required" >&2; exit 2; } +[[ -f "$SRC" && -r "$SRC" ]] || { echo "source PNG not found/readable: $SRC" >&2; exit 2; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/build/generate_icon_placeholder.sh` around lines 17 - 24, Add a preflight check that validates the source file before entering the resize loop: after the existing args check (using the SRC and OUT variables) verify that SRC exists and is readable (e.g., [ -r "$SRC" ] or [ -f "$SRC" ] && [ -r "$SRC" ]) and, if not, print a clear error to stderr and exit with a non-zero status; keep the ICONSET creation and the for loop (icon generation using sips) unchanged so failures won’t occur mid-loop due to a missing/unreadable source.mac/build/notarize.sh-8-12 (1)
8-12:⚠️ Potential issue | 🟡 MinorHarden
--dmgargument parsing for missing values.On Line 10,
DMG="$2"assumes a value exists. Withset -u,--dmgwithout a value exits abruptly with a shell error instead of a clear usage failure.Proposed fix
while [[ $# -gt 0 ]]; do case "$1" in - --dmg) DMG="$2"; shift 2 ;; + --dmg) + [[ $# -ge 2 ]] || { echo "notarize.sh: --dmg requires a value" >&2; exit 2; } + DMG="$2"; shift 2 ;; *) echo "notarize.sh: unknown arg $1" >&2; exit 2 ;; esac done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/build/notarize.sh` around lines 8 - 12, The --dmg branch assigns DMG="$2" without validating a following value; update the case for "--dmg" in the while/case loop to first verify a non-empty next argument exists and is not another flag (e.g. check ${2:-} and/or that it doesn't start with "-"); if the value is missing print a clear usage/error and exit non-zero, otherwise assign DMG="$2" and shift 2 as before so missing/invalid invocations fail with a friendly message instead of a shell error.mac/build/package_dmg.sh-19-24 (1)
19-24:⚠️ Potential issue | 🟡 MinorAdd preflight checks for
--appand output directory.Before Line 23, validate that the app bundle exists and create the output directory. This avoids opaque failures in
create-dmg.Proposed fix
[[ -n "$APP" && -n "$VERSION" && -n "$OUTPUT" ]] || { echo "all args required" >&2; exit 2; } +[[ -d "$APP" ]] || { echo "app bundle not found: $APP" >&2; exit 2; } +mkdir -p "$OUTPUT"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/build/package_dmg.sh` around lines 19 - 24, Add preflight checks before DMG creation: verify the app bundle referenced by $APP exists (e.g., test -e "$APP" or test -d "$APP/Contents") and fail with a clear error if missing, and ensure the output directory for $OUTPUT exists (mkdir -p "$OUTPUT" or test -d "$OUTPUT" and create it) before computing DMG="$OUTPUT/taOS-$VERSION.dmg" and calling create-dmg; include clear error messages mentioning $APP and $OUTPUT so create-dmg can't fail with opaque errors.mac/build/RELEASE_TESTING.md-46-46 (1)
46-46:⚠️ Potential issue | 🟡 MinorClarify ambiguous wording in failure-isolation step.
At Line 46, “new creates fail” is unclear; use explicit phrasing so testers interpret the step consistently.
✏️ Suggested wording
-- [ ] Stop the container daemon → existing containers listed but new creates fail with the documented note → restart daemon → recovers. +- [ ] Stop the container daemon → existing containers are listed, but creating new containers fails with the documented note → restart daemon → recovers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/build/RELEASE_TESTING.md` at line 46, Replace the ambiguous checklist line "Stop the container daemon → existing containers listed but new creates fail with the documented note → restart daemon → recovers." with an explicit step that clearly states the expected behavior, e.g. "Stop the container daemon → existing containers continue running and are listed, but attempts to create new containers fail with the documented error/note; restart the daemon and verify new container creation succeeds and the system recovers." Target the checklist item containing the original sentence to update the wording.mac/build/build.sh-88-89 (1)
88-89:⚠️ Potential issue | 🟡 MinorAvoid printing non-existent artifacts as successful outputs.
Lines 88–89 always print Sparkle artifact paths even when sparkle-sign was skipped, which can cause false positives in release checks.
🧩 Suggested fix
echo "[build] done" echo " app: $APP" echo " dmg: $DMG" -echo " sig: ${DMG}.sig" -echo " appcast snippet: $REPO_ROOT/$OUTPUT/appcast-snippet.xml" +if [[ -f "${DMG}.sig" ]]; then + echo " sig: ${DMG}.sig" +fi +if [[ -f "$REPO_ROOT/$OUTPUT/appcast-snippet.xml" ]]; then + echo " appcast snippet: $REPO_ROOT/$OUTPUT/appcast-snippet.xml" +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/build/build.sh` around lines 88 - 89, The script currently always echoes Sparkle artifact paths (echo " sig: ${DMG}.sig" and echo " appcast snippet: $REPO_ROOT/$OUTPUT/appcast-snippet.xml") even when sparkle-sign was skipped; change those prints to run only if the corresponding files actually exist—check for "${DMG}.sig" and "$REPO_ROOT/$OUTPUT/appcast-snippet.xml" (e.g., test -f) before echoing so the script reports only real artifacts and avoids false positives.mac/build/bump_version.sh-24-24 (1)
24-24:⚠️ Potential issue | 🟡 MinorUse fixed-string matching for changelog section lookup.
At Line 24, regex matching with unescaped dots in semantic versions can produce false positives.
Suggested fix
-if ! grep -q "^## $NEW_VER" CHANGELOG.md 2>/dev/null; then +if ! grep -Fqx "## $NEW_VER" CHANGELOG.md 2>/dev/null; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/build/bump_version.sh` at line 24, The grep call that checks for the changelog header uses regex matching so dots in NEW_VER are treated as wildcards; change it to fixed-string matching by using grep -F (or -qF) and match "## $NEW_VER" (e.g., replace the existing grep invocation with grep -qF "## $NEW_VER" CHANGELOG.md 2>/dev/null) so semantic-version dots are treated literally; update the command that currently contains grep -q "^## $NEW_VER" to use -F and the literal string instead.mac/launcher/Sources/taOSLauncher/WindowController.swift-55-56 (1)
55-56:⚠️ Potential issue | 🟡 MinorEnable layer-backing before setting corner radius.
At Line 55–56,
cornerRadiuswon’t take effect unless the view is layer-backed.Suggested fix
let webView = WKWebView(frame: w.contentView!.bounds) webView.autoresizingMask = [.width, .height] +webView.wantsLayer = true webView.layer?.cornerRadius = 47 webView.layer?.masksToBounds = true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/launcher/Sources/taOSLauncher/WindowController.swift` around lines 55 - 56, The webView cornerRadius will not apply because the view is not layer-backed; in WindowController ensure the webView is layer-backed before mutating its CALayer by setting webView.wantsLayer = true (or otherwise enabling a backing layer) and then set webView.layer?.cornerRadius = 47 and webView.layer?.masksToBounds = true; locate the configuration in the WindowController (e.g., windowDidLoad/awakeFromNib or wherever webView is initialized) and move or add the wantsLayer enablement immediately before the cornerRadius/masksToBounds assignments.mac/launcher/Sources/taOSLauncher/App.swift-62-63 (1)
62-63:⚠️ Potential issue | 🟡 MinorDisable or hide unimplemented menu actions.
These closures make “Pause Agents” and “Preferences…” clickable no-ops. Since
MenuBar.buildMenu(...)creates enabled items for every action, the launcher currently advertises two broken paths in the shipped UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/launcher/Sources/taOSLauncher/App.swift` around lines 62 - 63, The menu currently passes empty closures for togglePause and openPreferences which makes “Pause Agents” and “Preferences…” appear enabled but do nothing; change the call-site in App.swift to pass nil for togglePause and openPreferences instead of empty closures (i.e., remove `{ }` and use nil) and ensure MenuBar.buildMenu(...) treats nil callbacks as disabled menu items (so the menu creation logic for MenuBar.buildMenu checks callback optionality and sets .isEnabled = false when nil). This will hide/disable those unimplemented actions until real handlers are provided.
🧹 Nitpick comments (4)
.gitignore (1)
5-7: Allow-list tracked sources undermac/build/instead of reopening the whole tree.Line 6 makes generated files under
mac/build/trackable again, so the first local bundle/DMG/signing run can dirty the repo with artifacts. Re-ignore descendants and explicitly unignore only the checked-in scripts/checksums.Suggested `.gitignore` shape
build/ !mac/build/ +mac/build/** +!mac/build/*.sh +!mac/build/checksums/ +!mac/build/checksums/** mac/launcher/.build/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 5 - 7, Current allow-list "!mac/build/" reopens the entire mac/build tree; instead re-ignore descendants and only unignore the specific checked-in items. Replace the "!mac/build/" entry with a re-ignore (e.g., "mac/build/**") and add explicit allow-list lines for the checked-in items (for example "mac/build/scripts/" and "mac/build/scripts/checksums") so only those files under mac/build remain tracked; leave the top-level "build/" and "mac/launcher/.build/" entries unchanged.mac/launcher/Tests/taOSLauncherTests/SparkleBridgeTests.swift (1)
5-12: AssertcanCheckForUpdatesin the positive path too.
testFeedURLReadFromBundlevalidates parsing but does not verify the eligibility flag turns on when both fields are present.Proposed addition
XCTAssertEqual(bridge.feedURL, URL(string: "https://taos.app/appcast.xml")) XCTAssertEqual(bridge.publicKey, "fakekey==") + XCTAssertTrue(bridge.canCheckForUpdates)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/launcher/Tests/taOSLauncherTests/SparkleBridgeTests.swift` around lines 5 - 12, Add an assertion to the existing testFeedURLReadFromBundle to also verify the eligibility flag is enabled: after creating the SparkleBridge(infoDict: ...) and asserting feedURL and publicKey, assert that bridge.canCheckForUpdates is true; this ensures the positive path is covered alongside the parsing checks for SparkleBridge's feedURL, publicKey and canCheckForUpdates properties.mac/launcher/Tests/taOSLauncherTests/PlaceholderTests.swift (1)
4-5: Replace the always-pass placeholder with a real assertion.
XCTAssertTrue(true)is non-informative and can mask missing coverage. Please assert an actual launcher behavior (or remove this placeholder once real tests are in place).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/launcher/Tests/taOSLauncherTests/PlaceholderTests.swift` around lines 4 - 5, The testPackageBuilds placeholder (function testPackageBuilds in PlaceholderTests.swift) currently uses XCTAssertTrue(true); replace it with a meaningful assertion that verifies actual launcher behavior — for example assert that the built package exists, that the launcher binary is present and executable, or that a sample launch invocation returns the expected exit code or output; update the test to perform the build step (or mock it), check the resulting artifact/path or process result, and assert that the real condition is met (or remove the placeholder test entirely if no real test is available).mac/launcher/Sources/taOSLauncher/KeyboardMonitor.swift (1)
33-42: Makeinstall()idempotent.At Line 33–42, calling
install()more than once can register multiple monitors. Guarding on existingmonitoravoids duplicate interception handlers.Suggested fix
public func install() { + guard monitor == nil else { return } monitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak self] event in guard let self else { return event } if self.shouldIntercept(keyCode: event.keyCode, modifiers: event.modifierFlags.intersection(.deviceIndependentFlagsMask)) { return nil } return event } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mac/launcher/Sources/taOSLauncher/KeyboardMonitor.swift` around lines 33 - 42, install() currently registers a new NSEvent local monitor each call, causing duplicate handlers; make install() idempotent by returning early if monitor is already non-nil. In the install() function check the existing monitor property (monitor) and only call NSEvent.addLocalMonitorForEvents(matching: .keyDown) to set monitor when it is nil; keep the existing closure logic (using [weak self] and shouldIntercept(keyCode:modifiers:)) and ensure monitor is assigned the result of addLocalMonitorForEvents so future calls detect it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mac/build/build_python.sh`:
- Around line 22-26: The script sets CHECKSUM_FILE to a static path while TAG
and PYTHON_VER parameterize the downloaded artifact, so integrity checks will
fail for any version not represented by that single checksum file; change
CHECKSUM_FILE to include the version/tag (e.g., incorporate ${PYTHON_VER} and
${TAG} into the filename), test for its existence before use and emit a clear
error if missing, and update the checksum lookup/validation logic accordingly;
apply the same pattern to the analogous variable/validation in
fetch_container_cli.sh so its checksum file is also version-specific.
- Around line 47-48: Remove the unpinned pip upgrade to keep builds
deterministic: delete the "$PYBIN -m pip install --upgrade pip" invocation in
build_python.sh and leave the subsequent "$PYBIN -m pip install --no-deps -r
\"$REPO_ROOT/tinyagentos/requirements.lock\"" line intact so the pinned
requirements.lock controls dependency versions.
In `@mac/build/fetch_container_cli.sh`:
- Around line 22-24: The script currently hardcodes
CHECKSUM_FILE=".../apple-container-cli.sha256" while URL uses the version
variable CLI_VER, causing checksum mismatch when a custom --version is used;
update CHECKSUM_FILE to include CLI_VER (e.g., derive CHECKSUM_FILE using
CLI_VER) so the checksum filename matches the downloaded package version, ensure
any downstream validation reads that versioned checksum file, and keep using
CLI_VER in both CHECKSUM_FILE and URL consistently.
In `@mac/build/sign.sh`:
- Around line 33-35: The current find|xargs invocation that runs codesign (the
line invoking codesign --force --sign "$IDENTITY" "${EA[@]+"${EA[@]}"}") hides
failures with `2>/dev/null || true`; remove that suppression and ensure the
script fails when codesign fails by not redirecting stderr or swallowing the
exit code, and either enable immediate-fail shell behavior (e.g., set -e at
script top) or explicitly capture the exit status of xargs/codesign and exit 1
on non‑zero so any signing error using IDENTITY / EA is visible and causes the
pipeline to fail.
In `@mac/build/sparkle_sign.sh`:
- Around line 40-41: The script currently extracts SIG_FIELD from SIGNATURE_LINE
and writes it to "${DMG}.sig" without validation; modify the logic after
SIG_FIELD is assigned (the block that currently writes via echo -n "$SIG_FIELD"
> "${DMG}.sig") to check if SIG_FIELD is empty or unset (e.g., test -z
"$SIG_FIELD") and if so print an error to stderr mentioning the failed
sparkle:edSignature extraction and exit with a non-zero status; only proceed to
write the sidecar when SIG_FIELD is non-empty.
In `@mac/launcher/Package.swift`:
- Around line 19-21: The Package.swift currently sets the deployment target to
.macOS(.v14) in the Package initializer (name: "taOSLauncher") which mismatches
the app's Info.plist.in (macOS 26); update the platforms declaration in
Package.swift to use .macOS(.v26) so the Package constant's platforms array
matches the intended macOS 26 deployment target and prevents building on
unsupported systems.
In `@mac/launcher/Sources/taOSLauncher/App.swift`:
- Around line 33-35: The observer was registered on
NSWorkspace.shared.notificationCenter so windowWillClose(_:) never fires and it
doesn’t filter to the taOS main window; change the registration to
NotificationCenter.default and pass the specific taOS window as the object so
only that window’s willClose notification triggers. Locate the addObserver calls
(currently using NSWorkspace.shared.notificationCenter) and replace them to use
NotificationCenter.default.addObserver(self, selector:
`#selector`(windowWillClose(_:)), name: NSWindow.willCloseNotification, object:
<the taOS main window instance>), updating both the earlier and the later
occurrences (the ones referenced at lines 33–35 and 133–136). Ensure
windowWillClose(_:) still reads the notification.sender/window and updates
keyboardMonitor.fullscreen accordingly.
In `@mac/launcher/Sources/taOSLauncher/SparkleBridge.swift`:
- Line 28: The computed property canCheckForUpdates currently only checks
publicKey != nil, which treats an empty SUPublicEDKey template as available;
update canCheckForUpdates to also ensure publicKey is non-empty (e.g.,
publicKey?.isEmpty == false or trimmed length > 0) so that an empty
SUPublicEDKey is treated as unavailable; locate the canCheckForUpdates computed
property and the publicKey declaration in SparkleBridge.swift to implement this
additional emptiness check.
In `@tinyagentos/__main__.py`:
- Around line 18-35: The startup logic reads config from PROJECT_DIR / "data" /
"config.yaml" when TAOS_HOST/PORT are unset but creates the app using data_dir
from TAOS_DATA_DIR, causing a mismatch; modify the flow in __main__.py so that
if env_data_dir is set and TAOS_HOST/PORT are not, you load config via
load_config(Path(env_data_dir) / "config.yaml") (use that host/port), still call
_seed_data_dir(data_dir) and pass data_dir to create_app(data_dir=...), and then
call uvicorn.run(app, host=host, port=port) so the server binds to the same
config the app uses (references: env_data_dir, load_config, _seed_data_dir,
create_app, uvicorn.run).
In `@tinyagentos/app.py`:
- Around line 594-600: The settings route currently rejects the new "apple"
runtime so users can't set it via /api/settings/container-runtime; update
tinyagentos/routes/settings.py to include "apple" in the allowed/validated
runtime values and map it to the same behavior as in app.py (instantiate/use
AppleContainerBackend when the runtime value is "apple"). Locate the validation
or allowed list for the container runtime (the handler that processes the
"container-runtime" setting) and add "apple" to that list and to the switch/if
logic so the route accepts the value and applies the AppleContainerBackend via
set_backend(AppleContainerBackend()) just like the app startup path does.
In `@tinyagentos/containers/apple_backend.py`:
- Around line 72-109: The create_container implementation currently ignores the
host_uid parameter; update create_container to apply host_uid when provided (so
bind-mounted files keep expected ownership) by adding logic that sets the
container user/UID before launching: inside create_container (the function
shown) if host_uid is not None, append the appropriate CLI option (e.g.,
"--user", str(host_uid)) to argv (or, if the Apple CLI uses a different flag for
UID mapping, use that flag) so the created container runs with the requested
UID; ensure this branch executes before argv.append(image) so the flag is
included in the run command and keep existing behavior for root_size_gib and
set_root_quota unchanged.
- Around line 27-35: The _run method currently lets FileNotFoundError and
asyncio.TimeoutError propagate and can leave the child process running; update
async def _run(self, cmd: list[str], timeout: int = 120) to catch
FileNotFoundError and asyncio.TimeoutError and always return a normalized (int,
str) tuple: on FileNotFoundError return non-zero code (e.g., 127) with a clear
message including the missing binary/self.binary; on asyncio.TimeoutError ensure
you kill and await the subprocess (proc.kill(); await proc.wait()) to avoid
zombies and then return a non-zero code (e.g., 124) with a timeout message; also
catch unexpected exceptions and return (1, str(exc)) so all callers of _run see
a consistent (code, output) response.
---
Minor comments:
In `@mac/build/build.sh`:
- Around line 88-89: The script currently always echoes Sparkle artifact paths
(echo " sig: ${DMG}.sig" and echo " appcast snippet:
$REPO_ROOT/$OUTPUT/appcast-snippet.xml") even when sparkle-sign was skipped;
change those prints to run only if the corresponding files actually exist—check
for "${DMG}.sig" and "$REPO_ROOT/$OUTPUT/appcast-snippet.xml" (e.g., test -f)
before echoing so the script reports only real artifacts and avoids false
positives.
In `@mac/build/bump_version.sh`:
- Line 24: The grep call that checks for the changelog header uses regex
matching so dots in NEW_VER are treated as wildcards; change it to fixed-string
matching by using grep -F (or -qF) and match "## $NEW_VER" (e.g., replace the
existing grep invocation with grep -qF "## $NEW_VER" CHANGELOG.md 2>/dev/null)
so semantic-version dots are treated literally; update the command that
currently contains grep -q "^## $NEW_VER" to use -F and the literal string
instead.
In `@mac/build/generate_icon_placeholder.sh`:
- Around line 17-24: Add a preflight check that validates the source file before
entering the resize loop: after the existing args check (using the SRC and OUT
variables) verify that SRC exists and is readable (e.g., [ -r "$SRC" ] or [ -f
"$SRC" ] && [ -r "$SRC" ]) and, if not, print a clear error to stderr and exit
with a non-zero status; keep the ICONSET creation and the for loop (icon
generation using sips) unchanged so failures won’t occur mid-loop due to a
missing/unreadable source.
In `@mac/build/notarize.sh`:
- Around line 8-12: The --dmg branch assigns DMG="$2" without validating a
following value; update the case for "--dmg" in the while/case loop to first
verify a non-empty next argument exists and is not another flag (e.g. check
${2:-} and/or that it doesn't start with "-"); if the value is missing print a
clear usage/error and exit non-zero, otherwise assign DMG="$2" and shift 2 as
before so missing/invalid invocations fail with a friendly message instead of a
shell error.
In `@mac/build/package_dmg.sh`:
- Around line 19-24: Add preflight checks before DMG creation: verify the app
bundle referenced by $APP exists (e.g., test -e "$APP" or test -d
"$APP/Contents") and fail with a clear error if missing, and ensure the output
directory for $OUTPUT exists (mkdir -p "$OUTPUT" or test -d "$OUTPUT" and create
it) before computing DMG="$OUTPUT/taOS-$VERSION.dmg" and calling create-dmg;
include clear error messages mentioning $APP and $OUTPUT so create-dmg can't
fail with opaque errors.
In `@mac/build/RELEASE_TESTING.md`:
- Line 46: Replace the ambiguous checklist line "Stop the container daemon →
existing containers listed but new creates fail with the documented note →
restart daemon → recovers." with an explicit step that clearly states the
expected behavior, e.g. "Stop the container daemon → existing containers
continue running and are listed, but attempts to create new containers fail with
the documented error/note; restart the daemon and verify new container creation
succeeds and the system recovers." Target the checklist item containing the
original sentence to update the wording.
In `@mac/launcher/Sources/taOSLauncher/App.swift`:
- Around line 62-63: The menu currently passes empty closures for togglePause
and openPreferences which makes “Pause Agents” and “Preferences…” appear enabled
but do nothing; change the call-site in App.swift to pass nil for togglePause
and openPreferences instead of empty closures (i.e., remove `{ }` and use nil)
and ensure MenuBar.buildMenu(...) treats nil callbacks as disabled menu items
(so the menu creation logic for MenuBar.buildMenu checks callback optionality
and sets .isEnabled = false when nil). This will hide/disable those
unimplemented actions until real handlers are provided.
In `@mac/launcher/Sources/taOSLauncher/ServerProcess.swift`:
- Around line 35-46: The FileHandle opened and assigned to self.logHandle before
launching the Process can be leaked if proc.run() throws; wrap the Process
launch so that if proc.run() fails you close the FileHandle (handle.closeFile()
or try handle.close()) and clear self.logHandle (set to nil) before rethrowing
or returning; update the block around FileHandle/Process creation (references:
FileHandle, self.logHandle, proc, proc.run(),
proc.standardOutput/proc.standardError) to ensure the handle is always closed on
error.
In `@mac/launcher/Sources/taOSLauncher/WindowController.swift`:
- Around line 55-56: The webView cornerRadius will not apply because the view is
not layer-backed; in WindowController ensure the webView is layer-backed before
mutating its CALayer by setting webView.wantsLayer = true (or otherwise enabling
a backing layer) and then set webView.layer?.cornerRadius = 47 and
webView.layer?.masksToBounds = true; locate the configuration in the
WindowController (e.g., windowDidLoad/awakeFromNib or wherever webView is
initialized) and move or add the wantsLayer enablement immediately before the
cornerRadius/masksToBounds assignments.
In `@mac/README.md`:
- Line 11: Update the README sentence that lists build artifacts so it clarifies
the signature file is conditional: replace the current line that reads "Produces
`dist/taOS.app`, `dist/taOS-0.1.0.dmg`, and `dist/taOS-0.1.0.dmg.sig`" with a
version stating that `dist/taOS.app` and `dist/taOS-0.1.0.dmg` are always
produced, and that `dist/taOS-0.1.0.dmg.sig` is produced only when Sparkle
signing credentials (or signing is enabled) are provided; mention Sparkle
signing credentials explicitly to link the condition to the signing step.
In `@tests/test_app_apple_backend_wiring.py`:
- Around line 17-20: The test leaves the module-level backend singleton set;
after using create_app and asserting get_backend() is an AppleContainerBackend,
clear the global backend by resetting the module's _active_backend to None (or
call any provided reset/clear function) so later tests don't see a configured
backend; update the test around create_app / async with
app.router.lifespan_context(app) to clear _active_backend after the context
exits (referencing create_app, get_backend, AppleContainerBackend, and
_active_backend).
In `@tests/test_main_entry.py`:
- Around line 26-42: The test test_main_falls_back_to_config_when_env_unset
should stub the configuration loader so it doesn't depend on
PROJECT_DIR/data/config.yaml; patch tinyagentos.__main__.load_config (or import
m.load_config) to return a dict/config object with host "0.0.0.0" and port 6969
before calling m.main(), keep the existing patches for uvicorn.run and
m.create_app, and assert captured values remain "0.0.0.0" and 6969 to make the
fallback deterministic.
---
Nitpick comments:
In @.gitignore:
- Around line 5-7: Current allow-list "!mac/build/" reopens the entire mac/build
tree; instead re-ignore descendants and only unignore the specific checked-in
items. Replace the "!mac/build/" entry with a re-ignore (e.g., "mac/build/**")
and add explicit allow-list lines for the checked-in items (for example
"mac/build/scripts/" and "mac/build/scripts/checksums") so only those files
under mac/build remain tracked; leave the top-level "build/" and
"mac/launcher/.build/" entries unchanged.
In `@mac/launcher/Sources/taOSLauncher/KeyboardMonitor.swift`:
- Around line 33-42: install() currently registers a new NSEvent local monitor
each call, causing duplicate handlers; make install() idempotent by returning
early if monitor is already non-nil. In the install() function check the
existing monitor property (monitor) and only call
NSEvent.addLocalMonitorForEvents(matching: .keyDown) to set monitor when it is
nil; keep the existing closure logic (using [weak self] and
shouldIntercept(keyCode:modifiers:)) and ensure monitor is assigned the result
of addLocalMonitorForEvents so future calls detect it.
In `@mac/launcher/Tests/taOSLauncherTests/PlaceholderTests.swift`:
- Around line 4-5: The testPackageBuilds placeholder (function testPackageBuilds
in PlaceholderTests.swift) currently uses XCTAssertTrue(true); replace it with a
meaningful assertion that verifies actual launcher behavior — for example assert
that the built package exists, that the launcher binary is present and
executable, or that a sample launch invocation returns the expected exit code or
output; update the test to perform the build step (or mock it), check the
resulting artifact/path or process result, and assert that the real condition is
met (or remove the placeholder test entirely if no real test is available).
In `@mac/launcher/Tests/taOSLauncherTests/SparkleBridgeTests.swift`:
- Around line 5-12: Add an assertion to the existing testFeedURLReadFromBundle
to also verify the eligibility flag is enabled: after creating the
SparkleBridge(infoDict: ...) and asserting feedURL and publicKey, assert that
bridge.canCheckForUpdates is true; this ensures the positive path is covered
alongside the parsing checks for SparkleBridge's feedURL, publicKey and
canCheckForUpdates properties.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d6ad24fd-0fa2-48ec-8a31-d3a915cf12f7
⛔ Files ignored due to path filters (1)
tinyagentos/requirements.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
.gitignoremac/README.mdmac/appcast/appcast.xmlmac/build/RELEASE_TESTING.mdmac/build/assemble_bundle.shmac/build/build.shmac/build/build_frontend.shmac/build/build_python.shmac/build/bump_version.shmac/build/checksums/.gitkeepmac/build/checksums/apple-container-cli.sha256mac/build/checksums/python-build-standalone.sha256mac/build/fetch_container_cli.shmac/build/generate_icon_placeholder.shmac/build/notarize.shmac/build/package_dmg.shmac/build/sign.shmac/build/sparkle_sign.shmac/launcher/Package.swiftmac/launcher/Sources/taOSLauncher/App.swiftmac/launcher/Sources/taOSLauncher/KeyboardMonitor.swiftmac/launcher/Sources/taOSLauncher/MenuBar.swiftmac/launcher/Sources/taOSLauncher/Resources/Assets.xcassets/AppIcon.appiconset/Contents.jsonmac/launcher/Sources/taOSLauncher/Resources/Assets.xcassets/Contents.jsonmac/launcher/Sources/taOSLauncher/Resources/Info.plist.inmac/launcher/Sources/taOSLauncher/ServerProcess.swiftmac/launcher/Sources/taOSLauncher/SparkleBridge.swiftmac/launcher/Sources/taOSLauncher/WindowController.swiftmac/launcher/Tests/taOSLauncherTests/KeyboardMonitorTests.swiftmac/launcher/Tests/taOSLauncherTests/MenuBarTests.swiftmac/launcher/Tests/taOSLauncherTests/PlaceholderTests.swiftmac/launcher/Tests/taOSLauncherTests/ServerProcessTests.swiftmac/launcher/Tests/taOSLauncherTests/SparkleBridgeTests.swiftmac/launcher/Tests/taOSLauncherTests/WindowControllerTests.swifttests/test_app_apple_backend_wiring.pytests/test_apple_backend.pytests/test_container_detection.pytests/test_main_entry.pytinyagentos/__main__.pytinyagentos/app.pytinyagentos/containers/apple_backend.pytinyagentos/containers/backend.py
Comment on lines
+22
to
+26
| CHECKSUM_FILE="$REPO_ROOT/mac/build/checksums/python-build-standalone.sha256" | ||
|
|
||
| # Latest stable release naming convention from astral-sh/python-build-standalone | ||
| TAG="20260414" | ||
| URL="https://github.com/astral-sh/python-build-standalone/releases/download/${TAG}/cpython-${PYTHON_VER}+${TAG}-aarch64-apple-darwin-install_only.tar.gz" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "=== build_python.sh version/checksum wiring ==="
rg -n 'PYTHON_VER|TAG|CHECKSUM_FILE|URL=|EXPECTED_SHA|ACTUAL_SHA' mac/build/build_python.sh
echo
echo "=== checksum file contents ==="
nl -ba mac/build/checksums/python-build-standalone.sha256Repository: jaylfc/tinyagentos
Length of output: 984
🏁 Script executed:
cat mac/build/checksums/python-build-standalone.sha256Repository: jaylfc/tinyagentos
Length of output: 127
🏁 Script executed:
fd -type f -name "*checksum*" -o -name "*.sha256" mac/Repository: jaylfc/tinyagentos
Length of output: 233
🏁 Script executed:
sed -n '1,45p' mac/build/build_python.shRepository: jaylfc/tinyagentos
Length of output: 1522
🏁 Script executed:
find mac/build/checksums -type fRepository: jaylfc/tinyagentos
Length of output: 189
🏁 Script executed:
ls -la mac/build/checksums/Repository: jaylfc/tinyagentos
Length of output: 382
🏁 Script executed:
grep -r "PYTHON_VER\|TAG\|CHECKSUM" mac/build/ --include="*.sh" -B2 -A2Repository: jaylfc/tinyagentos
Length of output: 7813
Version argument conflicts with static checksum validation.
The script accepts --version <PYTHON_VER> to parameterize the Python distribution, and the download URL correctly incorporates this version (line 26). However, validation against a single static checksum file (line 22) defeats this flexibility—any PYTHON_VER other than the one matching python-build-standalone.sha256 will fail integrity checks, even when the artifact is valid from the upstream repository.
Consider using version-specific checksum file paths:
CHECKSUM_FILE="$REPO_ROOT/mac/build/checksums/python-build-standalone-${PYTHON_VER}+${TAG}.sha256"
[[ -f "$CHECKSUM_FILE" ]] || {
echo "[build_python] missing checksum file for ${PYTHON_VER}+${TAG}: $CHECKSUM_FILE" >&2
exit 2
}
Also applies to: fetch_container_cli.sh—same pattern with static checksum file while version is parameterized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mac/build/build_python.sh` around lines 22 - 26, The script sets
CHECKSUM_FILE to a static path while TAG and PYTHON_VER parameterize the
downloaded artifact, so integrity checks will fail for any version not
represented by that single checksum file; change CHECKSUM_FILE to include the
version/tag (e.g., incorporate ${PYTHON_VER} and ${TAG} into the filename), test
for its existence before use and emit a clear error if missing, and update the
checksum lookup/validation logic accordingly; apply the same pattern to the
analogous variable/validation in fetch_container_cli.sh so its checksum file is
also version-specific.
Comment on lines
+33
to
+35
| find "$APP/Contents" \( -type f -perm -u+x -o -name "*.dylib" -o -name "*.framework" \) -print0 \ | ||
| | xargs -0 -I{} codesign --force --sign "$IDENTITY" "${EA[@]+"${EA[@]}"}" {} 2>/dev/null || true | ||
|
|
There was a problem hiding this comment.
Don’t swallow nested codesign failures.
At Line 34, 2>/dev/null || true can hide signing breakages and let the pipeline continue in a partially signed state.
Suggested fix
-find "$APP/Contents" \( -type f -perm -u+x -o -name "*.dylib" -o -name "*.framework" \) -print0 \
- | xargs -0 -I{} codesign --force --sign "$IDENTITY" "${EA[@]+"${EA[@]}"}" {} 2>/dev/null || true
+while IFS= read -r -d '' bin; do
+ codesign --force --sign "$IDENTITY" "${EA[@]+"${EA[@]}"}" "$bin"
+done < <(
+ find "$APP/Contents" -depth \( -type f -perm -u+x -o -type f -name "*.dylib" \) -print0
+)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mac/build/sign.sh` around lines 33 - 35, The current find|xargs invocation
that runs codesign (the line invoking codesign --force --sign "$IDENTITY"
"${EA[@]+"${EA[@]}"}") hides failures with `2>/dev/null || true`; remove that
suppression and ensure the script fails when codesign fails by not redirecting
stderr or swallowing the exit code, and either enable immediate-fail shell
behavior (e.g., set -e at script top) or explicitly capture the exit status of
xargs/codesign and exit 1 on non‑zero so any signing error using IDENTITY / EA
is visible and causes the pipeline to fail.
Comment on lines
+27
to
+35
| async def _run(self, cmd: list[str], timeout: int = 120) -> tuple[int, str]: | ||
| """Run a command and return (returncode, output).""" | ||
| proc = await asyncio.create_subprocess_exec( | ||
| *cmd, | ||
| stdout=asyncio.subprocess.PIPE, | ||
| stderr=asyncio.subprocess.STDOUT, | ||
| ) | ||
| stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=timeout) | ||
| return proc.returncode, stdout.decode() if stdout else "" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Python's asyncio.create_subprocess_exec()raiseFileNotFoundErrorwhen the executable is missing, and what happens to a subprocess ifawait asyncio.wait_for(proc.communicate(), timeout=...) times out?
💡 Result:
Yes, Python's asyncio.create_subprocess_exec raises FileNotFoundError when the executable is missing. This occurs because it internally uses subprocess.Popen, which raises FileNotFoundError (a subclass of OSError) if the executable specified as the first argument cannot be found in the PATH or does not exist. When await asyncio.wait_for(proc.communicate, timeout=...) times out, an asyncio.TimeoutError is raised. However, the subprocess continues running in the background because wait_for cancels the communicate coroutine but does not terminate the process. The process must be manually terminated using methods like proc.terminate or proc.kill, followed by await proc.wait to ensure cleanup. The official documentation recommends using wait_for for timeouts since communicate and wait lack a built-in timeout parameter, but emphasizes handling the process termination explicitly after timeout.
Citations:
- 1: https://stackoverflow.com/questions/59631599/asyncio-create-subprocess-exec-filenotfounderror-errno-2-no-such-file-or-dire
- 2: bug from asyncio.subprocess import Process python/cpython#104315
- 3: https://stackoverflow.com/questions/38730288/unable-use-bash-commands-through-asyncio
- 4: https://docs.python.org/3/library/asyncio-subprocess.html
- 5: https://docs.python.org/3.15/library/asyncio-subprocess.html
- 6: subprocess raises misleading exceptions from child failure pre-exec python/cpython#104522
- 7: subprocess: add a helper/parameter to catch exec() OSError exception python/cpython#86814
- 8: https://stackoverflow.com/questions/70160450/subprocess-command-shows-filenotfounderror-errno-2-no-such-file-or-directory
- 9: https://stackoverflow.com/questions/71814738/asyncio-terminate-subprocess-on-wait-for-timeout
- 10: https://runebook.dev/en/docs/python/library/asyncio-subprocess/asyncio.subprocess.Process.communicate
- 11: https://bugs.python.org/issue38988
- 12: asyncio's Process.communicate() is unsafe to cancel python/cpython#139373
- 13: https://runebook.dev/en/docs/python/library/asyncio-subprocess/interacting-with-subprocesses
🏁 Script executed:
# Find calls to _run() in the apple_backend file
rg "_run\(" tinyagentos/containers/apple_backend.py -A 2 -B 2Repository: jaylfc/tinyagentos
Length of output: 2755
🏁 Script executed:
# Check if there are any try-except blocks wrapping _run() calls
rg "try:" tinyagentos/containers/apple_backend.py -A 5 | head -50Repository: jaylfc/tinyagentos
Length of output: 509
Handle missing binaries and timeouts inside _run().
All 13+ callers of _run() expect a normalized (code, output) tuple and check if code != 0: to determine failure. Currently, a missing self.binary raises FileNotFoundError and a command timeout raises asyncio.TimeoutError, both of which propagate uncaught and crash the backend. Additionally, a timeout leaves the subprocess running in the background without cleanup.
Suggested fix
async def _run(self, cmd: list[str], timeout: int = 120) -> tuple[int, str]:
"""Run a command and return (returncode, output)."""
- proc = await asyncio.create_subprocess_exec(
- *cmd,
- stdout=asyncio.subprocess.PIPE,
- stderr=asyncio.subprocess.STDOUT,
- )
- stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=timeout)
- return proc.returncode, stdout.decode() if stdout else ""
+ try:
+ proc = await asyncio.create_subprocess_exec(
+ *cmd,
+ stdout=asyncio.subprocess.PIPE,
+ stderr=asyncio.subprocess.STDOUT,
+ )
+ except FileNotFoundError as exc:
+ return 127, str(exc)
+
+ try:
+ stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=timeout)
+ except asyncio.TimeoutError:
+ proc.terminate()
+ try:
+ await asyncio.wait_for(proc.wait(), timeout=5)
+ except asyncio.TimeoutError:
+ proc.kill()
+ await proc.wait()
+ return 124, f"command timed out after {timeout}s"
+
+ return proc.returncode, stdout.decode(errors="replace") if stdout else ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/containers/apple_backend.py` around lines 27 - 35, The _run
method currently lets FileNotFoundError and asyncio.TimeoutError propagate and
can leave the child process running; update async def _run(self, cmd: list[str],
timeout: int = 120) to catch FileNotFoundError and asyncio.TimeoutError and
always return a normalized (int, str) tuple: on FileNotFoundError return
non-zero code (e.g., 127) with a clear message including the missing
binary/self.binary; on asyncio.TimeoutError ensure you kill and await the
subprocess (proc.kill(); await proc.wait()) to avoid zombies and then return a
non-zero code (e.g., 124) with a timeout message; also catch unexpected
exceptions and return (1, str(exc)) so all callers of _run see a consistent
(code, output) response.
Comment on lines
+72
to
+109
| async def create_container( | ||
| self, | ||
| name: str, | ||
| image: str = "docker.io/library/debian:bookworm", | ||
| memory_limit: str | None = None, | ||
| cpu_limit: int | None = None, | ||
| mounts: list[tuple[str, str]] | None = None, | ||
| env: dict[str, str] | None = None, | ||
| host_uid: int | None = None, | ||
| root_size_gib: int | None = None, | ||
| ) -> dict: | ||
| argv = [self.binary, "run", "-d", "--name", name] | ||
| if memory_limit: | ||
| # Convert "2GB"/"512MB" → "2g"/"512m" for Apple CLI | ||
| ml = memory_limit.strip().lower().replace("gb", "g").replace("mb", "m") | ||
| argv += ["--memory", ml] | ||
| if cpu_limit: | ||
| argv += ["--cpus", str(cpu_limit)] | ||
| for host_path, guest_path in mounts or []: | ||
| argv += ["-v", f"{host_path}:{guest_path}"] | ||
| for key, value in (env or {}).items(): | ||
| argv += ["-e", f"{key}={value}"] | ||
| argv.append(image) | ||
|
|
||
| code, output = await self._run(argv) | ||
| if code != 0: | ||
| return {"success": False, "output": output} | ||
|
|
||
| if root_size_gib is not None: | ||
| quota_result = await self.set_root_quota(name, root_size_gib) | ||
| if isinstance(quota_result, dict) and not quota_result.get("success"): | ||
| logger.warning( | ||
| "set_root_quota for %s did not succeed: %s", | ||
| name, | ||
| quota_result.get("note") or quota_result.get("output"), | ||
| ) | ||
|
|
||
| return {"success": True, "output": output.strip()} |
There was a problem hiding this comment.
Don’t silently ignore host_uid.
ContainerBackend.create_container() advertises UID mapping support, but this implementation drops host_uid completely. If callers rely on that for bind-mounted host paths, Apple containers will start with different ownership semantics and no signal that the request was ignored.
Suggested fix
) -> dict:
argv = [self.binary, "run", "-d", "--name", name]
+ if host_uid is not None:
+ return {
+ "success": False,
+ "output": "",
+ "note": "apple container backend does not support host_uid mapping yet",
+ }
if memory_limit:
# Convert "2GB"/"512MB" → "2g"/"512m" for Apple CLI
ml = memory_limit.strip().lower().replace("gb", "g").replace("mb", "m")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/containers/apple_backend.py` around lines 72 - 109, The
create_container implementation currently ignores the host_uid parameter; update
create_container to apply host_uid when provided (so bind-mounted files keep
expected ownership) by adding logic that sets the container user/UID before
launching: inside create_container (the function shown) if host_uid is not None,
append the appropriate CLI option (e.g., "--user", str(host_uid)) to argv (or,
if the Apple CLI uses a different flag for UID mapping, use that flag) so the
created container runs with the requested UID; ensure this branch executes
before argv.append(image) so the flag is included in the run command and keep
existing behavior for root_size_gib and set_root_quota unchanged.
Build pipeline reproducibility: - build_python.sh: version-specific checksum file path; remove unpinned pip upgrade so requirements.lock alone controls package versions - fetch_container_cli.sh: version-specific checksum file path - checksums/: rename existing files to include version suffix - sign.sh: replace `xargs ... 2>/dev/null || true` (which swallowed codesign failures) with explicit while-read loop - sparkle_sign.sh: fail fast when sparkle:edSignature parse yields empty Swift launcher correctness: - Package.swift: bump deployment target .v14 -> .v26 to match Info.plist LSMinimumSystemVersion; bump swift-tools-version to 6.2 for the .v26 constant - App.swift: observe NSWindow.willCloseNotification on NotificationCenter.default (NSWorkspace.notificationCenter only delivers workspace-level notifications); filter to the taOS main window so Sparkle / future auxiliary windows can't reset activation state. Mark TaOSAppDelegate @mainactor for Swift 6 strict concurrency - WindowController.swift: expose `window` as public(get) so the close filter can compare instances - ServerProcess.swift: mark @unchecked Sendable; lifecycle is single-threaded under the launcher - SparkleBridge.swift: treat empty SUPublicEDKey as unavailable, not just nil Python backend: - __main__.py: load server config from the same data_dir passed to create_app() so TAOS_DATA_DIR doesn't desync host/port from app config - routes/settings.py: accept "apple" as a valid container-runtime value and instantiate AppleContainerBackend so users can switch via the API - apple_backend.py: handle FileNotFoundError + asyncio.TimeoutError in _run() instead of crashing; explicitly terminate the subprocess on timeout to avoid leaking child processes; reject host_uid in create_container() rather than silently dropping it Tests: - test_routes_settings.py: cover apple runtime acceptance
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
tests/test_routes_settings.py (1)
107-115: Strengthen the apple-runtime assertion.Consider asserting
resp.json()["runtime"] == "apple"as well, so the test verifies both status and selected runtime value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_routes_settings.py` around lines 107 - 115, The test test_set_apple_runtime should also assert the returned runtime value: after calling client.put on "/api/settings/container-runtime" and verifying resp.status_code and resp.json()["status"], add an assertion that resp.json()["runtime"] == "apple" to ensure the API response includes the selected runtime; update the test_set_apple_runtime function to check resp.json()["runtime"] in addition to resp.json()["status"].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mac/build/sparkle_sign.sh`:
- Around line 10-17: The case arms in sparkle_sign.sh currently dereference $2
for --dmg, --version and --output while set -u is enabled, causing an
unbound-variable error when an option is passed without a value; update each
option handler in the while/case block to guard before assigning DMG, VERSION or
OUTPUT by checking there is a next argument (e.g. verify $# -ge 2 and that $2
does not start with '-' or is non-empty) and emit the same friendly "all args
required" style error and exit if the value is missing; keep the later
validation logic intact and only set the variables after the guard passes.
- Around line 48-55: The NOTES variable is injected raw into a CDATA block which
breaks if the changelog contains the sequence "]]>"; before writing to SNIPPET,
transform NOTES by replacing every literal "]]>" with a sequence that closes and
reopens the CDATA section (i.e. replace "]]>" with "]]]]><![CDATA[>") so the XML
stays well-formed, then write the transformed NOTES into the heredoc used to
create SNIPPET (adjust references to NOTES, SNIPPET, NOTES_FILE, and VERSION in
the script accordingly).
In `@mac/launcher/Sources/taOSLauncher/App.swift`:
- Around line 88-97: The child process environment currently overwrites the
entire environment (see the env dictionary and where proc.environment is set),
which drops important parent vars like DYLD_LIBRARY_PATH, TMPDIR, HOME; fix by
merging the parent environment (ProcessInfo.processInfo.environment) with the
new keys (e.g., "PYTHONPATH", "PYTHONDONTWRITEBYTECODE", "TAOS_DATA_DIR",
"TAOS_HOST", "TAOS_PORT", "TAOS_CONTAINER_BIN") so the proc.environment
assignment uses a combined dictionary that prioritizes your explicit keys but
preserves inherited variables; update the code around env and the
proc.environment assignment to build this merged dictionary before setting
proc.environment.
In `@mac/launcher/Sources/taOSLauncher/ServerProcess.swift`:
- Around line 39-50: The code opens a FileHandle named handle and assigns it to
self.logHandle before starting the Process, but if proc.run() throws the handle
is leaked and self.logHandle remains set; update the launch sequence in
ServerProcess (where handle, self.logHandle, proc, and proc.run() are used) to
ensure the file handle is closed and self.logHandle cleared on failure: either
move assignment to self.logHandle until after proc.run() succeeds, or wrap
proc.run() in a do/catch with a defer/cleanup that closes handle and sets
self.logHandle = nil when an error is thrown, and only set self.process = proc
after a successful run.
In `@tinyagentos/__main__.py`:
- Around line 28-31: The code currently parses env_port into port with
int(env_port) which can raise ValueError and crash startup; update the parsing
around env_host/env_port (variables env_host, env_port and the assignments to
host and port) to validate env_port with a guarded parse (try/except or using
str.isdigit()/safe conversion), fall back to the default 6969 on invalid input,
and emit a clear warning or error message via the existing logger before
proceeding so malformed TAOS_PORT does not abort startup.
In `@tinyagentos/containers/apple_backend.py`:
- Around line 65-83: list_containers() assumes the CLI JSON is a list of dicts
so calling it.get(...) can raise if items is not a list or an element is not a
dict; before iterating, validate that parsed items is a list and then for each
element ensure it's a dict (skip or log non-dict entries), then safely extract
fields for ContainerInfo (name, status, ip, memory, cpus) using the existing
defaulting logic; reference the variables/items and the ContainerInfo
construction in list_containers to locate where to add isinstance checks and
minimal logging for unexpected shapes.
- Around line 192-222: snapshot_create and snapshot_list currently use global
"taos/{snapshot}" references causing cross-container collisions; update
snapshot_create to tag images with the container name (e.g.,
"taos/{name}/{snapshot_name}:latest" instead of "taos/{snapshot_name}:latest")
and update snapshot_list to filter refs that start with f"taos/{name}/" and
extract the snapshot tag by removing that prefix and trimming any ":<tag>"
suffix (use the existing local variables name, snapshot_name and the ref field
from the parsed JSON). Keep snapshot_restore unchanged.
---
Nitpick comments:
In `@tests/test_routes_settings.py`:
- Around line 107-115: The test test_set_apple_runtime should also assert the
returned runtime value: after calling client.put on
"/api/settings/container-runtime" and verifying resp.status_code and
resp.json()["status"], add an assertion that resp.json()["runtime"] == "apple"
to ensure the API response includes the selected runtime; update the
test_set_apple_runtime function to check resp.json()["runtime"] in addition to
resp.json()["status"].
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 86339b60-bd85-41c7-a616-8d8bc9d18349
📒 Files selected for processing (15)
mac/build/build_python.shmac/build/checksums/apple-container-cli-0.12.0.sha256mac/build/checksums/python-build-standalone-3.12.13+20260414.sha256mac/build/fetch_container_cli.shmac/build/sign.shmac/build/sparkle_sign.shmac/launcher/Package.swiftmac/launcher/Sources/taOSLauncher/App.swiftmac/launcher/Sources/taOSLauncher/ServerProcess.swiftmac/launcher/Sources/taOSLauncher/SparkleBridge.swiftmac/launcher/Sources/taOSLauncher/WindowController.swifttests/test_routes_settings.pytinyagentos/__main__.pytinyagentos/containers/apple_backend.pytinyagentos/routes/settings.py
✅ Files skipped from review due to trivial changes (4)
- mac/build/checksums/apple-container-cli-0.12.0.sha256
- mac/build/checksums/python-build-standalone-3.12.13+20260414.sha256
- mac/build/sign.sh
- mac/launcher/Sources/taOSLauncher/SparkleBridge.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- mac/build/build_python.sh
- mac/launcher/Package.swift
- mac/build/fetch_container_cli.sh
Comment on lines
+10
to
+17
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --dmg) DMG="$2"; shift 2 ;; | ||
| --version) VERSION="$2"; shift 2 ;; | ||
| --output) OUTPUT="$2"; shift 2 ;; | ||
| *) echo "sparkle_sign.sh: unknown arg $1" >&2; exit 2 ;; | ||
| esac | ||
| done |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Verify current parser branches read $2 directly.
sed -n '10,17p' mac/build/sparkle_sign.sh
# Expected after fix: each value-taking flag checks $# / ${2:-} before assignment.Repository: jaylfc/tinyagentos
Length of output: 285
🏁 Script executed:
head -20 mac/build/sparkle_sign.shRepository: jaylfc/tinyagentos
Length of output: 559
🏁 Script executed:
# Check if there are any other argument parsing patterns in the file
tail -n +20 mac/build/sparkle_sign.sh | head -50Repository: jaylfc/tinyagentos
Length of output: 1818
Guard option values before dereferencing $2.
With set -u at line 5, accessing an unbound $2 when an option lacks a value causes a shell unbound-variable error before the validation at line 19–20 can trigger a friendly error message. For example, ./sparkle_sign.sh --dmg (missing value) fails with an unhelpful error instead of the expected "all args required" message.
Proposed fix
while [[ $# -gt 0 ]]; do
case "$1" in
- --dmg) DMG="$2"; shift 2 ;;
- --version) VERSION="$2"; shift 2 ;;
- --output) OUTPUT="$2"; shift 2 ;;
+ --dmg|--version|--output)
+ [[ $# -ge 2 && -n "${2:-}" && "${2:0:1}" != "-" ]] || {
+ echo "sparkle_sign.sh: $1 requires a value" >&2
+ exit 2
+ }
+ case "$1" in
+ --dmg) DMG="$2" ;;
+ --version) VERSION="$2" ;;
+ --output) OUTPUT="$2" ;;
+ esac
+ shift 2
+ ;;
*) echo "sparkle_sign.sh: unknown arg $1" >&2; exit 2 ;;
esac
done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mac/build/sparkle_sign.sh` around lines 10 - 17, The case arms in
sparkle_sign.sh currently dereference $2 for --dmg, --version and --output while
set -u is enabled, causing an unbound-variable error when an option is passed
without a value; update each option handler in the while/case block to guard
before assigning DMG, VERSION or OUTPUT by checking there is a next argument
(e.g. verify $# -ge 2 and that $2 does not start with '-' or is non-empty) and
emit the same friendly "all args required" style error and exit if the value is
missing; keep the later validation logic intact and only set the variables after
the guard passes.
Comment on lines
+48
to
+55
| NOTES="$(awk -v v="$VERSION" 'BEGIN{p=0} /^## /{p=($2==v)} p' "$NOTES_FILE" 2>/dev/null || echo "")" | ||
|
|
||
| cat > "$SNIPPET" <<XML | ||
| <item> | ||
| <title>v${VERSION}</title> | ||
| <sparkle:version>${VERSION}</sparkle:version> | ||
| <sparkle:minimumSystemVersion>26.0</sparkle:minimumSystemVersion> | ||
| <description><![CDATA[${NOTES}]]></description> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Check whether CHANGELOG currently contains the CDATA terminator sequence.
rg -n '\]\]>' CHANGELOG.md || true
# Any match indicates current generation can emit invalid XML without escaping.Repository: jaylfc/tinyagentos
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Locate sparkle_sign.sh
find . -name "sparkle_sign.sh" -type fRepository: jaylfc/tinyagentos
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Find changelog/release notes files
find . -name "*CHANGE*" -o -name "*changelog*" -o -name "*RELEASE*" -o -name "*NEWS*" | head -20Repository: jaylfc/tinyagentos
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Examine the sparkle_sign.sh file content
if [ -f "mac/build/sparkle_sign.sh" ]; then
cat -n "mac/build/sparkle_sign.sh" | sed -n '40,65p'
fiRepository: jaylfc/tinyagentos
Length of output: 1110
🏁 Script executed:
#!/bin/bash
# Find CHANGELOG.md
find . -name "CHANGELOG.md" -type fRepository: jaylfc/tinyagentos
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check for ]]> in any changelog/release files
rg '\]\]>' . --type md 2>/dev/null || echo "No ]]> sequences found"Repository: jaylfc/tinyagentos
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Verify the CDATA escaping strategy - check if the fix is valid XML-wise
cat > /tmp/test_cdata.sh <<'EOF'
#!/bin/bash
# Test: What happens when ]]> appears in CDATA?
test_notes="This has ]]> in it"
# Original (vulnerable)
echo "=== Original (vulnerable) ==="
cat > /tmp/test1.xml <<XMLEND
<description><![CDATA[${test_notes}]]></description>
XMLEND
cat /tmp/test1.xml
echo ""
# Proposed fix escaping ]]> as ]]]]><![CDATA[>
test_notes_escaped="${test_notes//]]>/]]]]><![CDATA[>}"
echo "=== With proposed escaping ==="
cat > /tmp/test2.xml <<XMLEND
<description><![CDATA[${test_notes_escaped}]]></description>
XMLEND
cat /tmp/test2.xml
echo ""
# Check if proposed fix creates valid XML structure
echo "=== Analysis ==="
echo "Proposed escaping breaks ]]> as: ]]]]><![CDATA[>"
echo "This closes CDATA with ]], then adds >], then reopens with <![CDATA[>"
echo "Net result in output: ]]>"
EOF
bash /tmp/test_cdata.shRepository: jaylfc/tinyagentos
Length of output: 415
Escape ]]> sequences in release notes before embedding in CDATA.
Line 55 injects raw NOTES into CDATA. If changelog text ever contains ]]>, the generated XML becomes malformed. The proposed fix correctly handles this edge case by breaking the CDATA section, outputting the literal sequence, and reopening a new CDATA section.
Proposed fix
NOTES="$(awk -v v="$VERSION" 'BEGIN{p=0} /^## /{p=($2==v)} p' "$NOTES_FILE" 2>/dev/null || echo "")"
+NOTES_CDATA_SAFE="${NOTES//]]>/]]]]><![CDATA[>}"
@@
- <description><![CDATA[${NOTES}]]></description>
+ <description><![CDATA[${NOTES_CDATA_SAFE}]]></description>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mac/build/sparkle_sign.sh` around lines 48 - 55, The NOTES variable is
injected raw into a CDATA block which breaks if the changelog contains the
sequence "]]>"; before writing to SNIPPET, transform NOTES by replacing every
literal "]]>" with a sequence that closes and reopens the CDATA section (i.e.
replace "]]>" with "]]]]><![CDATA[>") so the XML stays well-formed, then write
the transformed NOTES into the heredoc used to create SNIPPET (adjust references
to NOTES, SNIPPET, NOTES_FILE, and VERSION in the script accordingly).
Comment on lines
+88
to
+97
| let env: [String: String] = [ | ||
| "PYTHONPATH": taosRoot.path, | ||
| // Keep .pyc files out of the bundle — Python writes them next to | ||
| // sources by default, which would invalidate the codesign seal. | ||
| "PYTHONDONTWRITEBYTECODE": "1", | ||
| "TAOS_DATA_DIR": dataDir.path, | ||
| "TAOS_HOST": "127.0.0.1", | ||
| "TAOS_PORT": "\(port)", | ||
| "TAOS_CONTAINER_BIN": containerBin.path, | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify child env is currently replaced (not merged) and locate both call sites.
rg -n -C2 --type=swift 'let env: \[String: String\] = \[|ProcessInfo\.processInfo\.environment|proc\.environment = env'Repository: jaylfc/tinyagentos
Length of output: 1493
🏁 Script executed:
# Examine full context in App.swift around the env setup
sed -n '70,100p' mac/launcher/Sources/taOSLauncher/App.swiftRepository: jaylfc/tinyagentos
Length of output: 1306
🏁 Script executed:
# Check ServerProcess.swift to understand how env is used
cat -n mac/launcher/Sources/taOSLauncher/ServerProcess.swiftRepository: jaylfc/tinyagentos
Length of output: 4260
🏁 Script executed:
# Check if there are any other references to environment or subprocess calls
rg -n --type=swift 'environment|Process|executable' mac/launcher/Sources/taOSLauncher/Repository: jaylfc/tinyagentos
Length of output: 1422
🌐 Web query:
Foundation Process environment assignment behavior Swift - does assigning to proc.environment replace or merge with parent environment
💡 Result:
Assigning to proc.environment replaces the entire environment of the Process instance with the new dictionary provided. It does not merge with the parent (current process) environment. By default, a newly initialized Process inherits a copy of the current process's environment (via ProcessInfo.processInfo.environment). Setting proc.environment = someDict overrides this completely with someDict, using only the variables in that dictionary for the subprocess. To modify specific variables while preserving others, copy the parent's environment first, update it, then assign: var env = ProcessInfo.processInfo.environment env["MY_VAR"] = "value" proc.environment = env
Citations:
- 1: https://developer.apple.com/documentation/Foundation/Process
- 2: https://developer.apple.com/documentation/foundation/process/environment
- 3: https://developer.apple.com/documentation/foundation/process
Preserve parent environment when constructing child process env.
proc.environment = env (ServerProcess.swift:46) replaces the entire child process environment with only the 8 specified variables. This breaks runtime features that depend on inherited system environment variables, notably dynamic library search paths on macOS (DYLD_LIBRARY_PATH), TMPDIR, and HOME. Even bundled Python typically requires these system variables to function correctly.
Merge the parent environment into the new dictionary instead:
Suggested fix
- let env: [String: String] = [
- "PYTHONPATH": taosRoot.path,
- // Keep .pyc files out of the bundle — Python writes them next to
- // sources by default, which would invalidate the codesign seal.
- "PYTHONDONTWRITEBYTECODE": "1",
- "TAOS_DATA_DIR": dataDir.path,
- "TAOS_HOST": "127.0.0.1",
- "TAOS_PORT": "\(port)",
- "TAOS_CONTAINER_BIN": containerBin.path,
- ]
+ var env = ProcessInfo.processInfo.environment
+ env["PYTHONPATH"] = taosRoot.path
+ // Keep .pyc files out of the bundle — Python writes them next to
+ // sources by default, which would invalidate the codesign seal.
+ env["PYTHONDONTWRITEBYTECODE"] = "1"
+ env["TAOS_DATA_DIR"] = dataDir.path
+ env["TAOS_HOST"] = "127.0.0.1"
+ env["TAOS_PORT"] = "\(port)"
+ env["TAOS_CONTAINER_BIN"] = containerBin.path📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let env: [String: String] = [ | |
| "PYTHONPATH": taosRoot.path, | |
| // Keep .pyc files out of the bundle — Python writes them next to | |
| // sources by default, which would invalidate the codesign seal. | |
| "PYTHONDONTWRITEBYTECODE": "1", | |
| "TAOS_DATA_DIR": dataDir.path, | |
| "TAOS_HOST": "127.0.0.1", | |
| "TAOS_PORT": "\(port)", | |
| "TAOS_CONTAINER_BIN": containerBin.path, | |
| ] | |
| var env = ProcessInfo.processInfo.environment | |
| env["PYTHONPATH"] = taosRoot.path | |
| // Keep .pyc files out of the bundle — Python writes them next to | |
| // sources by default, which would invalidate the codesign seal. | |
| env["PYTHONDONTWRITEBYTECODE"] = "1" | |
| env["TAOS_DATA_DIR"] = dataDir.path | |
| env["TAOS_HOST"] = "127.0.0.1" | |
| env["TAOS_PORT"] = "\(port)" | |
| env["TAOS_CONTAINER_BIN"] = containerBin.path |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mac/launcher/Sources/taOSLauncher/App.swift` around lines 88 - 97, The child
process environment currently overwrites the entire environment (see the env
dictionary and where proc.environment is set), which drops important parent vars
like DYLD_LIBRARY_PATH, TMPDIR, HOME; fix by merging the parent environment
(ProcessInfo.processInfo.environment) with the new keys (e.g., "PYTHONPATH",
"PYTHONDONTWRITEBYTECODE", "TAOS_DATA_DIR", "TAOS_HOST", "TAOS_PORT",
"TAOS_CONTAINER_BIN") so the proc.environment assignment uses a combined
dictionary that prioritizes your explicit keys but preserves inherited
variables; update the code around env and the proc.environment assignment to
build this merged dictionary before setting proc.environment.
Comment on lines
+39
to
+50
| let handle = try FileHandle(forWritingTo: logFile) | ||
| try handle.seekToEnd() | ||
| self.logHandle = handle | ||
|
|
||
| let proc = Process() | ||
| proc.executableURL = executable | ||
| proc.arguments = arguments | ||
| proc.environment = env | ||
| proc.standardOutput = handle | ||
| proc.standardError = handle | ||
| try proc.run() | ||
| self.process = proc |
There was a problem hiding this comment.
Close log handle on process launch failure.
If proc.run() throws, the opened log handle is never closed, causing a resource leak and stale logHandle state.
💡 Suggested fix
let proc = Process()
proc.executableURL = executable
proc.arguments = arguments
proc.environment = env
proc.standardOutput = handle
proc.standardError = handle
- try proc.run()
- self.process = proc
+ do {
+ try proc.run()
+ self.process = proc
+ } catch {
+ try? handle.close()
+ self.logHandle = nil
+ throw error
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let handle = try FileHandle(forWritingTo: logFile) | |
| try handle.seekToEnd() | |
| self.logHandle = handle | |
| let proc = Process() | |
| proc.executableURL = executable | |
| proc.arguments = arguments | |
| proc.environment = env | |
| proc.standardOutput = handle | |
| proc.standardError = handle | |
| try proc.run() | |
| self.process = proc | |
| let handle = try FileHandle(forWritingTo: logFile) | |
| try handle.seekToEnd() | |
| self.logHandle = handle | |
| let proc = Process() | |
| proc.executableURL = executable | |
| proc.arguments = arguments | |
| proc.environment = env | |
| proc.standardOutput = handle | |
| proc.standardError = handle | |
| do { | |
| try proc.run() | |
| self.process = proc | |
| } catch { | |
| try? handle.close() | |
| self.logHandle = nil | |
| throw error | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mac/launcher/Sources/taOSLauncher/ServerProcess.swift` around lines 39 - 50,
The code opens a FileHandle named handle and assigns it to self.logHandle before
starting the Process, but if proc.run() throws the handle is leaked and
self.logHandle remains set; update the launch sequence in ServerProcess (where
handle, self.logHandle, proc, and proc.run() are used) to ensure the file handle
is closed and self.logHandle cleared on failure: either move assignment to
self.logHandle until after proc.run() succeeds, or wrap proc.run() in a do/catch
with a defer/cleanup that closes handle and sets self.logHandle = nil when an
error is thrown, and only set self.process = proc after a successful run.
Comment on lines
+28
to
+31
| if env_host or env_port: | ||
| host = env_host or "127.0.0.1" | ||
| port = int(env_port) if env_port else 6969 | ||
| else: |
There was a problem hiding this comment.
Handle invalid TAOS_PORT values without crashing startup.
Line 30 can raise ValueError if TAOS_PORT is malformed, which aborts startup. Prefer a guarded parse with fallback or a clear error message.
💡 Suggested fix
if env_host or env_port:
host = env_host or "127.0.0.1"
- port = int(env_port) if env_port else 6969
+ if env_port:
+ try:
+ port = int(env_port)
+ except ValueError:
+ raise ValueError(f"Invalid TAOS_PORT: {env_port!r}")
+ else:
+ port = 6969🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/__main__.py` around lines 28 - 31, The code currently parses
env_port into port with int(env_port) which can raise ValueError and crash
startup; update the parsing around env_host/env_port (variables env_host,
env_port and the assignments to host and port) to validate env_port with a
guarded parse (try/except or using str.isdigit()/safe conversion), fall back to
the default 6969 on invalid input, and emit a clear warning or error message via
the existing logger before proceeding so malformed TAOS_PORT does not abort
startup.
Comment on lines
+65
to
+83
| items = json.loads(output) if output.strip() else [] | ||
| except json.JSONDecodeError: | ||
| logger.error("apple container ls returned non-JSON: %s", output[:200]) | ||
| return [] | ||
|
|
||
| results: list[ContainerInfo] = [] | ||
| for it in items: | ||
| name = it.get("name", "") | ||
| if not name.startswith(prefix): | ||
| continue | ||
| results.append( | ||
| ContainerInfo( | ||
| name=name, | ||
| status=it.get("status", "unknown"), | ||
| ip=it.get("ip"), | ||
| memory_mb=_parse_memory(str(it.get("memory", "0"))), | ||
| cpu_cores=int(it.get("cpus", 0) or 0), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Guard JSON shape before field access in list_containers().
If the CLI returns non-list JSON (or non-dict items), it.get(...) can fail at runtime. Add lightweight type guards before iterating/accessing keys.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/containers/apple_backend.py` around lines 65 - 83,
list_containers() assumes the CLI JSON is a list of dicts so calling it.get(...)
can raise if items is not a list or an element is not a dict; before iterating,
validate that parsed items is a list and then for each element ensure it's a
dict (skip or log non-dict entries), then safely extract fields for
ContainerInfo (name, status, ip, memory, cpus) using the existing defaulting
logic; reference the variables/items and the ContainerInfo construction in
list_containers to locate where to add isinstance checks and minimal logging for
unexpected shapes.
Comment on lines
+192
to
+222
| async def snapshot_create(self, name: str, snapshot_name: str) -> dict: | ||
| code, output = await self._run( | ||
| [self.binary, "commit", name, f"taos/{snapshot_name}:latest"] | ||
| ) | ||
| return {"success": code == 0, "output": output} | ||
|
|
||
| async def snapshot_restore(self, name: str, snapshot_name: str) -> dict: | ||
| return { | ||
| "success": False, | ||
| "output": "", | ||
| "note": "apple container snapshot restore not supported", | ||
| } | ||
|
|
||
| async def snapshot_list(self, name: str) -> dict: | ||
| code, output = await self._run( | ||
| [self.binary, "images", "ls", "--format", "json"] | ||
| ) | ||
| if code != 0: | ||
| return {"success": False, "snapshots": [], "output": output} | ||
| try: | ||
| items = json.loads(output) if output.strip() else [] | ||
| except json.JSONDecodeError: | ||
| return {"success": False, "snapshots": [], "output": output} | ||
|
|
||
| snapshots: list[str] = [] | ||
| for it in items: | ||
| ref = it.get("reference", "") | ||
| if ref.startswith("taos/"): | ||
| tag = ref[len("taos/"):].split(":", 1)[0] | ||
| snapshots.append(tag) | ||
| return {"success": True, "snapshots": snapshots, "output": output} |
There was a problem hiding this comment.
Scope snapshots to the container name.
snapshot_create() and snapshot_list(name=...) are currently global (taos/*) and ignore container scoping, so snapshots from different containers can collide or be mislisted.
💡 Suggested fix
async def snapshot_create(self, name: str, snapshot_name: str) -> dict:
+ ref = f"taos/{name}:{snapshot_name}"
code, output = await self._run(
- [self.binary, "commit", name, f"taos/{snapshot_name}:latest"]
+ [self.binary, "commit", name, ref]
)
return {"success": code == 0, "output": output}
@@
async def snapshot_list(self, name: str) -> dict:
@@
- snapshots: list[str] = []
+ snapshots: list[str] = []
+ prefix = f"taos/{name}:"
for it in items:
ref = it.get("reference", "")
- if ref.startswith("taos/"):
- tag = ref[len("taos/"):].split(":", 1)[0]
- snapshots.append(tag)
+ if ref.startswith(prefix):
+ snapshots.append(ref[len(prefix):])
return {"success": True, "snapshots": snapshots, "output": output}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/containers/apple_backend.py` around lines 192 - 222,
snapshot_create and snapshot_list currently use global "taos/{snapshot}"
references causing cross-container collisions; update snapshot_create to tag
images with the container name (e.g., "taos/{name}/{snapshot_name}:latest"
instead of "taos/{snapshot_name}:latest") and update snapshot_list to filter
refs that start with f"taos/{name}/" and extract the snapshot tag by removing
that prefix and trimming any ":<tag>" suffix (use the existing local variables
name, snapshot_name and the ref field from the parsed JSON). Keep
snapshot_restore unchanged.
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
1 participant
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.
Summary
First end-to-end take on the Mac distribution: a self-contained
taOS.appbundle for Apple Silicon + macOS 26 Tahoe.End-to-end smoke test (local)
Built and launched on this branch:
/Library/Application Support/taOS/`, server log at `/Library/Logs/taOS/server.log`Known gaps (not blocking the C1+A milestone)
Test plan
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests