102 lines
4.5 KiB
Markdown
102 lines
4.5 KiB
Markdown
# T3.3 — Load-time validation
|
|
|
|
| Field | Value |
|
|
|---|---|
|
|
| Phase | P3 — Workflow as data |
|
|
| Size | M — 1 to 3 days |
|
|
| Status | Not started |
|
|
| Flags | — |
|
|
| Spec | inlined below |
|
|
| Blocks | — |
|
|
|
|
## Goal
|
|
|
|
Every structural check the kernel makes on a workflow IR, each producing a
|
|
specific diagnostic. Runs at load, never at execution.
|
|
|
|
## Facts (inlined — no spec read needed)
|
|
|
|
Checks, all on the IR so every format inherits them:
|
|
|
|
- **DAG check.** The domain machine is validated as a DAG with explicit loop
|
|
bounds. An unbounded loop is a rejection, not a runtime concern.
|
|
- **`StepId` uniqueness** within a version.
|
|
- **Sub-workflow cycle detection at load, not at execution**, with recursion
|
|
depth bounded by the kernel.
|
|
- **Capability requirements resolvable** — every `ToolSelector` and
|
|
`VerifierRef` resolves to something registered.
|
|
- Sandbox eligibility belongs here too: a workflow whose steps cannot run
|
|
sandboxed is ineligible for shadow evaluation and **must say so at load time
|
|
rather than at 3am** (enforced fully in T6.5).
|
|
- Every rejection names the specific problem. A generic parse error moves the
|
|
diagnosis cost onto the user, who has less context than the validator.
|
|
- Related lesson: *suspect the guards before the mechanism* — a prior "batching"
|
|
failure turned out to be a depth guard, because two unrelated limits shared a
|
|
default value. Give each limit its own named constant.
|
|
|
|
## Steps
|
|
|
|
1. Write the validator as a pure function `validate(&WorkflowDef) -> Result<(), Vec<ValidationError>>`.
|
|
Collect all errors, do not stop at the first.
|
|
2. `ValidationError` is an enum with one variant per check, each carrying the
|
|
offending ids. No stringly-typed messages.
|
|
3. DAG check over `transitions`, with loop bounds required and verified finite.
|
|
4. `StepId` uniqueness by set insertion; report the duplicate id.
|
|
5. Sub-workflow graph walk with a kernel depth bound; report the cycle path, not
|
|
just "cycle detected".
|
|
6. Resolve every tool and verifier reference against the registries; report the
|
|
unresolved name and what registry was searched.
|
|
7. Give each numeric limit its own named constant with its own value, even where
|
|
two currently coincide.
|
|
8. Build the table-driven suite: one malformed workflow per check, asserting the
|
|
**specific** error variant.
|
|
|
|
## Acceptance
|
|
|
|
- A table-driven suite of malformed workflows, each rejected with a specific
|
|
diagnostic — not a generic parse error.
|
|
|
|
## Verify
|
|
|
|
**Harness:** a table of malformed workflow files, one per check, each paired with
|
|
the **exact `ValidationError` variant** it must produce.
|
|
|
|
**Integration test** — `tests/it_validation_table.rs`:
|
|
1. For each fixture, assert the returned error list **contains the expected
|
|
variant** and that the variant carries the offending ids.
|
|
2. Multi-error fixture: a workflow with three independent defects; assert **all
|
|
three** are reported in one pass, not just the first.
|
|
3. Cycle case: assert the error carries the **cycle path**, not just a boolean.
|
|
4. Depth case: a sub-workflow chain exceeding the kernel bound; assert rejection
|
|
at load and that no execution was attempted.
|
|
5. Unresolved capability: assert the error names both the missing name and the
|
|
registry searched.
|
|
6. Positive control: a valid workflow produces **zero** errors — otherwise a
|
|
validator that rejects everything passes the whole table.
|
|
7. Constant audit: assert each limit reads a distinct named constant; a test that
|
|
changes one constant and asserts only the matching check moves.
|
|
|
|
**Command:** `cargo test -p workflow validation`
|
|
|
|
**False pass:**
|
|
- Asserting `is_err()` per fixture. A validator returning one generic parse error
|
|
for everything passes the entire table — which is exactly the outcome the
|
|
acceptance criterion forbids. Match on the **variant**.
|
|
- Step 6 omitted, so a reject-everything implementation is green.
|
|
- Step 2 omitted, so fail-fast looks correct until a user with two mistakes has to
|
|
make two round trips.
|
|
- Two limits sharing a constant: step 7 is what surfaces the documented "batching
|
|
failure was actually the depth guard" class of bug.
|
|
|
|
## Traps
|
|
|
|
- Fail-fast on the first error. A user fixes one thing per load cycle.
|
|
- Deferring cycle detection to execution, where it becomes a stack overflow.
|
|
- Two limits sharing one constant. When one trips, you will debug the other.
|
|
|
|
---
|
|
|
|
Background (not required to do this task):
|
|
[rust-agentic-sys.md](../../../rust-agentic-sys.md) §4.4, §5.1, §12.4, §19 ·
|
|
[rust-agentic-task.md](../../../rust-agentic-task.md)
|