Skip to content

Commit 4caadd5

Browse files
Bnjoroge1bnjoroge
andauthored
feat(runner-watch): structured gate, value normalizers, coverage map, tree-sitter deltas (#208)
* feat(runner-watch): structured gate, value normalizers, coverage map, tree-sitter deltas Addresses four conformance-tooling gaps. #1 Structured verdict instead of scraping the markdown report. - compare::analyze() -> ConformanceReport with failures(&GatePolicy) + to_json(); conform() gates on the struct and writes conformance-result.json. render_report is now a human-only artifact, no longer parsed for pass/fail. Removed the four string-scraping gate fns and their fixture tests; gate semantics preserved (status-ignore, request-schema-any-diff, acquirejob response-field-drop via a structural schema_drops_fields). #4 Declarative value normalizers + optional value gating. - normalize_value() collapses volatile scalars (GUID/JWT/token/URL/timestamp/ big-int/high-entropy) while keeping stable values visible. New ValueGate policy; default Off preserves legacy pass/fail and only reports value diffs. --value-gate-strict opts into normalized response-value gating for all shared endpoints except known-volatile ones. Strict is opt-in because validating its false-positive rate needs a live golden replay. #2 Endpoint coverage map (new coverage.rs + `runner-watch coverage`). - Cross-references golden-exercised endpoints against routes scraped from the server router, restricted to the runner-facing protocol surface. Warn by default; --strict fails on uncovered runner-facing routes minus .runner-watch/coverage-allow.txt. Against v2.336.0: 18 covered, 103 uncovered alias routes, 3 genuine gaps (runner calls /_apis/distributedtask/pools etc. that the server serves only under /runner/server or /_apis/v1). #3 tree-sitter-c-sharp delta extraction (was line heuristics). - Detects fields, record parameters, enum members, methods, expression-bodied properties; heuristic fallback retained. DeltaEntry/change_type unchanged. Verification: cargo test -p runner-watch (47 passing incl. gate-the-gate and coverage-over-real-goldens), fmt, workspace clippy -D warnings. Entire-Checkpoint: 01M1CS16K18E9CMAQWHQREYS0X * parser: serialize job env as TemplateToken maps for the official runner The official runner deserializes AgentJobRequestMessage .EnvironmentVariables into IList<TemplateToken> with a schema-driven evaluator. A plain JSON object per variable (the previous wire shape) converts to a token the workflow schema rejects, failing every job that declares job-level env with "The template is not valid. Unexpected value ''". Emit the {type: 2, map: [{Key, Value}]} shape GitHub sends (values are already resolved, so literal type-0 scalars are correct). Found via pl-official conformance captures: every job-level env workflow failed on the official runner against preloop while passing on GitHub. Entire-Checkpoint: 01M1CVAAMRC4D8H66JVAKBPBNV * fix(runner-watch): address PR review on coverage, value gate, and C# deltas - coverage: unify path normalization across both sides. `canonicalize` now reuses `compare::strip_transport_prefixes` (shared with `normalize_path`) and drops a leading org/base segment before `/_apis`, so `/runner/server/…` and `/{org}/_apis/…` aliases match the goldens instead of showing as false gaps. Real corpus: covered 18->22, uncovered 103->67, false gaps 3->1. (Codex A1 / CodeRabbit prompt) - coverage: preserve HTTP method — routes and golden keys are now `METHOD path`, so GET+POST on one path aren't conflated. Route methods parsed from the handler chain (paren-aware), `*` when none found. (Codex A2) - coverage: catch-all (`*wild`) routes match by prefix instead of exact equality, so multi-segment golden paths under `/replay/results/*path` count as covered. (CodeRabbit major) - coverage: `coverage_cmd` prints the allowlist-filtered uncovered count, not the raw one. (CodeRabbit minor) - compare: `normalize_value` collapses large integer JSON numbers (>=1e9: runner ids, epochs) so `--value-gate-strict` doesn't false-positive on numeric ids; small numbers stay intact. (CodeRabbit + Codex) - main: C# methods are keyed by name+parameter types, so an added/removed overload produces a distinct delta instead of collapsing on the `(type, name)` map. (Codex) - docstrings added on touched items. Tests: 50 passing (adds numeric-normalize, method-overload, and method/alias/ catch-all coverage tests). fmt + clippy -D warnings clean. Entire-Checkpoint: 01M1D2B39HZ2PJVEF224QS9K0E * fix(runner-watch,parser): address second-round PR review runner-watch/compare.rs - body_comparison compares every occurrence in an endpoint group by index, not just the first, so a later call that regresses its body is caught (a deduped array would have hidden a per-occurrence field drop). (CodeRabbit major) runner-watch/main.rs - method_signature now encodes generic arity (`M<2>`) and each parameter's passing mode (`ref int`), so ref/out/in and generic-arity overloads produce distinct keys instead of overwriting each other. (Codex/CodeRabbit) - parse_csharp returns None when the tree has syntax errors, so extract_fields falls back to the line heuristic instead of using an incomplete model. (CodeRabbit) runner-watch/coverage.rs - covers() treats a root catch-all ("/") as matching every descendant instead of building a "//" prefix. (CodeRabbit) parser/job_builder.rs - job env values serialize through template_string_token, so runtime-only expressions (e.g. `${{ github.workspace }}/bin`) reach the runner as expression tokens rather than raw literals; plain values stay type-0 literals. (CodeRabbit major) - tests assert the TemplateToken wire shape (type 2 map, literal Key/Value) and add a github.workspace regression. Tests: runner-watch 54, gha-parser job_env 2/2; fmt + clippy -D warnings clean on both crates. (Unrelated uncommitted runs.rs WIP in the checkout left untouched.) Entire-Checkpoint: 01M1D6JSFYFCVW1SM7ZYNRHW9D * fix(runner-watch): address cubic review compare.rs - body_comparison: treat a `null` or absent body as "no body" so an omitted official body vs a replayed `null` is not a false RequestSchema mismatch (P1 blocker); align bodies by flow slot (Option per flow) instead of filtering, so occurrences at different indexes aren't silently re-aligned; compare only the shared min-count of occurrences so a cardinality difference (extra poll / skipped retry) isn't folded into the body gate. coverage.rs - add /api/v1/actions to runner-facing prefixes so the bearerless action-download route's coverage is tracked. - restrict the leading-segment strip before /_apis to the `{p}` (org param) case, so a literal base like /foo_bar/_apis/... is no longer collapsed onto the bare /_apis route. - render_markdown: "0/0 (n/a)" for an empty route set (not 100%), and trim a leading `v` so a v-prefixed version doesn't render "vv2.336.0". main.rs - method_signature builds each parameter from the `type` field + `modifier` nodes rather than slicing raw text before the name, so a default value that contains the parameter name (`string s = "s"`) no longer truncates the signature. - coverage_cmd reads the allowlist before writing the JSON artifact and adds an `uncovered_after_allowlist` field so the JSON matches the printed/strict verdict. - read_coverage_allowlist propagates non-NotFound read errors instead of silently treating an unreadable allowlist as empty. Deliberately not changed: the >=1e9 numeric normalization in normalize_value. Cubic flags it as possibly hiding a stable large number, but it was explicitly requested last round (Codex/CodeRabbit) to stop false ValueMismatch on numeric ids/epochs, only affects opt-in --value-gate-strict, and there is no clean value-only heuristic distinguishing an id from a large count. Left as-is. Tests: runner-watch 56, gha-parser 139; fmt + clippy -D warnings clean. Entire-Checkpoint: 01M1D8JTW5DT9F2BXJXZXXQVS4 --------- Co-authored-by: bnjoroge <bnjoroge@local>
1 parent e8c38b0 commit 4caadd5

6 files changed

Lines changed: 1935 additions & 223 deletions

File tree

‎Cargo.lock‎

Lines changed: 38 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/preloop-gha-parser/src/job_builder.rs‎

Lines changed: 131 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -458,12 +458,28 @@ pub fn build_agent_job_message_with_normalized_context(
458458
// reaching action processes — moby's `DESTDIR` driving `docker buildx
459459
// bake` HCL variables is the canonical case — silently see empty values:
460460
// bake resolves `${DESTDIR}` to `""` and drops the target's output.
461-
// Emit one plain object per variable; the runner's template-map reader
462-
// accepts that shape (and the `{map: [{Key, Value}]}` form upstream
463-
// sends).
461+
// Emit one TemplateToken-shaped mapping per variable. The official
462+
// runner deserializes `EnvironmentVariables` into `IList<TemplateToken>`
463+
// via its Newtonsoft TemplateToken converter, which only understands the
464+
// `{type: 2, map: [{Key: {type: 0, lit: K}, Value: ...}]}` wire shape; a
465+
// plain JSON object deserializes to a token the schema evaluator rejects
466+
// with "The template is not valid. Unexpected value ''". Most values here
467+
// are already resolved literals, but runtime-only ones (e.g.
468+
// `${{ github.workspace }}/bin`, preserved above) are still templates the
469+
// runner must evaluate at step time — so serialize the value through
470+
// `template_string_token`, which emits a literal (type 0) for plain text
471+
// and an expression/format token for anything containing `${{ … }}`.
464472
let environment_variables: Vec<serde_json::Value> = resolved_env
465473
.iter()
466-
.map(|(k, v)| serde_json::json!({ k: v }))
474+
.map(|(k, v)| {
475+
serde_json::json!({
476+
"type": 2,
477+
"map": [{
478+
"Key": { "type": 0, "lit": k },
479+
"Value": template_string_token(v)
480+
}]
481+
})
482+
})
467483
.collect();
468484

469485
// GitHub supplies baseline regexes in addition to value-derived secret masks.
@@ -1020,6 +1036,36 @@ jobs:
10201036
/// Job/workflow-level `env:` must reach the wire as
10211037
/// `environmentVariables` (the field the official runner materializes
10221038
/// into step environments), not only as `variables`.
1039+
/// Flatten the TemplateToken-shaped `environmentVariables` entries back
1040+
/// into key/value pairs (mirror of what the official runner reads).
1041+
fn template_token_env_pairs(value: &serde_json::Value) -> Vec<(String, String)> {
1042+
let mut out = Vec::new();
1043+
if let Some(map) = value.get("map").and_then(|m| m.as_array()) {
1044+
for pair in map {
1045+
let key = pair
1046+
.get("Key")
1047+
.and_then(|k| k.get("lit"))
1048+
.and_then(|l| l.as_str())
1049+
.map(str::to_owned);
1050+
let val = pair
1051+
.get("Value")
1052+
.and_then(|v| v.get("lit"))
1053+
.and_then(|l| l.as_str())
1054+
.map(str::to_owned);
1055+
if let (Some(k), Some(v)) = (key, val) {
1056+
out.push((k, v));
1057+
}
1058+
}
1059+
} else if let Some(obj) = value.as_object() {
1060+
for (k, v) in obj {
1061+
if let Some(v) = v.as_str() {
1062+
out.push((k.clone(), v.to_owned()));
1063+
}
1064+
}
1065+
}
1066+
out
1067+
}
1068+
10231069
#[test]
10241070
fn job_env_is_exposed_on_environment_variables() {
10251071
let workflow = parse_workflow(
@@ -1050,15 +1096,32 @@ jobs:
10501096
let env: std::collections::BTreeMap<String, String> = message
10511097
.environment_variables
10521098
.iter()
1053-
.flat_map(|value| {
1054-
value.as_object().into_iter().flat_map(|obj| {
1055-
obj.iter()
1056-
.filter_map(|(k, v)| v.as_str().map(|s| (k.clone(), s.to_owned())))
1057-
})
1058-
})
1099+
.flat_map(template_token_env_pairs)
10591100
.collect();
10601101
assert_eq!(env.get("DESTDIR").map(String::as_str), Some("./build"));
10611102
assert_eq!(env.get("GLOBAL_VAR").map(String::as_str), Some("gv"));
1103+
// The wire shape must be the TemplateToken map the official runner's
1104+
// converter accepts (type 2 map of literal Key/Value tokens), not a
1105+
// plain object — otherwise the runner rejects it as "not valid".
1106+
for entry in &message.environment_variables {
1107+
assert_eq!(
1108+
entry["type"],
1109+
serde_json::json!(2),
1110+
"env entry must be a TemplateToken map: {entry}"
1111+
);
1112+
let map = entry["map"].as_array().expect("map array");
1113+
assert_eq!(map.len(), 1, "one Key/Value pair per entry: {entry}");
1114+
let pair = &map[0];
1115+
assert_eq!(pair["Key"]["type"], serde_json::json!(0));
1116+
assert!(
1117+
pair["Key"]["lit"].is_string(),
1118+
"Key must be a literal token"
1119+
);
1120+
assert!(
1121+
pair["Value"].get("type").is_some(),
1122+
"Value must be a token: {entry}"
1123+
);
1124+
}
10621125
assert_eq!(
10631126
message
10641127
.variables
@@ -1107,12 +1170,7 @@ jobs:
11071170
let env: std::collections::BTreeMap<String, String> = message
11081171
.environment_variables
11091172
.iter()
1110-
.flat_map(|value| {
1111-
value.as_object().into_iter().flat_map(|obj| {
1112-
obj.iter()
1113-
.filter_map(|(k, v)| v.as_str().map(|s| (k.clone(), s.to_owned())))
1114-
})
1115-
})
1173+
.flat_map(template_token_env_pairs)
11161174
.collect();
11171175
assert_eq!(
11181176
env.get("ENABLED").map(String::as_str),
@@ -1122,6 +1180,63 @@ jobs:
11221180
assert_eq!(env.get("PLAIN").map(String::as_str), Some("hello"));
11231181
}
11241182

1183+
/// Runtime-only expressions (`github.workspace` is filled in by the runner)
1184+
/// must reach the runner as an expression token to evaluate, not as a raw
1185+
/// literal of the template text.
1186+
#[test]
1187+
fn job_env_runtime_expression_values_reach_the_runner_as_tokens() {
1188+
let workflow = parse_workflow(
1189+
r#"
1190+
on: push
1191+
jobs:
1192+
build:
1193+
runs-on: ubuntu-latest
1194+
env:
1195+
BIN_DIR: ${{ github.workspace }}/bin
1196+
steps:
1197+
- run: echo hi
1198+
"#,
1199+
)
1200+
.unwrap();
1201+
let plan = &crate::expand_jobs(&workflow).unwrap()[0];
1202+
let message = build_agent_job_message(
1203+
plan,
1204+
&serde_json::json!({"event_name": "push"}),
1205+
&BTreeMap::new(),
1206+
&BTreeMap::new(),
1207+
&BTreeMap::new(),
1208+
)
1209+
.unwrap();
1210+
let value = message
1211+
.environment_variables
1212+
.iter()
1213+
.find_map(|entry| {
1214+
let pair = entry.get("map")?.as_array()?.first()?;
1215+
if pair.get("Key")?.get("lit")?.as_str()? == "BIN_DIR" {
1216+
Some(pair.get("Value")?.clone())
1217+
} else {
1218+
None
1219+
}
1220+
})
1221+
.expect("BIN_DIR must be present on the wire");
1222+
assert_ne!(
1223+
value["type"],
1224+
serde_json::json!(0),
1225+
"a runtime expression must not be shipped as a raw literal: {value}"
1226+
);
1227+
assert!(
1228+
value.get("lit").is_none(),
1229+
"expression value must not carry a raw lit: {value}"
1230+
);
1231+
assert!(
1232+
value["expr"]
1233+
.as_str()
1234+
.unwrap_or("")
1235+
.contains("github.workspace"),
1236+
"the value token must reference github.workspace: {value}"
1237+
);
1238+
}
1239+
11251240
#[test]
11261241
fn wrapped_conditions_are_unwrapped_for_the_official_runner() {
11271242
let workflow = parse_workflow(

‎crates/runner-watch/Cargo.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,6 @@ walkdir.workspace = true
2222
toml.workspace = true
2323
regex.workspace = true
2424
similar.workspace = true
25+
tree-sitter = "0.25"
26+
tree-sitter-c-sharp = "0.23"
2527
preloop-gha-protocol = { path = "../preloop-gha-protocol" }

0 commit comments

Comments
 (0)