fix(compile): honor author-declared mode: read on exec nodes (#165)#166
Merged
Conversation
Node had no mode field so serde dropped node-level mode: read on parse. compile_node then hard-coded write for any command not found in the agent manifest (e.g. exec). Fix: add mode: Option<Mode> to Node and check it before falling back to safe-default write-mode in both the command-not-found and agent-not-installed branches. When the author declares mode: read, the lock records read and notes the declaration was honored; when no mode is declared, the prior safe default applies. Two unit tests guard both sides. Also adds allow(dead_code) to the pre-existing unused panel field in the same file.
Codex review on the compiler fix noted the lock recorded mode: read for a declared exec node, but runtime::orchestrator::lookup_command_mode still fell back to Mode::Write for the same unknown command — so under aware app run --dry-run / --simulate the node was short-circuited as a write, diverging from the lock it was compiled into. Thread the node's declared mode into lookup_command_mode so it resolves the otherwise-unknowable mode before the write-mode safety default, mirroring compile_node exactly. Both dry-run/simulate call sites pass node.mode. Adds a runtime test asserting a declared-read exec node is NOT emitted as a would-write under dry-run.
3 tasks
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
Fixes #165 — an author-declared node-level
mode: readon a hostexecnode was silently overridden tomode: writeat compile, with only a misleading "defaulting … for safety" note.Root cause:
Node(manifest/app.rs) had nomodefield, so serde dropped the documentedmode:declaration on parse.compile_nodethen hard-codedwritefor any command it couldn't resolve from the agent manifest (e.g.exec, whose read/write semantics are unknowable from the manifest alone).This implements the issue's preferred option (1): honor it.
Changes
manifest/app.rs— addmode: Option<Mode>toNodeso the author declaration survives deserialization.app_lock.rs(compile_node) — when the command can't be inferred (command-not-found or agent-not-installed), honornode.modebefore the write-mode default. The lock records the declared mode and the note states the declaration was honored (no misleading "defaulting" message). Absent any declaration, the prior safe write-mode default is unchanged.runtime/orchestrator.rs(lookup_command_mode) — threadnode.modethrough soaware app run --dry-run/--simulateresolves the same mode as the lock, instead of short-circuiting a declared-readexecnode as a would-write. (This second commit addresses a Codex review finding on the first.)Test plan
compile_honors_author_declared_mode_on_unknown_command— lock recordsmode: read, no "defaulting" note.compile_defaults_write_mode_on_unknown_command_without_declaration— regression guard: write-mode default still applies when nomode:is declared.dry_run_honors_author_declared_read_mode_on_unknown_command— runtime treats a declared-readexecnode as a read (noWouldWrite) under dry-run.