Skip to content

feat(core): pluggable zero-token verifiers for verifyTaskflow#84

Open
hirisov wants to merge 1 commit into
heggria:mainfrom
hirisov:feat/verify-pluggable-verifiers
Open

feat(core): pluggable zero-token verifiers for verifyTaskflow#84
hirisov wants to merge 1 commit into
heggria:mainfrom
hirisov:feat/verify-pluggable-verifiers

Conversation

@hirisov

@hirisov hirisov commented Jul 18, 2026

Copy link
Copy Markdown

Checklist

  • I opened an issue first and linked it below.
  • I read CONTRIBUTING.md.
  • pnpm run typecheck passes.
  • pnpm test passes for this change (see Tests — 5 pre-existing failures on main are unrelated).
  • I added tests for new behavior.
  • The diff is focused — one concern per PR.
  • I signed off my commits (git commit -s).

Related issue

Closes #82

What changed

Implements the v1 scope accepted in #82: a pluggable, zero-token verifier seam on the existing verify/compile layer. A host registers domain-specific static checks that run alongside the built-in structural detectors — merged into the same VerificationResult, the same compile Mermaid/report overlay, and the same runtime preflight.

  • verifyTaskflow(flow, { verifiers }) — extensible options shape. Built-in detectors run first (unchanged), then each TaskflowVerifier against the same sanitized flow, in registration order.
  • New TaskflowVerifier ({ name, verify }) + VerifierIssue. Plugin issues are stamped category: "plugin" with source: <name>; the closed IssueCategory union gains "plugin" and VerificationIssue gains optional source. Verifiers can't impersonate built-in categories.
  • Fail-closed: a throwing or malformed verifier (e.g. non-array return) normalizes to one error/plugin issue; the remaining verifiers still run.
  • compileTaskflow forwards CompileOptions.verifiers, so plugin issues overlay on the diagram + report like built-ins.
  • RuntimeDeps.verifiers is threaded into every place verification already runs: both imperative dynamic-subflow sites (ctx_spawn, inline flow{def}) and the event-kernel inline-def site. A top-level preflight in executeTaskflow blocks only on plugin error-severity issues — built-in detectors stay advisory at the top level, as before. Opt-in and behavior-preserving: hosts that register no verifiers get byte-for-byte identical behavior.

Scope (per #82): programmatic Core API only — no project-local discovery, no built-in shell-command lint (both deferred). Pure functions; zero new runtime dependencies.

Tests

12 new tests in packages/taskflow-core/test/verify-pluggable.test.ts: ordering, warning/error semantics, fail-closed (throwing + malformed), safeFlow sanitization, back-compat, compile Mermaid/report overlay, no-spend runtime blocking on both the imperative and event-kernel engines, and a regression test that the top-level gate keys off category === "plugin" (not source).

Test status: typecheck passes. The full pnpm test suite has 5 pre-existing failures on mainCacheStore: clear, peek: is read-only, F8: saveFlow hint, findProjectFlowsDir: stops at home dir, first-run trace: sink built — all environment/flaky (filesystem + home-dir dependent; one flips pass/fail between runs) and present on the base branch. Verified by stashing the diff and re-running: this PR adds 0 new failures.

Sign-off

Not DCO-signed; happy to amend with git commit -s if required.

Screenshots / TUI output

N/A — no TUI/render change.

Add a verifier seam to the verify/compile layer so a host can register
domain-specific static checks that run alongside the built-in structural
detectors — same report, same Mermaid overlay, same runtime preflight.

- verifyTaskflow(flow, { verifiers }) — options-object 2nd arg. Built-in
  detectors run first (unchanged), then each TaskflowVerifier against the same
  sanitized flow, in registration order.
- New TaskflowVerifier interface + VerifierIssue; plugin issues are stamped
  category "plugin" with source = verifier name. IssueCategory (closed union)
  gains "plugin"; VerificationIssue gains optional source. Fail-closed: a
  throwing or malformed verifier is normalized to one error/plugin issue and
  the remaining verifiers still run.
- compileTaskflow forwards CompileOptions.verifiers, so plugin issues overlay
  on the Mermaid diagram + report like built-ins.
- RuntimeDeps.verifiers flows into both imperative dynamic-subflow verify sites
  (ctx_spawn, flow{def}) and the event-kernel inline-def site. A top-level
  preflight in executeTaskflow blocks only on plugin error-severity issues —
  built-in detectors stay advisory at the top level, so existing flows are
  unaffected. Opt-in: hosts that register no verifiers get identical behavior.

Pure functions, zero new runtime dependencies. Programmatic API only — no
project-local discovery and no built-in shell-command lint (both deferred per heggria#82).

Refs heggria#82
@heggria

heggria commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Thanks for the focused implementation. The API direction matches the accepted v1 scope in #82, and I verified the PR at 54f8397.

Local validation is healthy: typecheck passed, taskflow-core built successfully, the 12 new tests passed, the full suite passed 1954/1954 outside the filesystem sandbox, and git diff --check passed.

I do not think this is ready to merge yet because the following runtime and boundary cases are not closed:

  1. Event-kernel saved flow bypasses verifier preflight

    In packages/taskflow-core/src/exec/driver.ts:207-277, runNested validates/admit-checks the child and then recursively calls runEventKernel. packages/taskflow-core/src/exec/step-kinds.ts:689-703 verifies only the inline def branch, not saved flow use.

    Reproduction: a parent contains flow use: child, while the verifier returns an error only when flow.name is child. Imperative execution returns ok:false with zero runTask calls. Event-kernel execution returns ok:true and executes the child task. This breaks engine parity and the no-spend guarantee.

    Please centralize the plugin-error preflight before every actual child-flow dispatch, including before cache/resume reuse, and cover root, inline def, ctx_spawn, saved use, and cache/resume across both engines. The current two engine tests stop in the shared top-level preflight before engine dispatch, so they do not exercise engine-specific plumbing.

  2. A verifier can mutate the real execution plan

    verify.ts:472-496 creates a new phase array but keeps the original phase and budget object references. A verifier can change a task from SAFE to MUTATED or raise maxTokens, and runtime then executes the mutated definition. Please pass an isolated deep-readonly snapshot, ideally with defensive cloning/freezing, and test that the original definition and later verifiers remain unchanged.

  3. Malformed-verifier fail-closed handling is incomplete

    With verifiers: [null, goodVerifier], the try block fails on v.verify and the catch block reads v.name again, causing a TypeError; the sibling verifier never runs. A non-array verifier registry also throws before normalization. Please validate the collection and each entry before invocation, and ensure the catch path does not access the original malformed object.

  4. Unknown plugin phaseId can alter Mermaid syntax

    compile.ts:317-320 falls back from idMap.get(id) to the raw plugin-supplied id. A phaseId containing newlines or Mermaid syntax is emitted directly into a class statement and can inject additional Mermaid directives. Please apply overlays only to phase IDs present in idMap; unknown IDs can remain as escaped global findings in the report.

Non-blocking follow-ups: render VerificationIssue.source in human-readable compile/runtime diagnostics, and avoid running imperative inline-child verifiers twice.

The GitHub Actions run is currently action_required with zero jobs, so CI still needs approval and a real green run after the fixes. Happy to re-review an update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Pluggable zero-token verifiers for verifyTaskflow / compileTaskflow

2 participants