Declare typed external action requests by flyingrobots · Pull Request #175 · flyingrobots/edict

flyingrobots · 2026-07-29T23:13:40Z

Closes #172.

Plain-English Walkthrough

TL;DR

Edict can now compile a digest-locked, domain-specific external-action request into canonical Core and Target IR data without performing the action or granting a provider import. Echo admission and settlement execution remain downstream work.

Walkthrough

flowchart LR
  S[Edict request source] --> C[Typed Core request]
  C --> T[Capability-closed Target IR]
  T --> R[Echo admission and settlement]
  C -. no callable import .-> P[Provider seam]

[VERIFIED, HIGH] Source authority is explicit. use capability supplies an exact digest-locked operation resource. Request construction type-checks settlement, schemas, input, authority scope, basis, and budgets; admission uses a case-insensitive domain allowlist rather than ambient authority. (crates/edict-syntax/src/compiler.rs#L863-L958@6bb6552929f75768fa5815c53a8c56524ec469a0, crates/edict-syntax/src/compiler.rs#L2185-L2191@6bb6552929f75768fa5815c53a8c56524ec469a0)
[VERIFIED, HIGH] Lowering preserves request data without creating an intrinsic. External requests enter a dedicated Target IR collection with deterministic identity; ordinary target steps remain separate. (crates/edict-syntax/src/target_ir.rs#L610-L641@6bb6552929f75768fa5815c53a8c56524ec469a0)
[VERIFIED, HIGH] Capability closure and request identity fail closed. Canonical Target IR requires exact capability closure, nonempty resources, and unique per-intent request ids before bytes exist. (crates/edict-syntax/src/canonical.rs#L496-L531@6bb6552929f75768fa5815c53a8c56524ec469a0, crates/edict-syntax/src/canonical.rs#L616-L653@6bb6552929f75768fa5815c53a8c56524ec469a0)
[VERIFIED, HIGH] Public review projection exposes the complete boundary. CLI review JSON includes capability closure and every Core/Target request field as structured data, not execution authority. (crates/edict-cli/src/main.rs#L1257-L1300@6bb6552929f75768fa5815c53a8c56524ec469a0, crates/edict-cli/src/main.rs#L1345-L1364@6bb6552929f75768fa5815c53a8c56524ec469a0)
[VERIFIED, HIGH] The negative envelope is executable. Tests prove non-callability, exact closure, empty-resource and duplicate-id refusal, ambient-family rejection, Core and Target mutation sensitivity, a fixed-seed corpus, and a bounded 64-request stress module. (crates/edict-syntax/tests/external_action_requests.rs#L187-L277@6bb6552929f75768fa5815c53a8c56524ec469a0, crates/edict-syntax/tests/external_action_requests.rs#L279-L447@6bb6552929f75768fa5815c53a8c56524ec469a0, crates/edict-syntax/tests/external_action_requests.rs#L470-L523@6bb6552929f75768fa5815c53a8c56524ec469a0)
[SCOPED, HIGH] Runtime settlement is not claimed. This Edict slice owns deterministic request construction and explicit awaiting posture. Echo and Hello Effect own admission, adapter execution, settlement, recovery, and replay. (docs/topics/external-action-requests/README.md#L86-L101@6bb6552929f75768fa5815c53a8c56524ec469a0)

Validation and citations

cargo xtask verify
- cargo fmt --all --check
- workspace Clippy with -D warnings
- all workspace tests and four doctests
- 13 CLI golden cases
- five provider-component fixtures
- provider contract-pack check
- provider runtime dependency boundary
- 25 topic shelves
- git diff --check origin/main...HEAD
Core, Target IR, and provider contract artifacts were regenerated only through their owning xtask commands and then passed check mode.
The public contract and non-goals are recorded in docs/topics/external-action-requests/README.md#L1-L104@6bb6552929f75768fa5815c53a8c56524ec469a0.

flyingrobots · 2026-07-29T23:13:46Z

@coderabbitai review

flyingrobots · 2026-07-29T23:13:47Z

@codex please review

coderabbitai · 2026-07-29T23:13:55Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-07-29T23:13:57Z

Summary by CodeRabbit

New Features
- Added typed external-action requests that preserve operation, schema, authority, budget, and reconciliation details through Core and Target IR.
- Added digest-locked capability imports and request syntax with validation for supported operation families.
- CLI review output now displays external requests as structured, non-callable data awaiting settlement.
- Added capability closure tracking and canonical encoding support.
Documentation
- Added comprehensive language, architecture, provider, Target IR, and external-request documentation and test plans.
Bug Fixes
- Added fail-closed validation for missing capabilities, invalid digests, unsupported ambient operations, and altered request identity data.

Walkthrough

Adds typed external-action request syntax and capability imports, lowers requests into non-callable Core and Target IR data, enforces digest-locked capability closure during canonical encoding, exposes review JSON, updates schemas and documentation, and adds determinism, authority, closure, and bounded-stress tests.

Changes

Typed external-action request pipeline

Layer / File(s)	Summary
Source syntax and IR lowering `crates/edict-syntax/src/{ast,parser,semantic,compiler,core_ir,target_ir,result_projection}.rs`	Capability imports and `request` statements now produce typed external-action request nodes, with recursive typing, digest validation, ambient-family rejection, capability closure tracking, and non-callable Target IR lowering.
Canonical encoding and contracts `crates/edict-syntax/src/{canonical,contract_bundle}.rs`, `docs/abi/`, `fixtures/provider-contracts/v1/`	Core and Target IR canonical encoders serialize request data and reject missing capability bindings; schemas and contract corroboration cover capabilities, request arrays, budgets, and settlement posture.
Projection and validation `crates/edict-cli/`, `crates/edict-syntax/tests/`, `crates/edict-provider-schema/tests/*`	CLI review output exposes request data and capabilities, while tests cover closure enforcement, rejected authority families, reproducibility, identity sensitivity, projection, and bounded request collections.
Specification and documentation `ARCHITECTURE.md`, `README.md`, `ROADMAP.md`, `docs/*`, `CHANGELOG.md`	Language, architecture, roadmap, requirements, topic documentation, provider boundaries, and deferred admission/resumption responsibilities describe the new request behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

flyingrobots/echo issue 694 — Defines the durable request/settlement protocol that consumes Edict’s typed request representation.

Possibly related PRs

flyingrobots/edict#9 — Introduced the parser scaffolding extended here with capability imports and request statements.
flyingrobots/edict#67 — Established the Target IR lowering surface extended here with request collections and capability closure.
flyingrobots/edict#108 — Added Target IR canonical encoding framing used by the new external-request encoding.

Poem

A request takes shape, digest-bound and bright,
Through Core it travels, non-callable in flight.
Target steps stay empty; closures guard the door,
Canonical bytes remember every field in store.
Settlement waits, while Echo holds the key—
Typed little promises, deterministic as can be.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.59% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy `#172`: syntax, Core/Target IR, deterministic lowering, closure checks, and rejection tests are all present.
Out of Scope Changes check	✅ Passed	I see no clearly unrelated changes; the docs, tests, and fixture updates all support the same feature.
Title check	✅ Passed	The title clearly summarizes the main change: adding typed external action requests.
Description check	✅ Passed	The description is directly about the changeset and accurately describes the new request flow and downstream scope.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76e9f37527

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

coderabbitai

Actionable comments posted: 15

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/edict-syntax/src/canonical.rs (1)
588-620: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract a shared helper for lawpack/capability resource-set handling. Both sites hand-roll the same "lawpacks vs capabilities" logic side-by-side — one root cause: no reusable abstraction for digest-locked resource-set dedupe/compare, so the pattern get copy-pasted every time a second resource kind (capabilities) is bolted onto the first (lawpacks).

crates/edict-syntax/src/canonical.rs#L588-L620: extract the dedupe+conflict-detect loop (BTreeMap<&str, &ResourceRef> build + conflict error) into a helper taking (&[ResourceRef], field_name) and call it once for lawpacks and once for capabilities.

crates/edict-syntax/src/contract_bundle.rs#L606-L640: extract the "compare canonical_resource_set(actual.X) vs expected.X, error with field name" pattern into a helper parameterized by field name, and call it once for lawpacks and once for capabilities in both match arms.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/edict-syntax/src/canonical.rs` around lines 588 - 620, Extract
reusable resource-set helpers: in crates/edict-syntax/src/canonical.rs lines
588-620, move the BTreeMap deduplication and conflicting-resource validation
into a helper accepting a ResourceRef slice and field name, then use it for both
lawpacks and capabilities; in crates/edict-syntax/src/contract_bundle.rs lines
606-640, extract the canonical_resource_set comparison and field-specific error
handling into a field-name-parameterized helper and invoke it for lawpacks and
capabilities in both match arms.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ARCHITECTURE.md`:
- Line 176: Update the architecture flow around “typed external-action request
data” so it is shown as part of Core IR rather than as a separate post-Core-IR
artifact. Align the diagram with the existing module map by documenting the
sequence as Core IR including typed external-action request data, followed by
canonical Core bytes.

In `@crates/edict-syntax/src/compiler.rs`:
- Around line 2140-2147: The ambient operation gate in ambient_operation_family
must enforce the registry’s guarantee: normalize the root segment
case-insensitively, replace the denylist with an explicit allowlist of
requestable operation families, and add a dedicated CompilerErrorKind for
refusals so tests can assert the stable kind. In docs/REQUIREMENTS.md at line
70, retain status: impl only after the allowlist, error kind, and executable
negative evidence exist; otherwise downgrade the status and record the residual
ambient-coverage gap in the owning topic shelf’s test-plan.md.
- Around line 1661-1674: Update the Stmt::ExternalActionRequest arm to
accumulate results with a mutable accepted flag and &= for every
check_known_effect_profiles call, matching the sibling Stmt::If and Stmt::For
arms. Ensure operation, authority_scope, basis, max_settlement_bytes, and
max_attempts are all checked so diagnostics are not short-circuited.
- Line 286: Remove the unused _errors: &mut Vec<CompilerError> parameter from
resolve_imports, then update its sole call site to pass only the imports
argument. Do not add replacement validation or error handling.
- Around line 994-1013: Update the capability lookup in
check_external_action_operation() to require the import’s digest-lock predicate
in addition to CoreImportKind::Capability and the matching alias. Reuse the
existing digest predicate used by canonical Core lowering so capability imports
without a digest fall through to the MissingContextFact error instead of
producing an operation_resource.

In `@crates/edict-syntax/src/parser.rs`:
- Around line 964-969: In the external-action request validation around
is_call_expr, replace ParseErrorKind::NonCallEffect with a new dedicated
ParseErrorKind for non-call request operations. Define the new kind alongside
the existing parse error kinds and assign it its own stable diagnostic code,
preserving the current error message and validation behavior.

In `@crates/edict-syntax/src/target_ir.rs`:
- Around line 635-666: Update the ExternalActionRequest branch in lower_node to
record the produced request binding in state.step_outputs as well as
state.external_action_requests, so require arms can resolve it. Remove the
redundant step_outputs insertion from the Effect arm, since lower_effect_node
does not reliably create these steps. Preserve the existing request construction
and binding validation behavior.
- Around line 403-438: Extract the duplicated digest-lock and
coordinate-conflict validation from the lawpack and capability-import loops into
a shared helper near the target-lowering logic. Have both loops invoke the
helper while preserving their respective maps and failure-kind/label behavior,
including existing error details and insertion semantics.

In `@crates/edict-syntax/tests/external_action_requests.rs`:
- Around line 85-95: Add an alternate value for maxAttempts to the ObserveInput
fixture type and use it in the mutation cases exercised by
every_request_authority_field_moves_core_identity. Ensure the maxAttempts
mutation changes only that field while preserving the test’s existing coverage
pattern for the other authority fields.

In `@docs/REQUIREMENTS.md`:
- Line 70: Update the EDICT-LANG-EXTERNAL-REQUEST-001 row in
docs/REQUIREMENTS.md so its positive and negative fixture cells identify
distinct evidence, naming the specific negative test functions or a separate
negative artifact for missing-closure and ambient-family rejection. Because the
requirement has hash-significant Core and Target encoding behavior, add the
appropriate generated CBOR golden artifact and checksum reference consistent
with neighboring hash-impacting rows.

In `@docs/topics/cli/test-plan.md`:
- Line 102: Correct the CLI-TP-029 test-plan entry to match the assertions in
project_exposes_external_requests_as_non_callable_review_data: describe only the
verified Core/Target IR fields, including node kind, operation and closure
coordinates, awaitingSettlement state, and zero steps. Do not claim a complete
request payload unless the cited test is expanded to assert the additional
projected fields.

In `@docs/topics/external-action-requests/README.md`:
- Around line 71-74: Update the authority allowlist terminology in the external
action requests documentation to use the canonical `Git` and `GitHub`
operation-family names instead of lowercase `git` and `github`, while preserving
the surrounding rejection rules unchanged.

In `@docs/topics/external-action-requests/test-plan.md`:
- Line 57: Update the EXTREQ-TP-006 mutation-sensitivity test plan entry and its
referenced every_request_authority_field_moves_core_identity coverage to assert
Target IR canonical bytes or digests for every listed mutation, in addition to
Core identity, so target encoding changes are detected.

In `@docs/topics/providers/README.md`:
- Around line 131-134: Clarify the provider-boundary paragraph around “use
capability” declarations and Core external-action request nodes: describe
capability declarations as digest-locked module-import closure resources, and
reserve “canonical request data” and request fields for the Core request nodes.
Preserve the statement that neither becomes component imports, host functions,
or provider-owned execution authority.

In `@docs/topics/target-ir/test-plan.md`:
- Line 109: Update the TP-035 entry in the test plan to remove TIR-REQ-006 from
its requirements mapping while retaining TIR-REQ-001, TIR-REQ-008, and
TIR-REQ-017. Keep the existing behavioral evidence and the Notes boundary
statement unchanged, since the policy non-goal is not established by these test
functions.

---

Outside diff comments:
In `@crates/edict-syntax/src/canonical.rs`:
- Around line 588-620: Extract reusable resource-set helpers: in
crates/edict-syntax/src/canonical.rs lines 588-620, move the BTreeMap
deduplication and conflicting-resource validation into a helper accepting a
ResourceRef slice and field name, then use it for both lawpacks and
capabilities; in crates/edict-syntax/src/contract_bundle.rs lines 606-640,
extract the canonical_resource_set comparison and field-specific error handling
into a field-name-parameterized helper and invoke it for lawpacks and
capabilities in both match arms.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3d13a243-baf3-460a-88ee-59150b61d20c

📥 Commits

Reviewing files that changed from the base of the PR and between 21c4400 and 76e9f37.

📒 Files selected for processing (39)

ARCHITECTURE.md
CHANGELOG.md
README.md
ROADMAP.md
crates/edict-cli/src/main.rs
crates/edict-cli/tests/jsonl_cli.rs
crates/edict-provider-schema/tests/provider_contract_pack.rs
crates/edict-syntax/src/ast.rs
crates/edict-syntax/src/canonical.rs
crates/edict-syntax/src/compiler.rs
crates/edict-syntax/src/contract_bundle.rs
crates/edict-syntax/src/core_ir.rs
crates/edict-syntax/src/lib.rs
crates/edict-syntax/src/parser.rs
crates/edict-syntax/src/result_projection.rs
crates/edict-syntax/src/semantic.rs
crates/edict-syntax/src/target_ir.rs
crates/edict-syntax/tests/contract_bundle.rs
crates/edict-syntax/tests/external_action_requests.rs
crates/edict-syntax/tests/parse_review_regressions.rs
docs/REQUIREMENTS.md
docs/SPEC_edict-language-v1.md
docs/abi/edict-core.cddl
docs/abi/edict-target-ir.cddl
docs/topics/README.md
docs/topics/cli/test-plan.md
docs/topics/contract-bundles/test-plan.md
docs/topics/core-ir/README.md
docs/topics/core-ir/test-plan.md
docs/topics/external-action-requests/README.md
docs/topics/external-action-requests/test-plan.md
docs/topics/providers/README.md
docs/topics/providers/test-plan.md
docs/topics/syntax/README.md
docs/topics/syntax/test-plan.md
docs/topics/target-ir/README.md
docs/topics/target-ir/test-plan.md
fixtures/provider-contracts/v1/edict-provider-contracts.cddl
fixtures/provider-contracts/v1/manifest.json

📜 Review details

⏰ Context from checks skipped due to timeout. (3)

GitHub Check: rust msrv 1.94.0 (fmt · clippy · test)
GitHub Check: supply-chain (cargo-deny)
GitHub Check: rust stable (fmt · clippy · test)

🧰 Additional context used

📓 Path-based instructions (6)

**/*