fix: honor node-level mode: read on mode-overridable exec nodes (#165)#168
Merged
Conversation
) The #166 fix only covered the branch where a command is absent from the agent manifest. But the tekla manifest now declares `exec` with a conservative `mode: write`, so both `validate_app_safety` and `compile_node` hit the "command found" path — which resolved mode solely from the manifest via `mode_of` and ignored the author's node-level `mode:` entirely. Result on aware 0.46.0: a read-only `tekla/exec` node declaring `mode: read` was refused at install with E_APP_WRITE_WITHOUT_SAFETY, and (if it compiled) the lock recorded `mode: write` with no signal that the declaration was dropped — exactly the footgun the issue flags, now escalated to a hard install failure. Fix (issue's preferred option 1): introduce a per-command manifest flag `mode-overridable: true` for commands whose read/write behavior is caller-determined (they run caller-supplied logic the agent can't inspect — `exec`). For such a command the manifest `mode:` is a conservative default, but an explicit node-level `mode:` wins. A new `Agent::effective_mode` resolver is the single source of truth shared by validate + compile (they previously drifted, which is what let the bug hide). For every other command the manifest mode stays authoritative, and a *conflicting* node-level `mode:` is now rejected loudly (E_APP_NODE_MODE_NOT_OVERRIDABLE) instead of being silently dropped — so the authoring field is never quietly ignored in either direction. - agent.rs: Command.mode_overridable, Agent::effective_mode, Mode::as_str - validate.rs: effective-mode gate + conflict rejection - app_lock.rs: effective-mode resolution + accurate override note (no more "defaulting to write-mode for safety" on a declared read) - tekla manifest: exec marked mode-overridable (keeps mode: write default) - app-spec.md: document node-level mode + mode-overridable semantics - bump 0.46.0 -> 0.47.0 Verified end-to-end: read-only exec app installs without a safety: block and compiles to `mode: read`. 6 new tests; full suite green.
) Codex review caught that the effective-mode resolution was applied in validate + compile but two other mode consumers still called `mode_of` directly, so a read-only `tekla/exec` node declaring `mode: read` was inconsistent across surfaces: - runtime::orchestrator::lookup_command_mode (--dry-run / --simulate) treated the manifest-declared `exec` as write and short-circuited it as `would-write` instead of running the read-only probe. - `aware app explain` / glass-box listed the same node under writes. Both now route through `Agent::effective_mode(cmd, node.mode)`, so an explicit node-level `mode:` wins on mode-overridable commands everywhere the lock + validator already honor it. Adds a dry-run test for the command-present mode-overridable path.
Codex review (round 2): the E_APP_NODE_MODE_NOT_OVERRIDABLE conflict check lived only in `validate_app_safety`, which `aware app compile`/`inspect` and `app run --dry-run` skip — so those paths still silently ignored a `mode: read` declared on a non-overridable write command, preserving the exact footgun for the issue's own repro command (`app compile`). Move the check into `validate_app_agents`, the agent-aware validator that runs on every lock-producing + run path (install, compile, inspect, validate, run, dry-run) and recurses into for-each/sweep `do:` bodies. `validate_app_safety` now only resolves the effective mode for the write-without-safety gate. Tests repointed + added (nested-body coverage); verified end-to-end that `aware app compile` refuses the conflict.
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. A read-only
tekla/execnode declaringmode: readwas refused at install withE_APP_WRITE_WITHOUT_SAFETY(and, if it compiled, the lock recordedmode: writewith no signal the declaration was dropped). The earlier #166 fix only handled the branch where a command is absent from the agent manifest — but the tekla manifest now declaresexecwith a conservativemode: write, so every consumer hit the "command found" path that resolved mode solely from the manifest and ignored the author's node-levelmode:.The fix (issue's preferred option 1)
mode-overridable: truefor commands whose read/write behavior is caller-determined — they run caller-supplied logic the agent can't inspect (e.g.execruns an arbitrary C# script). The manifestmode:becomes a conservative default; an explicit node-levelmode:wins.Agent::effective_mode— a single shared resolver, now the one source of truth used by validate, lock-compile, dry-run/simulate, and explain. These had drifted apart, which is what let the bug hide.E_APP_NODE_MODE_NOT_OVERRIDABLE— a node-levelmode:that conflicts with a non-overridable command is now rejected loudly (on every agent-aware path: install, compile, inspect, validate, run, dry-run; recurses intofor-each/sweepbodies) instead of being silently dropped. Closes the footgun in both directions.teklaexecmarkedmode-overridable(keepsmode: writedefault, so un-annotated exec nodes still fall under the safety contract).app-spec.mddocuments the semantics; version bumped 0.46.0 → 0.47.0.Behavior after the fix
exec(mode-overridable)mode: readmode: writemode:)A
mode: readon a genuinely-writing, non-overridable command (e.g.uda-set) → hard validation error.Review
codex exec review --base main); it caught two real consistency gaps (dry-run/explain consumers, and the conflict check being skipped by compile/inspect/dry-run) — both fixed in follow-up commits. Final round: clean, no findings.Test plan
cargo test— 502 passed, 0 failed (incl. new unit tests for compile, validate-conflict + nested body, dry-run override, write default)execapp installs without asafety:block and compiles tomode: readwith note "command exec is mode-overridable; using author-declared mode: read"aware app compilerefuses amode: readdeclared onuda-set(non-overridable write) withE_APP_NODE_MODE_NOT_OVERRIDABLECloses #165.