docs: remove unreachable action == "modify" branch from orchestrator contract

[Michael J. Jabbour (michaeljabbour)]

Summary

• docs/contracts/ORCHESTRATOR_CONTRACT.md told orchestrator authors to consume a hook modification with if pre_result.action == "modify". That branch can never execute — emit() normalizes the aggregate result to continue on every path.
• Replaces the dead branch with the consumption that actually works (read data unconditionally, guarded on it being a dict) and adds a Consuming modifications section explaining why.
• Documents an interaction currently written down nowhere: an inject_context or ask_user result from any handler on the same event is returned instead of the accumulated payload, so an earlier handler's modification is silently discarded.

Filed as the fix rather than an issue because issues are disabled on this repo.

The verification

Checked directly against crates/amplifier-core/src/hooks.rs at 92b339a:

Path	Line	Returns
no handlers registered	hooks.rs:170-174	HookAction::Continue, data: Some(value_to_map(&data))
no matching entries	hooks.rs:180-184	HookAction::Continue, data: Some(value_to_map(&data))
normal path	hooks.rs:269-274	HookAction::Continue, data: Some(value_to_map(&current_data)) — under the comment "Return final result with potentially modified data"

modify is a handler-to-handler chaining semantic inside the dispatch loop: each handler returning modify updates the payload passed to the next one. The aggregate result handed back to a caller is always continue, with the possibly-modified payload in data.

The inject_context / ask_user interaction is at hooks.rs:249-267, where special_result short-circuits the return — so an earlier handler's modification never reaches the caller, and on the approval path the returned result carries no data at all.

Why it matters

An orchestrator following the documented pattern contains a branch that never fires, and every handler that rewrites event data becomes a silent no-op: the handler runs, returns its correction, and the original data is used anyway — with no error and no log.

This is not hypothetical:

• amplifier-module-loop-streaming had exactly this bug at both tool:pre dispatch sites. Fixed in fix: always emit execution:end, honor tool:pre rewrites, and break identical failure loops amplifier-module-loop-streaming#41 (commit d4ce28b) by reading data unconditionally.
• The same repo's tool:post handling shows the author hit the wall too: it detects modification by object identity, with a comment explaining that action could not be checked. That workaround is itself unreliable — the kernel round-trips the payload through serde_json::Value, so the returned dict is always a fresh object.
• A grep across every module and bundle in the local bundle cache found zero consumers of action="modify" anywhere — which is what you would expect of a documented capability nobody can successfully use.

Scope and risk

Docs-only. No code, no behavior change — the only file touched is docs/contracts/ORCHESTRATOR_CONTRACT.md (+30 / −4). The risk is therefore confined to the documentation being wrong in the other direction, and the kernel citations above are there to check it against.

Because nothing shipped in the wheel changed, no version bump or E2E smoke test was run — docs/CORE_DEVELOPMENT_PRINCIPLES.md §10 scopes the release gate to "code shipped in the wheel". Flagging that reading explicitly so a maintainer can confirm it rather than have it silently omitted.

Test plan

• Kernel behavior verified by reading crates/amplifier-core/src/hooks.rs at all three HookAction::Continue return sites and the special_result short-circuit, cited above.
• N/A — no automated tests run: no code changed, and no test in this repo covers documentation text.

Suggested follow-up (not part of this change)

• A conformance test asserting that an orchestrator honors a hook modification. Nothing in amplifier-core asserts this today, which is why two dispatch sites in a shipped orchestrator could ignore it indefinitely.

Generated with Amplifier

[Michael J. Jabbour (michaeljabbour)]

Withdrawing this. It was pushed as a branch directly into this repo; it should
have gone to a fork, and it is now preserved at michaeljabbour/amplifier-core
on branch docs/modify-action-is-normalized-away (98121e3a9c26) if any of it
is useful. Closing rather than leaving it open so it does not sit in the queue
ahead of work that was here first.

Docs-only and conflicts with nothing, but withdrawing for consistency with the
other three. The finding stands on its own if anyone wants it:
ORCHESTRATOR_CONTRACT.md documents consuming a hook result via
action == "modify", but HookRegistry::emit normalises that away and always
returns Continue with the payload in data, so that branch is unreachable
and a modifying hook silently no-ops.