feat(compile): replace Python gate evaluator with bundled TypeScript gate.js#389
feat(compile): replace Python gate evaluator with bundled TypeScript gate.js#389jamesadevine wants to merge 7 commits intomainfrom
Conversation
…gate.js Migrates the trigger-filter gate evaluator from `scripts/gate-eval.py` to a bundled TypeScript artifact `scripts/gate.js` produced by a new `scripts/ado-script/` workspace. The compiler now emits a `NodeTool@0` install step and `node /tmp/ado-aw-scripts/gate.js` instead of `python3`. Architecture (variant A2 in the design walkthrough): - TypeScript workspace at scripts/ado-script/ built with @vercel/ncc. - shared/ modules (auth, ado-client, env-facts, policy state machine, vso-logger) reusable across future bundles. - gate/ entry implementing bypass, fact acquisition, 11 predicate evaluators, and self-cancel — full parity with the deleted Python evaluator (45/45 tests ported, +6 parity guards). Drift-proof codegen: - New hidden CLI subcommand `ado-aw export-gate-schema` emits a schemars-derived JSON Schema from the Rust IR types. - `npm run codegen` chains it through json-schema-to-typescript to produce src/shared/types.gen.ts. - New CI workflow .github/workflows/ado-script.yml runs codegen + `git diff --exit-code` to fail on IR/TS schema drift. Pipeline integration: - TriggerFiltersExtension prepends a NodeTool@0 step pinned to 20.x. - compile_gate_step_external invokes `node` instead of `python3`. - release.yml builds the bundle (npm ci && npm run build) before zipping scripts/, and excludes node_modules/dist/schema from the zip. - New tests/gate_e2e.rs (#[ignore]'d) compiles a real agent, extracts GATE_SPEC, runs gate.js end-to-end, and asserts SHOULD_RUN. - compiler_tests.rs assertions tightened: now check for `node '/tmp/ado-aw-scripts/gate.js'` instead of the loose `python3` match (which falsely passed via base.yml's mcpg-config validation). Cleanup: - Deleted scripts/gate-eval.py, scripts/gate-spec.schema.json, tests/gate_eval_tests.py. Documentation: - New docs/ado-script.md records the A2 decision, codegen pipeline, bundle-size budget (5 MB; gate.js is ~1.1 MB), and how to add new internal bundles (e.g. poll.js). - docs/filter-ir.md rewritten: Node evaluator, NodeTool@0 step, scripts.zip distribution. - AGENTS.md tree + tech stack updated; new entry in docs index. - ado-script-design.md added at repo root as the design walkthrough that produced the A2 decision. Validation: - 173/173 vitest tests pass (45 ports + 6 parity guards + smoke + shared-module units). - Full cargo test suite green. - cargo clippy --all-targets --all-features clean. - E2E: `cd scripts/ado-script && npm run build && cd ../.. && cargo test --test gate_e2e -- --ignored` passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Rust PR ReviewSummary: Looks good overall — solid migration with good test coverage and a clean codegen pipeline. Two actionable issues worth addressing before merge, plus one stale doc string. Findings🐛 Bugs / Logic Issues
|
…ate shared tests - Add vitest.config.ts with `include: ["src/**/*.test.ts"]` so the default `npm test` no longer picks up test/smoke.test.ts (which requires the ncc bundle to exist and was failing in CI before the build step ran). - Add vitest.config.smoke.ts targeting test/ for the smoke run. - Wire `npm run test:smoke` and the CI Smoke-test step to use the new smoke config (`vitest run -c vitest.config.smoke.ts`). - Move shared module tests under src/shared/__tests__/ to mirror the layout used by src/gate/__tests__/. Update relative imports (./foo.js → ../foo.js) and the vi.mock path in ado-client.test.ts. Validation: - `npm test` → 171/171 ✅ (smoke excluded) - `npx vitest run -c vitest.config.smoke.ts` after `npm run build` → 2/2 ✅ - `npm run typecheck` ✅ Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Rust PR ReviewSummary: Looks good overall — well-structured migration with solid test coverage and drift prevention. Three issues worth addressing before merge. Findings🐛 Bugs / Logic Issues
|
…ed unknown predicates Closes review findings on #389: 1. FACT_DEPS drift gap: the fact dependency graph in policy.ts was a manually-maintained mirror of Fact::dependencies() in Rust, outside the schema drift check. Added `dependencies: Vec<String>` to FactSpec populated from Fact::dependencies(). Regenerated types.gen.ts. PolicyTracker now reads the dep graph from the spec at construction; the hardcoded FACT_DEPS constant is removed. The CI `git diff --exit-code` on types.gen.ts now covers the dep graph too. 2. Unknown predicate silent pass: the default arm of evaluatePredicate returned true (auto-pass), so a stale gate.js paired with a newer compiler that emits a new predicate would silently pass filters. Now: emits a task.logissue type=warning naming the unknown type and returns false (fail-closed). Added a regression test. 3. Stale "Python" doc on GateSpec: updated to "Node gate evaluator (scripts/gate.js)". Propagates through codegen. 4. scripts.zip ships TS source: added -x patterns for ado-script/src/, test/, package*.json, tsconfig.json, vitest.config*.ts, README.md, and .gitignore. Only gate.js (already at scripts/gate.js) and any future bundle artefacts are shipped. 5. globMatch bracket divergence: documented inline that bracket expressions [seq] are escaped to literal characters (Python fnmatch supported them). The IR currently never emits bracket patterns; if a future predicate needs them, the builder must be extended. 6. release.yml missing npm cache: added cache: "npm" and cache-dependency-path to match the CI workflow. Avoids re-downloading node_modules on every release run. 7. Fact::is_pipeline_var() #[cfg(test)] opacity: added a doc comment noting the runtime mirror is isPipelineVarFact in env-facts.ts, so future readers don't wonder why the helper exists only for tests. Validation: 172/172 vitest tests, full cargo test suite, clippy clean, e2e gate test passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Rust PR ReviewSummary: Looks good — clean migration with solid test coverage. A few stale docstrings and one ignored test writing to a now-deleted file are worth cleaning up. Findings
|
Update stale doc comments referencing Python evaluator to TypeScript. Fix fnmatch reference in filter-ir.md to describe custom glob semantics. Hoist toLowerCase() map outside includes() for value_in_set/value_not_in_set predicates to avoid per-call array allocation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Rust PR ReviewSummary: Solid migration overall — the architecture is clean and the 45-case parity inventory gives good confidence. Two issues worth addressing before merge. Findings🐛 Bugs / Logic Issues
|
Use escapeMessage instead of escapeProperty for setOutput value — the value is in the message body (after closing ]) where ] and ; are not escaped. Also add trigger_filters.rs to ado-script workflow paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
7269ccf to
5959433
Compare
🔍 Rust PR ReviewSummary: Solid migration — architecture is well-designed and the security-sensitive paths are handled carefully. Two items worth addressing before merge. Findings🐛 Bugs / Logic Issues
|
- Update test_write_schema_to_scripts to write to the canonical scripts/ado-script/schema/ path instead of the deleted scripts/gate-spec.schema.json location. - Convert NodeTool@0 step from escaped single-line string to a raw string literal, consistent with the adjacent download step. - Add logWarning guard in globMatch when pattern contains "[" to catch compiler/evaluator parity drift early (bracket expressions are treated as literals, not character classes). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Rust PR ReviewSummary: Looks good overall — clean migration with solid test coverage, good security posture, and a clever drift-prevention mechanism. A couple of minor items below. Findings
|
|
@copilot rebadge scripts.zip as ado-script. Just directly upload the build ado-script folder |
Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/eb18237a-9266-4098-bb63-db9816eb03fd Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Done in cee11de. Release now uploads Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
@copilot can we just upload the dist folder directly as ado-script? Say no if it is a bad idea. Zip is fine. |
No—keep the zip. GitHub release assets are files, not directories, so we can’t upload a raw folder as an asset. |
Summary
Replaces the Python
gate-eval.pytrigger-filter evaluator with abundled TypeScript artifact
scripts/gate.js, produced by a newscripts/ado-script/workspace. This is variant A2 from thedesign walkthrough
— a per-use-site Node bundle, types codegen'd from the Rust IR via
schemarsto prevent drift.The compiler now emits a
NodeTool@0install step plusnode /tmp/ado-aw-scripts/gate.jsinstead ofpython3. The bundleships in the existing
scripts.ziprelease asset.Why
gate-eval.pywas a 388-line conflation of fact acquisition, ADO RESTcalls, predicate evaluation, and self-cancel — hand-rolled
urllib,no SDK, no real test framework. Moving to TypeScript lets us reuse
azure-devops-node-apifor REST calls, run vitest with proper mocks,and unblock future internal bundles (e.g.
poll.js) that follow thesame per-use-site pattern.
What's in the diff (53 files)
Compiler
src/compile/extensions/trigger_filters.rs— emitsNodeTool@0step;
GATE_EVAL_PATHswitched togate.js.src/compile/filter_ir.rs—python3→nodeincompile_gate_step_external; IR types gainJsonSchemaderives;new
generate_gate_spec_schema().src/main.rs— hiddenexport-gate-schemasubcommand consumed bythe workspace's
npm run codegen.TypeScript workspace —
scripts/ado-script/(40 new files)src/shared/— auth,ado-client(with retries againstazure-devops-node-api), env-facts, policy state machine,vso-logger.
src/gate/— bypass, fact acquisition, 11 predicate evaluators,self-cancel.
src/shared/types.gen.ts— auto-generated fromcargo run -- export-gate-schema→json-schema-to-typescript.guards + smoke test + shared-module units).
scripts/gate.js≈ 1.1 MB (well under the5 MB per-bundle budget).
Pipeline + CI
.github/workflows/release.yml— addssetup-node+npm ci && npm run buildbefore zippingscripts/; excludesnode_modules,dist/,schema/fromscripts.zip..github/workflows/ado-script.yml— new CI job: install, codegen,drift-check (
git diff --exit-codeontypes.gen.ts), tests,typecheck, build, smoke test, E2E.
Tests
tests/gate_e2e.rs(#[ignore]'d) — compiles a real agent,extracts
GATE_SPECfrom the emitted YAML, runsnode gate.jsend-to-end, asserts
SHOULD_RUN.tests/export_gate_schema.rs— verifies the new subcommand emitsvalid JSON Schema.
tests/compiler_tests.rs— assertion tightening: wascompiled.contains("python3")(which silently passed even afterthe migration via
base.yml's mcpg-config validation step); nowasserts the exact
node '/tmp/ado-aw-scripts/gate.js'substring.Docs
docs/ado-script.md(new) — A2 decision record, codegen pipeline,bundle-size budget, instructions for adding new bundles.
docs/filter-ir.md— Python references replaced with the Nodeevaluator;
NodeTool@0step added; scripts.zip distributiondocumented.
AGENTS.md— directory tree + tech stack + docs index updated.ado-script-design.md(new at repo root) — original designwalkthrough that produced the A2 decision.
Deleted
scripts/gate-eval.py(388 lines)scripts/gate-spec.schema.json(regenerable; the workspace ownsits own
schema/copy via codegen)tests/gate_eval_tests.py(45 cases, all ported to vitest)Behavior parity
The TypeScript port is a 1:1 reimplementation. Confirmed parity for
each of the 45 deleted Python test cases (see
scripts/ado-script/src/gate/__tests__/ports/INVENTORY.md). Twointentional minor deviations are documented inline:
pr_is_draftreturnsundefinedwhenpr_metadatais missing(Python returned the literal string
"unknown"). Topologicalordering + the policy tracker prevent the divergence from being
observable.
PolicyTrackercollapses simultaneousfail_closed+skip_dependentsreferences to"skip"for cleaner verdicts. ThePython evaluator set
should_run = falseglobally onfail_closedbut produced functionally equivalent output for the realistic fact
dependency graph.
Drift prevention
The
ado-scriptCI workflow runsnpm run codegenand thengit diff --exit-code -- scripts/ado-script/src/shared/types.gen.ts.If a contributor changes
Fact/Predicate/GateSpecshapes in Rustwithout regenerating, CI fails with a clear remediation message.
Out of scope (explicit non-goals)
ado-script:front-matter block letting authors runarbitrary TypeScript at pipeline runtime (Variant B in the design
doc). Separate RFC if pursued.
Test plan
Local validation (all green):
CI validates the same flow plus the codegen drift check.
Manual reviewer checklist:
and confirm only the
python3→nodeswitch +NodeTool@0prepend changed.INVENTORY.mdfor the 1:1 mapping of deleted Pythontests to their vitest counterparts.
docs/ado-script.mdto confirm the decision rationalereads coherently.
Note on diff scope
A handful of unrelated Rust files were rustfmt-reformatted by
sub-agents during implementation. Those reflows have been reverted
out of this commit so the diff stays surgical to the migration. If
desired, a follow-up
chore: rustfmt the treePR can land themseparately.