[zyrona-labs]

Second pass over the full (untruncated) review. Pass 1 (#62) already covered topo_sort, action.yml input injection and the wheel glob; those are not touched.

Fixed

• src/agent/exec.rs: a guest command killed by a signal was reported with exitcode 1. It now reports 128+signal (shell convention), so callers can tell a SIGKILL from an ordinary failure.
• install.sh: the systemd unit was written to the predictable path /tmp/guestkit-worker.service and then copied with sudo (symlink/race risk); it now uses mktemp with cleanup. ExecStart also honours INSTALL_DIR instead of hardcoding /usr/local/bin.

Verified

• On the Mac mini: cargo test -p guestkit --features agent --lib exec::tests passes (2 tests). The new signal test fails without the fix (left: 1, right: 137) and passes with it.
• bash -n install.sh passes. The install.sh change was not run end to end.

Skipped

• fs_frozen in snapshot_live.rs always false: real, but correct detection needs a design decision.
• exec_sync has no timeout: larger behavioural change.
• Passport score parsing, fail-fast and extras-on-failure in action.yml; signing seed on the command line (needs a CLI change to read from env/file).
• install.sh User=nobody and ReadWritePaths/PrivateTmp unit issues: need unit design.
• Helm DSN encoding, hardcoded max-concurrent, cronjob auth, NetworkPolicy scope: chart changes I could not render-test here.
• Owner decisions, not changed: default role operator in rbac.rs, auth off by default, privileged docker-compose, hostPID worker, SECURITY.md contact and claims, LGPL text in a presentation, latest default version.
• Docs drift (worker README, python examples README, GUESTKIT_ZYVOR_ACCEPT, Chart.yaml version): not bug fixes.
• The proposed tests file is truncated mid-file; not used.

🤖 Generated with Claude Code

[github-actions]

AI review (advisory)

install.sh ~L56–58 (unquoted heredoc): Removing the quotes from <<EOF makes the entire unit file subject to shell expansion, not just $INSTALL_DIR. Any $, backticks, or \ elsewhere in the unit (e.g. an Environment= line or ExecStart args in the unchanged portion) will be silently expanded or mangled. Use a quoted heredoc and substitute $INSTALL_DIR explicitly (e.g. sed), or confirm the full unit contains no other $/backtick/backslash.

install.sh ~L67 ($INSTALL_DIR unvalidated in the diff): If $INSTALL_DIR is unset/empty in this script path, ExecStart becomes /guestkit-worker ... and the service fails at start. If it contains spaces, the unit line is also broken (needs quoting). Verify it's set and defaulted earlier in the script, or use ${INSTALL_DIR:?}.

install.sh ~L57 (mktemp perms vs. cp): mktemp creates the file mode 0600. cp to $SYSTEMD_DIR preserves no mode, so the installed file takes the default mode via umask under root, which is usually fine, but it's untested. A restrictive root umask (e.g. 077) would yield a 0600 unit file; consider install -m 644 "$UNIT_TMP" ....

install.sh ~L57 (trap ... EXIT clobbers any existing trap): This overwrites any EXIT trap set earlier in the script. Check for existing traps, or chain them.

src/agent/exec.rs (exit_code, L10–23): The signal fallback is 128 + sig as i64, but the #[cfg(unix)] block is the only place sig is used. On non-unix this is fine, but there's no test for the 1 fallback path (neither a code nor a signal), which is acceptable. No bug, but note that exec_sync_qga (L52+) still appears to use its own exit code handling; if it also used .code().unwrap_or(1), it is inconsistent with this change and lacks a test. Verify and apply exit_code() there too.

Missing tests:

• No test for the install.sh change (e.g. generated unit has the correct ExecStart path and that the temp file is cleaned up).
• No test covering exec_sync_qga with a signal-killed process (if it shares the same code path).