Skip to content

Fix topo-sort silent drops, action.yml injection, wheel glob - #62

Merged
zyrona-labs merged 1 commit into
mainfrom
fix/ai-review-findings
Oct 20, 2026
Merged

zyrona-labs merged 1 commit into
mainfrom
fix/ai-review-findings

Conversation

@zyrona-labs

Copy link
Copy Markdown
Member

Fixed

  • src/cli/plan/topo_sort.rs: operations in a cycle or with an unknown depends_on id were silently dropped from the apply plan; ordering of independent ops followed HashMap order. Now returns an error (cycle, unknown dep, duplicate id) and keeps plan order. Callers in apply.rs and executor_live.rs propagate with ?. Added 3 unit tests.
  • action.yml: inputs.handoff/rescue/rescue-user/sbom/version were interpolated straight into run: (script injection); moved to env:.
  • build_python.sh: wheel glob guestkit-*.whl -> zyvor_guestkit-*.whl (matches the distribution name in pyproject.toml).

Verified

On a Mac mini: cargo test --lib topo_sort -> 3 passed (debug build). action.yml parses as YAML. Not run: the full test suite, the Action itself, build_python.sh (maturin).

Skipped

  • Agent framing mismatch, fs_frozen, virtio_net_count, Helm DSN, RBAC/auth defaults, sign-key via argv, gate-step restructuring, install.sh, schema drift, docs/license items: unverified, design-level, or larger/riskier than this PR.
  • tests.md proposed tests: not applied.

🤖 Generated with Claude Code

…heel glob

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@zyrona-labs
zyrona-labs merged commit bf23153 into main Oct 20, 2026
20 of 32 checks passed
@github-actions

Copy link
Copy Markdown

AI review (advisory)

Missing tests

  • src/cli/plan/topo_sort.rs (new tests module): no test for the duplicate-id error path (bail!("duplicate operation id ...")), which is new behavior in this PR.
  • src/cli/plan/apply.rs:171 and src/cli/plan/executor_live.rs:95: the new ? propagation has no test showing apply() returns an error (and applies nothing) for a cyclic, unknown-dependency, or duplicate-id plan.
  • action.yml (rescue / rescue-user / handoff / sbom env refactor): no test or workflow case exercising inputs with shell metacharacters (e.g. $(...), quotes) to prove the injection fix.

Bugs/security

No concrete bugs or security issues found in the diff. The action.yml change correctly moves ${{ inputs.* }} out of the script body into env:, eliminating script injection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant