Skip to content

Commit e265443

Browse files
fix(wire-shape): close 4 review-surfaced gaps
- load_baseline raises SystemExit with regen hint when the JSON file is malformed instead of dumping an opaque traceback. - write_baseline cleans up its tmp sidecar on any exception so a crashed run can't poison the next refresh from the same PID. - The validator now exits 1 (not 0) when AXONFLOW_OPENAPI_SPECS_DIR is set but doesn't point at a directory. A misconfigured CI step silently disabling the gate produces a green check on a non-running validator, which we refuse to do. An unset env still skips with 0. - Workflow's SHA-bump guard distinguishes "baseline missing on base branch" (genuine first-pin introduction) from "baseline present but unparseable" (bypass attempt). A malformed baseline on the base branch can no longer route a labelless PR through the first-pin path. - Java source parser: * Recognises Java 15+ text blocks (`"""..."""`). Previously the first `"""` looked like quote-empty-quote and the body parsed as live source — fake `@JsonProperty(...)` and `class X {` inside a text block silently corrupted attribution. * Filters annotation matches whose position falls inside a blanked range (string, comment, text block) so the wire-name regex on raw content can't grab annotations the structural pass already nullified. * Attributes record-parameter annotations to the record. Pending decls are now tracked between their token and the body `{`, so `record Foo(@JsonProperty("x") int x) {}` no longer drops the wire name. The SDK has no records today; this is forward defence for the next type-class refactor. - Documents the remaining anonymous-inner-class limitation (no `class` keyword, so annotations would leak to the enclosing type). The SDK does not put @JsonProperty inside anonymous inners. Synthetic-fixture tests cover record params, text blocks with fake annotations, line and block comments, nested classes, and enum values. Real Java SDK baseline unchanged: 70 registered types, 39 drift, 8 cross-spec, 1 intra-file.
1 parent 2873454 commit e265443

3 files changed

Lines changed: 137 additions & 60 deletions

File tree

.github/workflows/wire-shape-contract.yml

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,27 @@ jobs:
7272
PR_SHA: ${{ steps.specs_sha.outputs.sha }}
7373
run: |
7474
set -e
75-
BASE_SHA=$(
76-
git show "origin/${BASE_REF}:tests/fixtures/wire-shape-baseline.json" 2>/dev/null \
77-
| python3 -c "import json, sys; print(json.load(sys.stdin).get('openapi_specs_sha','') or '')" \
78-
|| true
79-
)
75+
BASE_FILE=$(mktemp)
76+
if git show "origin/${BASE_REF}:tests/fixtures/wire-shape-baseline.json" > "$BASE_FILE" 2>/dev/null; then
77+
# File exists on the base branch. It MUST parse — a malformed
78+
# baseline file would otherwise let `BASE_SHA` come back empty
79+
# and route this PR through the "first pin introduction"
80+
# bypass below, silently authorizing a SHA bump.
81+
BASE_SHA=$(python3 -c "import json, sys; print(json.load(open(sys.argv[1])).get('openapi_specs_sha','') or '')" "$BASE_FILE")
82+
else
83+
# File genuinely absent on the base branch (first-time
84+
# introduction). Distinguishing this from "file present but
85+
# unparseable" is what guards the bypass.
86+
BASE_SHA=""
87+
fi
88+
rm -f "$BASE_FILE"
8089
if [ -z "$BASE_SHA" ]; then
81-
echo "::notice::Base branch has no openapi_specs_sha yet; treating this PR as first pin introduction."
90+
if git cat-file -e "origin/${BASE_REF}:tests/fixtures/wire-shape-baseline.json" 2>/dev/null; then
91+
echo "::error::tests/fixtures/wire-shape-baseline.json on origin/${BASE_REF} parsed with empty openapi_specs_sha."
92+
echo "::error::This is unrecoverable from inside the gate. Re-run scripts/wire_shape/refresh.py on main."
93+
exit 1
94+
fi
95+
echo "::notice::Base branch has no wire-shape-baseline.json yet; treating this PR as first pin introduction."
8296
exit 0
8397
fi
8498
if [ "$BASE_SHA" = "$PR_SHA" ]; then

scripts/wire_shape/lib.py

Lines changed: 98 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ class whose body contains them.
2020
- Avoids the `mvn compile` round-trip in CI and stays dependency-free
2121
beyond PyYAML. If nested-generic or annotation-inside-string edge
2222
cases start causing false positives, switch to `javalang`.
23+
- Record-parameter annotations (`record Foo(@JsonProperty("x") int x)`)
24+
are attributed to the record because the parser tracks pending
25+
decls between their token and their body `{`.
26+
- Anonymous inner classes (`new Foo() { @JsonProperty("x") ... }`) do
27+
NOT have a `class` keyword, so their annotations leak to the
28+
enclosing type. This SDK does not put `@JsonProperty` inside
29+
anonymous inners; if that ever lands, the `<anon>` sentinel pattern
30+
is the place to add coverage.
2331
"""
2432

2533
from __future__ import annotations
@@ -178,8 +186,26 @@ def _strip_strings_and_comments(src: str) -> str:
178186
j += 2
179187
i = j
180188
continue
181-
# String / char literal. Handles escapes; skips newlines (text
182-
# blocks in Java 15+ break this but none of our SDK types use them).
189+
# Java 15+ text block: `"""..."""`. Without explicit handling,
190+
# the leading `"""` looks like quote-emptyString-quote, then
191+
# content runs as live source until a stray `"` rebalances.
192+
if c == '"' and src[i : i + 3] == '"""':
193+
j = i + 3
194+
out[i] = out[i + 1] = out[i + 2] = " "
195+
while j < n and src[j : j + 3] != '"""':
196+
if src[j] != "\n":
197+
out[j] = " "
198+
j += 1
199+
if j < n:
200+
out[j] = out[j + 1] = out[j + 2] = " "
201+
j += 3
202+
i = j
203+
continue
204+
# String / char literal. Handles escapes; preserves newlines so
205+
# offsets line up with the original text. Java string literals
206+
# cannot span newlines (text blocks above are the only multi-
207+
# line form), so a bare newline mid-quote is a malformed source
208+
# we let fall through.
183209
if c == '"' or c == "'":
184210
quote = c
185211
j = i + 1
@@ -228,19 +254,24 @@ def _extract_types_from_java(content: str) -> dict[str, list[str]]:
228254
# @JsonProperty annotations: match on the RAW content so the
229255
# wire-name string literal is preserved. Positions line up with
230256
# `cleaned` because string-stripping replaces in place without
231-
# changing length.
257+
# changing length. Filter out matches whose `@` falls inside a
258+
# blanked range (string literal, comment, text block) — the
259+
# cleaner replaces those with whitespace, so cleaned[m.start()]
260+
# is a space rather than the original `@`.
232261
props: list[tuple[int, str]] = [
233-
(m.start(), m.group(1)) for m in _JSON_PROPERTY_RE.finditer(content)
262+
(m.start(), m.group(1))
263+
for m in _JSON_PROPERTY_RE.finditer(content)
264+
if m.start() < len(cleaned) and cleaned[m.start()] == "@"
234265
]
235266

236-
# Walk the cleaned source, tracking a stack of (open_brace_pos, type_name).
237-
# When we see `class X` followed (after modifiers/generics) by `{`,
238-
# push X onto the stack at that depth. Pop on matching `}`.
239-
stack: list[str] = []
240-
# Map from brace_depth_at_open -> type_name, so we only pop type
241-
# frames when the matching `}` fires (not every random `{` in
242-
# method bodies / initializers).
243-
type_frames: dict[int, str] = {}
267+
# Walk the cleaned source. Two queues: decls already seen but
268+
# whose body `{` hasn't been hit yet (pending), and active type
269+
# frames (stack). Annotations attribute to the innermost pending
270+
# decl (record-parameter case) if any, otherwise the innermost
271+
# active frame. This keeps record(@JsonProperty(...) X x) {} from
272+
# silently dropping its annotations.
273+
pending_decls: list[str] = []
274+
stack: list[tuple[int, str]] = [] # (depth_at_open, type_name)
244275
depth = 0
245276
next_decl_idx = 0
246277
next_prop_idx = 0
@@ -249,41 +280,47 @@ def _extract_types_from_java(content: str) -> dict[str, list[str]]:
249280
i = 0
250281
n = len(cleaned)
251282
while i < n:
252-
c = cleaned[i]
283+
# Promote any decls whose name token has now been passed.
284+
while next_decl_idx < len(decls) and decls[next_decl_idx][0] <= i:
285+
pending_decls.append(decls[next_decl_idx][1])
286+
next_decl_idx += 1
253287

254-
# Consume any pending annotation at or before this index.
288+
# Consume any annotation at this position. Pending-decl wins
289+
# so record params attribute correctly.
255290
while next_prop_idx < len(props) and props[next_prop_idx][0] <= i:
256291
_, wire = props[next_prop_idx]
257-
if stack:
258-
result.setdefault(stack[-1], []).append(wire)
292+
owner = (
293+
pending_decls[-1] if pending_decls
294+
else stack[-1][1] if stack
295+
else None
296+
)
297+
if owner is not None:
298+
result.setdefault(owner, []).append(wire)
259299
next_prop_idx += 1
260300

301+
c = cleaned[i]
261302
if c == "{":
262-
# Is this the opening brace of the most recent unconsumed
263-
# class declaration? i.e. the declaration's class-name
264-
# token ended before this `{` and no other `{` has consumed it.
265-
if (
266-
next_decl_idx < len(decls)
267-
and decls[next_decl_idx][0] <= i
268-
):
269-
name = decls[next_decl_idx][1]
270-
stack.append(name)
271-
type_frames[depth] = name
272-
next_decl_idx += 1
303+
if pending_decls:
304+
# This brace opens the body of the oldest pending decl.
305+
name = pending_decls.pop(0)
306+
stack.append((depth, name))
273307
depth += 1
274308
elif c == "}":
275309
depth -= 1
276-
if depth in type_frames:
277-
type_frames.pop(depth)
278-
if stack:
279-
stack.pop()
310+
if stack and stack[-1][0] == depth:
311+
stack.pop()
280312
i += 1
281313

282314
# Consume any trailing annotations (defensive; shouldn't happen).
283315
while next_prop_idx < len(props):
284316
_, wire = props[next_prop_idx]
285-
if stack:
286-
result.setdefault(stack[-1], []).append(wire)
317+
owner = (
318+
pending_decls[-1] if pending_decls
319+
else stack[-1][1] if stack
320+
else None
321+
)
322+
if owner is not None:
323+
result.setdefault(owner, []).append(wire)
287324
next_prop_idx += 1
288325

289326
return {k: sorted(set(v)) for k, v in result.items() if v}
@@ -323,24 +360,42 @@ def empty_baseline() -> dict[str, Any]:
323360
def load_baseline() -> dict[str, Any]:
324361
if not BASELINE_PATH.exists():
325362
return empty_baseline()
326-
with BASELINE_PATH.open() as f:
327-
parsed = json.load(f)
363+
try:
364+
with BASELINE_PATH.open() as f:
365+
parsed = json.load(f)
366+
except json.JSONDecodeError as e:
367+
# Truncated mid-write, hand-edited and corrupted, or otherwise
368+
# not parseable. Fail loudly with a regeneration hint instead
369+
# of letting the validator bail with an opaque traceback.
370+
raise SystemExit(
371+
f"❌ tests/fixtures/wire-shape-baseline.json is malformed "
372+
f"({e.__class__.__name__}: {e}).\n"
373+
f" Regenerate via:\n"
374+
f" python3 scripts/wire_shape/refresh.py "
375+
f"<axonflow community specs dir>"
376+
) from None
328377
base = empty_baseline()
329378
base.update(parsed)
330-
base["cross_spec_duplicates"] = parsed.get("cross_spec_duplicates", {})
331-
base["intra_file_duplicates"] = parsed.get("intra_file_duplicates", {})
332-
base["registered_types"] = parsed.get("registered_types", [])
333-
base["per_type_drift"] = parsed.get("per_type_drift", {})
334379
return base
335380

336381

337382
def write_baseline(baseline: dict[str, Any]) -> None:
338383
BASELINE_PATH.parent.mkdir(parents=True, exist_ok=True)
339384
tmp = BASELINE_PATH.with_suffix(f".json.tmp.{os.getpid()}")
340-
with tmp.open("w") as f:
341-
json.dump(baseline, f, indent=2, sort_keys=True)
342-
f.write("\n")
343-
tmp.replace(BASELINE_PATH)
385+
try:
386+
with tmp.open("w") as f:
387+
json.dump(baseline, f, indent=2, sort_keys=True)
388+
f.write("\n")
389+
tmp.replace(BASELINE_PATH)
390+
except BaseException:
391+
# If anything fails between open() and replace(), don't leave
392+
# a stray .tmp.<pid> sidecar — the next run from the same PID
393+
# would collide and confuse a future writer.
394+
try:
395+
tmp.unlink(missing_ok=True)
396+
except OSError:
397+
pass
398+
raise
344399

345400

346401
def difference(a: list[str], b: list[str]) -> list[str]:

scripts/wire_shape/validate.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,35 @@
3535
)
3636

3737

38-
def _specs_dir() -> Path | None:
38+
def main() -> int:
3939
env = os.environ.get("AXONFLOW_OPENAPI_SPECS_DIR")
4040
if not env:
41-
return None
42-
p = Path(env)
43-
return p if p.is_dir() else None
44-
45-
46-
def main() -> int:
47-
specs = _specs_dir()
48-
if specs is None:
41+
# No env at all → local dev / unconfigured CI. Skip.
4942
print(
50-
"⏭️ AXONFLOW_OPENAPI_SPECS_DIR not set to a directory; "
51-
"wire-shape gate skipped."
43+
"⏭️ AXONFLOW_OPENAPI_SPECS_DIR not set; wire-shape gate skipped."
5244
)
5345
print(
5446
" The dedicated CI job clones getaxonflow/axonflow at the "
5547
"pinned SHA and exports this variable before running the "
5648
"validator."
5749
)
5850
return 0
51+
specs = Path(env)
52+
if not specs.is_dir():
53+
# Env set but path is bogus. CI probably failed to check out the
54+
# specs at the pinned SHA; treating that as a skip would let a
55+
# broken pipeline produce a green check. Fail loudly instead.
56+
print(
57+
f"❌ AXONFLOW_OPENAPI_SPECS_DIR={env} is not a directory.",
58+
file=sys.stderr,
59+
)
60+
print(
61+
" The wire-shape job's specs-checkout step must run before "
62+
"this validator. A misconfigured path silently disables the "
63+
"gate, which we refuse to do.",
64+
file=sys.stderr,
65+
)
66+
return 1
5967

6068
merged, cross_spec, intra_file = load_all_schemas(specs)
6169
if not merged:

0 commit comments

Comments
 (0)