v8 Adversarial Review — new defects
critical | with: merge/replace semantics undefined for uses:tasks/x → uses:commands|workflows/y | §2.3 says step keys "override" the referenced task's keys and §2.2 says tasks/uses: tasks/x and task x itself is uses: commands/y with:{...} (or uses: workflows/y with:{...}), the spec never states whether the step's with: mapping merges key-by-key with task x's own with:, wholly replaces it, or is simply illegal (since with: "isn't cascaded" yet §2.3 implies task/step are "adjacent cascade layers"); the ambiguity is sharper because commands' with: fills {{name}}/$ARGUMENTS while workflows' with: maps to declared param flags, so "override" means something different depending on what x ultimately targets (refs: §2.2, §2.3, §4, §5.1 "resolves recursively once").
critical | hashVersion 5 preimage omits the persona snapshot | §4 item 2 introduces a frozen persona snapshot ({systemPrompt, toolPolicy, model, …valueFields}) resolved through the agent: selector and consumed by dispatch, explicitly to fix runner.ts:592-593 shipping raw agent-file bytes — but the §5.3 hashVersion-5 preimage table only lists "item / inputs / dispatch / invocation / schema | as v4," with no row for the persona snapshot; changing the agent: selector or the referenced persona's systemPrompt/toolPolicy (a writable, frequently-edited bundle asset) would not change the unit hash, silently reusing a completed journal row dispatched under the old system prompt — the exact staleness bug hashVersion 5 was introduced to close for run:/prose (refs: §4.2, §5.3 table, step-work.ts:376-392 current v4 preimage which this table is supposed to supersede).
critical | .yml tombstone hard-error has no remedy for read-only/third-party bundles | §8 says sync/doctor/lint "each hard-error naming the file and the fix" for any tasks/*.yml, "never installed and never silently skipped," with no carve-out for bundle writability — but the repo's own existing migrator (scripts/akm-migrate/migrate/legacy/task-target-ref-migration.ts:253-271) treats read-only bundles specially precisely because "the migration must not write into a read-only bundle" and a user cannot fix content in a bundle they don't own; v8's tombstone rule regresses that precedent by hard-erroring sync/doctor/lint for the whole bundle (or task) with a "fix" the operator (non-maintainer) cannot apply, and never says whether the error is per-task (skip just that task) or fails the entire sync/install (refs: §8 tombstone rule; scripts/akm-migrate/migrate/legacy/task-target-ref-migration.ts:57-64,253-271).
major | mapping-type value fields (extra_params) merge rule contradicts current deep-merge behavior | §4 states cascade fields are "merged per-field, nearest wins" with only env: singled out as concatenating; this reads as whole-value replace for extra_params (a mapping), but the current implementation (freeze.ts:211-216 mergedLlmOverrides) already deep-merges llm overrides across layers via deepMergeConfig. v8 doesn't say whether the new cascade module preserves deep-merge for mapping fields like extra_params or silently downgrades to whole-value replace — a real behavior change/regression risk left unstated (refs: §4 value-field table, src/workflows/ir/freeze.ts:211-216).
major | shell-kind IrInvocation union has no timeoutMs slot | §5.1/§5.2's discriminated union shows { kind: "agent", engine, … } vs { kind: "shell", script, shell, cwd } — the shell variant lists no timeout field, yet §4's value-field vocabulary explicitly includes timeout as legal on any step/task including run: shell work, and the current single-shape IrInvocation (schema.ts:82, 488-496) carries timeoutMs as a top-level required-ish field. The spec never states where a resolved shell-unit timeout is carried in IR v4, nor who enforces it (ShellUnitExecutor per §5.1, but with what deadline). (refs: §5.1, §5.2, §4 value fields, src/workflows/ir/schema.ts:82,488-496.)
major | orchestrator-owned shell units: no crash/restart semantics stated | §5.2 says shell units are "orchestrator-owned" and skip harness claiming ("a harness never claims shell work"), but the external driver protocol's claim/lease/reclaim machinery (native-executor.ts's claim-expiry/reclaim logic) exists specifically to recover from a crashed claimant; the spec never states what happens if the orchestrator process itself dies mid-Bun.spawn() for a shell unit — on restart, is the child process reattached, killed-and-retried (risking non-idempotent shell side effects), or left orphaned with a journal row stuck "dispatched"? This is unaddressed for the exact "races / blocked semantics / kill-abort paths" surface the spec claims to own (refs: §5.2, §5.1 ShellUnitExecutor, native-executor.ts claim/lease comments ~L87-103,645).
major | persona toolPolicy vs engine capability conflict has no resolution rule | §4 introduces both a persona snapshot with toolPolicy (item 2) and "capability notices" replacing kind-gates for engine-unconsumed fields (item 4), but never states what happens when a persona grants a tool (e.g. via agents/<name> toolPolicy) that the cascade-selected engine cannot support (e.g. an LLM engine with no tool-calling) — is that a lint/freeze notice like unconsumed value fields, a hard error, or silently inert? The two mechanisms (capability notices for value fields; provenance ceiling for persona tools) are never reconciled for this cross-cutting case explicitly called out as an attack surface.
major | improve-exclusion "by adapter policy" names no mechanism | §3 asserts "lint runs, improve is excluded by adapter policy" for task bodies, but no such exclusion mechanism (glob list, adapter capability flag, or improve-pipeline consult point) exists in the codebase today (grep for improve-exclusion hooks in src/core/adapter and src/improve returns nothing), and §9/§11 (M13) mark this "costed" without naming where in the improve pipeline the check lives — enforceability is asserted, not specified, leaving it plausible that a generic akm improve invocation over task-adapter-recognized files never consults "adapter policy" at all.
minor | H1-seeding collision when task body already has an H1 | §8's name: → body H1 migration doesn't say what happens when the source .yml's prompt: | block or an already-inlined file (prompt: ./file.md) already begins with its own # heading — silently prepend a duplicate name: H1, skip seeding, or error? Left unspecified despite the review brief calling this out explicitly.
minor | argv shell-escaping portability unaddressed for the POSIX/powershell split introduced by §2.1 | §8 says command: [argv…] → run: "with each element shell-escaped," but §2.1 states run: executes via sh on POSIX and powershell on Windows by default — a single shell-escaping strategy applied at migration time (necessarily POSIX-flavored, since [argv…] is a Unix convention) will not portably re-quote for PowerShell's escaping rules, so a migrated array-form command may silently misbehave on the Windows default shell with no shell: override emitted by the migrator.
minor | with.arguments overflow beyond $9 is unspecified | §2.4 defines with.arguments's whitespace-split words fill $1–$9; nothing states what happens to the 10th+ word (dropped silently vs concatenated into $9 vs left for $ARGUMENTS only) — a minor but real gap in an otherwise precisely specified filling contract.
minor | {{arguments}} named-placeholder vs reserved with.arguments key collision unaddressed | §2.4's two filling rules ("with: mapping keys fill {{name}}" and "the reserved key with.arguments … fills $ARGUMENTS") both key off the literal string "arguments"; if a command template also contains a literal {{arguments}} placeholder, it's unstated whether with: { arguments: "x" } fills that placeholder too (double duty) or only ever feeds $ARGUMENTS/$1-$9, leaving {{arguments}} an "unused-key" or "unmatched-placeholder" lint case despite the value being supplied.
minor | draft/deprecated status transition doesn't say whether sync uninstalls a previously-enabled task | §3 pins status: draft → sync never installs and status: deprecated → installs with a warning, but doesn't address the state transition case: a task already installed (OS cron/launchd/schtasks entry present) whose status: later flips to draft — does the next sync remove/uninstall the existing entry, or just skip re-installing and leave a stale scheduled entry running the old (or edited) body indefinitely?