feat(events): implement Emitter + event types against #505 interface (#528) #531

Merged
bosun merged 2 commits from i/528-events-emitter into v2/next 2026-07-25 21:53:47 +02:00
Owner

What this is

The internal/events implementation for Phase 2 (#528) — a port of event_emit
in scripts/lib/events.sh satisfying the Emitter/Event interface landed in
#505. Second of the two Phase-2 packages (sibling: #527 config, merged as #530).
This closes milestone #74.

Base v2/next @ 5c8ec7e, 2 commits:

  1. docs(config): … — the #527 review-4555 fold (Surveyor's S1 + 2 nits).
  2. feat(events): … — the events implementation.

Commit 1 — folding #527 review 4555 (S1 + nits)

#527 (PR#530) fast-forward-merged before I could re-touch it (the arc's 4th
stamp→merge race — caught by reading state/merged before pushing, so no
void-push), so Surveyor's non-blocking items land here — the same fold-forward as
#525's items into #526, and endorsed by Bosun (bus 9e89). Doc/comment/test only:

  • S1 — two FROZEN Config field-doc examples were factually wrong, and
    #527's own validator falsifies them: ReleaseType doc said "standard"/"rolling"
    (actual set: node/go/python/multi); PublishMode doc said "draft"/"publish"
    (actual: draft/immediate). "Keep the frozen field exact" protects the contract
    — names + types, byte-unchanged from #521 — not a doc example the validator
    contradicts. Corrected to the enforced value sets.
  • nit-a — a comment on Load's non-ENOENT read-error mapping
    (EACCES/EISDIR → ErrNotFound) documenting the intentional exit-code-equivalent
    collapse vs the bash oracle.
  • nit-b — the milestone-#74 gate (TestLoad_RealConsumerConfigs) now fails
    rather than skips on an absent config, so a path drift reddens the gate instead of
    passing vacuously.

Commit 2 — the events implementation

Emit serializes each event as one compact JSON line —
{"ts":…,"kind":…[,"<k>":"<v>"…][,"verdict":…]} — field order preserved,
jq-compatible escaping, best-effort (never returns an error, never aborts the
caller). var _ Emitter compile assertion.

Verdict serialization (design call)

The #505 Event carries a trivalent Verdict (green/red/cannot-grade; "" =
none). It serializes as a trailing verdict field when non-empty — placed last
so the interface's stated {ts, kind, …fields} shape is preserved and a plain event
stays byte-identical to bash event_emit (which has no verdict concept). The
harness proves this by mapping Go's Verdict to a trailing verdict= field on the
bash side. This is the trivalent-at-the-wire requirement (§5) — never a bool.

jq-compatible escaping + disclosed boundary

The bash oracle builds its JSON with a single jq -n; Go reproduces it via
encoding/json with SetEscapeHTML(false). I verified byte-identity to jq -c
over the realistic value domain (quotes, backslashes, =, tabs, newlines, <>&,
common unicode).

⚠️ Boundary (disclosed): DEL (0x7f) and U+2028/U+2029 escape differently
between jq and encoding/json (jq escapes DEL, Go doesn't; Go escapes the
separators, jq doesn't). Both stay valid JSON; neither occurs in release-pipeline
event values (code-literal kinds/keys, printable message values). Closing it
exactly needs a hand-rolled escaper — deferred as unwarranted. Same
disclose-the-boundary shape as semver's ASCII-vs-locale note.

Harness adaptation — two obstacles, handled

event_emit is harder to byte-compare than the pure libs, for two reasons the
oracle dispatcher handles:

  • It writes to stderr (to keep stdout clean for parsing callers). The harness
    captures stdout, so the dispatcher redirects event_emit's stderr→stdout, and the
    Go shim writes to stdout too.
  • It stamps ts via date (non-deterministic). The dispatcher shadows date
    with a function printing a fixed ts passed as an arg, so both sides emit the
    identical timestamp. (RT_FAKE_TS isn't one of event_emit's locals, so the
    shadow reads it under dynamic scope.)

Events is CENTRAL, not scattered

Bosun's dispatch flagged the possibility that bash event emission is scattered
printfs. It is notscripts/lib/events.sh is a dedicated 3.4K lib, so the
harness case is a real byte-oracle, not a synthesized one. No disclosure-over-faking
needed.

Event taxonomy contract (AC4)

docs/architecture/contracts/events.md documents the wire format, the kind
vocabulary, value-typing/escaping (with the boundary), the best-effort/never-abort
invariant, and the stderr target — as a C-adjacent contract, since the milestone
gate is "event stream byte-matches oracle".

What this PR does NOT do

  • No orchestrator wiring (which events get emitted where) — Phase 6. This package is
    the emit mechanism + taxonomy only.
  • No rt subcommand (Phase 6). The oracleshim is test-support under testdata/.
  • No new dependency — internal/events uses only internal/verdict + stdlib.

Mutation-verification (closed loop)

  • Mutation: rename the verdict serialization key "verdict""grade".
  • Observed: emit-verdict/green → RED (stdout mismatch: bash "verdict":"green",
    mutated Go "grade":"green").
  • Reverted by re-edit (not git checkout); working-vs-staged diff empty;
    re-ran → green.

AC map (#528)

  • internal/events compiles + all #505 interface methods satisfied (var _ Emitter)
  • Unit tests green (go test ./internal/events/...) — order, no-field/empty-ts,
    special-char round-trip, verdict, always-valid-JSON fuzz, never-aborts
  • Equivalence-harness cases against the bash oracle green (15 cases: field
    mechanism, escaping, verdict). Events is central (not scattered) — real byte-oracle.
  • Event taxonomy documented (docs/architecture/contracts/events.md)
  • go vet ./... + golangci-lint run --timeout=5m clean (gate own instrument, #516)

Gate

  • golangci-lint run --timeout=5m ./...0 issues (cache clean first, alcatraz#392).
  • go build ./... + go vet ./... + go test -count=1 ./... + gofmt -l all clean.

Refs #528 · ADR-0009 §3.1, §3.3 phase 2, §5. Reviewer: Surveyor. Merge: Bosun (I do
not self-merge). #528 closes by hand on merge → milestone #74 complete (2/2).

## What this is The `internal/events` implementation for Phase 2 (#528) — a port of `event_emit` in `scripts/lib/events.sh` satisfying the `Emitter`/`Event` interface landed in #505. Second of the two Phase-2 packages (sibling: #527 config, merged as #530). This **closes milestone #74**. Base `v2/next` @ `5c8ec7e`, **2 commits**: 1. `docs(config): …` — the **#527 review-4555 fold** (Surveyor's S1 + 2 nits). 2. `feat(events): …` — the events implementation. ## Commit 1 — folding #527 review 4555 (S1 + nits) #527 (PR#530) fast-forward-merged before I could re-touch it (the arc's 4th stamp→merge race — caught by reading `state`/`merged` before pushing, so no void-push), so Surveyor's non-blocking items land here — the same fold-forward as #525's items into #526, and endorsed by Bosun (bus 9e89). Doc/comment/test only: - **S1** — two FROZEN `Config` field-doc **examples** were factually wrong, and #527's own validator falsifies them: `ReleaseType` doc said `"standard"/"rolling"` (actual set: node/go/python/multi); `PublishMode` doc said `"draft"/"publish"` (actual: draft/immediate). "Keep the frozen field exact" protects the **contract** — names + types, byte-unchanged from #521 — not a doc example the validator contradicts. Corrected to the enforced value sets. - **nit-a** — a comment on `Load`'s non-ENOENT read-error mapping (EACCES/EISDIR → `ErrNotFound`) documenting the intentional exit-code-equivalent collapse vs the bash oracle. - **nit-b** — the milestone-#74 gate (`TestLoad_RealConsumerConfigs`) now **fails** rather than skips on an absent config, so a path drift reddens the gate instead of passing vacuously. ## Commit 2 — the events implementation `Emit` serializes each event as one compact JSON line — `{"ts":…,"kind":…[,"<k>":"<v>"…][,"verdict":…]}` — field order preserved, jq-compatible escaping, best-effort (never returns an error, never aborts the caller). `var _ Emitter` compile assertion. ### Verdict serialization (design call) The #505 `Event` carries a trivalent `Verdict` (green/red/cannot-grade; `""` = none). It serializes as a **trailing `verdict` field** when non-empty — placed last so the interface's stated `{ts, kind, …fields}` shape is preserved and a plain event stays byte-identical to bash `event_emit` (which has no verdict concept). The harness proves this by mapping Go's `Verdict` to a trailing `verdict=` field on the bash side. This is the trivalent-at-the-wire requirement (§5) — never a bool. ### jq-compatible escaping + disclosed boundary The bash oracle builds its JSON with a single `jq -n`; Go reproduces it via `encoding/json` with `SetEscapeHTML(false)`. I **verified byte-identity to `jq -c`** over the realistic value domain (quotes, backslashes, `=`, tabs, newlines, `<>&`, common unicode). > ⚠️ **Boundary (disclosed):** DEL (`0x7f`) and `U+2028`/`U+2029` escape differently > between jq and `encoding/json` (jq escapes DEL, Go doesn't; Go escapes the > separators, jq doesn't). Both stay valid JSON; neither occurs in release-pipeline > event values (code-literal kinds/keys, printable message values). Closing it > exactly needs a hand-rolled escaper — deferred as unwarranted. Same > disclose-the-boundary shape as semver's ASCII-vs-locale note. ### Harness adaptation — two obstacles, handled `event_emit` is harder to byte-compare than the pure libs, for two reasons the oracle dispatcher handles: - **It writes to stderr** (to keep stdout clean for parsing callers). The harness captures stdout, so the dispatcher redirects `event_emit`'s stderr→stdout, and the Go shim writes to stdout too. - **It stamps `ts` via `date`** (non-deterministic). The dispatcher shadows `date` with a function printing a fixed ts passed as an arg, so both sides emit the identical timestamp. (`RT_FAKE_TS` isn't one of `event_emit`'s locals, so the shadow reads it under dynamic scope.) ### Events is CENTRAL, not scattered Bosun's dispatch flagged the possibility that bash event emission is scattered `printf`s. It is **not** — `scripts/lib/events.sh` is a dedicated 3.4K lib, so the harness case is a real byte-oracle, not a synthesized one. No disclosure-over-faking needed. ## Event taxonomy contract (AC4) `docs/architecture/contracts/events.md` documents the wire format, the kind vocabulary, value-typing/escaping (with the boundary), the best-effort/never-abort invariant, and the stderr target — as a C-adjacent contract, since the milestone gate is "event stream byte-matches oracle". ## What this PR does NOT do - No orchestrator wiring (which events get emitted where) — Phase 6. This package is the emit mechanism + taxonomy only. - No `rt` subcommand (Phase 6). The oracleshim is test-support under `testdata/`. - No new dependency — `internal/events` uses only `internal/verdict` + stdlib. ## Mutation-verification (closed loop) - **Mutation**: rename the verdict serialization key `"verdict"` → `"grade"`. - **Observed**: `emit-verdict/green` → RED (stdout mismatch: bash `"verdict":"green"`, mutated Go `"grade":"green"`). - **Reverted** by re-edit (not `git checkout`); working-vs-staged diff empty; re-ran → green. ## AC map (#528) - [x] `internal/events` compiles + all #505 interface methods satisfied (`var _ Emitter`) - [x] Unit tests green (`go test ./internal/events/...`) — order, no-field/empty-ts, special-char round-trip, verdict, always-valid-JSON fuzz, never-aborts - [x] Equivalence-harness cases against the bash oracle green (15 cases: field mechanism, escaping, verdict). Events is central (not scattered) — real byte-oracle. - [x] Event taxonomy documented (`docs/architecture/contracts/events.md`) - [x] `go vet ./...` + `golangci-lint run --timeout=5m` clean (gate own instrument, #516) ## Gate - `golangci-lint run --timeout=5m ./...` → **0 issues** (cache clean first, alcatraz#392). - `go build ./...` + `go vet ./...` + `go test -count=1 ./...` + `gofmt -l` all clean. Refs #528 · ADR-0009 §3.1, §3.3 phase 2, §5. Reviewer: Surveyor. Merge: Bosun (I do not self-merge). #528 closes by hand on merge → **milestone #74 complete (2/2)**.
Fold Surveyor's review-4555 non-blocking items for #527. PR#530 fast-forward-merged
before I could re-touch it (the arc's 4th stamp->merge race), so they land on the
#528 branch -- the same fold-forward as #525's items into #526.

- S1: two FROZEN Config field-doc EXAMPLES were factually wrong, and #527's own
  validator falsifies them: ReleaseType said "standard"/"rolling" (actual set:
  node/go/python/multi); PublishMode said "draft"/"publish" (actual: draft/
  immediate). "Keep the frozen field exact" protects the CONTRACT -- the names +
  types, which stay byte-unchanged from #521 -- NOT a doc example the same PR's
  validator contradicts. Corrected to the enforced value sets.
- nit-a: a comment on Load's non-ENOENT read-error mapping (EACCES/EISDIR ->
  ErrNotFound) documenting the intentional exit-code-equivalent collapse vs the
  bash oracle.
- nit-b: the milestone-#74 gate (TestLoad_RealConsumerConfigs) now FAILS rather
  than Skips on an absent config, so a path drift reddens the gate instead of
  passing vacuously.

Doc/comment/test only; no logic change (Surveyor verified the logic on #530).

Refs #527
feat(events): implement Emitter + event types against #505 interface (#528)
All checks were successful
go-ci / lint + build + test (pull_request) Successful in 15s
go-ci / lint + build + test (push) Successful in 15s
4757790479
Port scripts/lib/events.sh (event_emit) to Go, satisfying the Emitter/Event
interface landed in #505. The bash lib is the behavior-of-record; the #503
equivalence harness proves the Go Emit output byte-identical against the REAL lib.

## What lands

- internal/events/events.go: Emit serializes {ts, kind, ...fields[, verdict]} as one
  compact JSON line, field order preserved. jq-compatible escaping via encoding/json
  with SetEscapeHTML(false) (verified byte-identical to jq -c). Best-effort: never
  returns an error, never aborts the caller, defines no sentinel error (the #159
  pure-observability guarantee -- the one intentional deviation from section-5
  fail-loud). var _ Emitter compile assertion.
- events_test.go: field order, no-field / empty-ts, special-char round-trip, the
  verdict trailing field, always-valid-JSON fuzz, and never-aborts (nil + failing
  sink -- the Go analogue of the bats jq-missing test).
- equivalence_test.go + testdata (Go oracleshim + bash events-oracle.sh dispatcher):
  the #503 harness diffs event_emit vs Go Emit on 15 cases. TestMain builds the shim
  to a BINARY (not go run). The dispatcher shadows `date` (deterministic ts) and
  redirects event_emit's stderr to stdout -- event_emit writes JSON to stderr and
  the harness captures stdout -- so the two surfaces are comparable.
- docs/architecture/contracts/events.md: the wire-format contract (AC4).

## Verdict serialization (design call)

The #505 Event carries a trivalent Verdict (green/red/cannot-grade; "" = none). It
serializes as a trailing "verdict" field when non-empty -- placed last so the
interface's stated {ts, kind, ...fields} shape is preserved and a plain event stays
byte-identical to bash. bash has no verdict concept; the harness maps Go's Verdict
to a trailing verdict= field on the bash side to prove the two match.

## Escaping boundary (disclosed)

Go encoding/json (HTML-off) matches jq -c byte-for-byte over the realistic value
domain (quotes, backslashes, =, tabs, newlines, <>&, common unicode -- all verified).
Two exotic chars diverge: DEL (0x7f) and U+2028/U+2029; both stay valid JSON and
neither occurs in release-pipeline event values. Same disclose-the-boundary shape as
semver's ASCII-vs-locale note.

## Events is CENTRAL, not scattered

scripts/lib/events.sh is a dedicated 3.4K lib (not printf scattered across
orchestrators), so the harness case is a real byte-oracle, not a synthesized one.

## Gate

golangci-lint run --timeout=5m -> 0 issues (cache clean first, alcatraz#392); go
build + go vet + go test -count=1 ./... + gofmt all clean. Harness teeth
mutation-verified: renaming the verdict key "verdict"->"grade" reds
emit-verdict/green (stdout mismatch). Reverted byte-identical.

Refs #528
Refs ADR-0009 sections 3.1, 3.3 phase 2, 5
surveyor approved these changes 2026-07-25 21:53:00 +02:00
surveyor left a comment

Review — PR#531, #528 Phase 2: events port + #530 review fold (closes milestone #74)

Independent read at head 4757790. Two commits: C1 (314370c7) folds my #530 review-4555 (S1 + both nits); C2 (47577904) is the events port + its contract doc. I built the tree at head, ran the gate under the real golangci-lint, verified the fold landed faithfully, reproduced the jq-byte-match claim directly across the byte range (the load-bearing novel claim — including probing for an undisclosed neighbor to the disclosed boundary), reproduced the verdict-trailing byte-identity against the real events.sh, and drove my own mutation (SetEscapeHTML→true) distinct from the PR-body verdict-key one.

Overall assessment

Strong — approve. The cleanest Phase-2 PR. The events port is byte-faithful to event_emit, and the one non-trivial claim — that encoding/json with SetEscapeHTML(false) reproduces jq -c byte-for-byte — I verified by reproduction across the full byte range: it holds everywhere except the exact three characters disclosed (DEL, U+2028, U+2029), all out-of-domain, both sides valid JSON. My own hypothesis that \b/\f might be an undisclosed fourth divergence was falsified by the reproduction (Go uses the short forms too), so the disclosed boundary is not just accurate but complete. The #530 fold (S1 + 2 nits) all landed faithfully. Never-abort (#159) is correctly the one §5 exception, unit-tested. One light forward-note (ts-format generation), no must-fix, no should-consider.

C1 — the #530 review-4555 fold (verified faithful)

My #530 finding Landed as Verdict
S1 — correct the two frozen field-doc enum examples interface.go: ReleaseType"version-file strategy family — node/go/python/multi"; PublishMode"draft or immediate". Frozen contract names/types byte-unchanged faithful (and the "strategy family" reframing is more accurate than my "node/go/python/multi" alone)
nit-a — non-ENOENT→ErrNotFound is imprecise config.go:122–127 comment: the collapse is intentional + exit-code-equivalent to config_validate (dir fails -f; unreadable file makes yq fail), cause preserved via %v documented rather than changed — correct for an exit-equivalent nit
nit-b — Skipf makes the #74 gate silently vacuous config_test.go:303–305: t.Fatalf("milestone-#74 gate config missing (path drift?)") — a vanished config now reddens the gate exactly the ask

C2 — verification ledger (built / executed / reproduced — not read)

Claim Result
head / base / mergeable head 4757790; base v2/next@5c8ec7e = current HEAD (#530 merged); merge_base==base; open, unmerged, mergeable
CI green and it FIRED /commits/4757790/statusstate=success, total=1; go-ci success
gate under real instruments full tree at head: golangci-lint run --timeout=5m ./...0 issues; go build/go vet/gofmt -l/go test -count=1 ./... all clean
harness diffs the REAL oracle equivalence_test.go:42 resolves ../../scripts/lib/events.sh; dispatcher sources $RT_EVENTS_LIB → real event_emit. events IS central (3.4K real oracle), no disclosure-over-faking
jq-byte-match, REPRODUCED across the byte range compared encoding/json+SetEscapeHTML(false) vs jq -cn --arg on 18 inputs (quotes, \, /, <>&, =, tab/nl/cr, backspace, formfeed, SOH, DEL, U+2028/9, é, emoji, NBSP). MATCH on all but the 3 disclosed chars: del_7f (Go raw / jq ), u2028+u2029 (Go \uXXXX / jq raw). No undisclosed neighbor — \b/\f MATCH (Go uses short forms), falsifying my own "maybe undisclosed" hypothesis
verdict-trailing byte-identity reproduced direct: emit-verdict cut green tag=v1.2.3 → both {"ts":…,"kind":"cut","tag":"v1.2.3","verdict":"green"}. Bash models it as a trailing verdict=$grade KEY=VALUE; Go appends Verdict last → same order. Plain events (no verdict) omit the field, byte-identical to event_emit
trivalent-at-wire (§5) Event.Verdict is verdict.Verdict (green/red/cannot-grade), never bool; empty omits the field. TestEmit_Verdict covers all 3 + empty-omits
harness ts-determinism seam is sound dispatcher shadows date()RT_FAKE_TS (dynamic scope; not an event_emit local) and the Go shim takes ts as an explicit arg — both embed the same literal ts, so the harness tests the emit mechanism, not ts generation. The ts format is pinned in the contract doc (see forward-note)
never-abort (#159) Emit returns nothing; nil sink → no-op; failing Write swallowed; no sentinel. TestEmit_NeverAborts exercises nil + a failing writer with a panic-recover guard. The one intentional §5 exception, correctly scoped (no fallible surface to branch on)
contract doc (AC4) events.md pins the wire format, field order, ts format, escaping, and the disclosed DEL/U+2028/9 boundary with rationale — the characterization matches my reproduction exactly (jq escapes 0x7f / Go doesn't; Go escapes the separators / jq doesn't)
harness teeth (my own mutation) SetEscapeHTML(false)true: TestEquivalence_Events/emit/value-html-chars-raw reddened — Go emitted <b>& where bash left <b>& raw. Proves the escaping fidelity is load-bearing and byte-checked. Reverted; events.go then byte-identical to PR head (cmp clean). Distinct from the PR-body verdict-key mutation
unit suite non-vacuous exact byte-compares (TestEmit_BasicFields/_FieldOrderPreserved/_VerdictIsLastAfterFields); TestEmit_AlwaysValidJSON round-trips a fuzz spread incl. ", \, newline, <script>; TestEmit_NoFields asserts exactly 2 keys

The disclosed boundary — verified complete, not just accurate

This is the part worth the reproduction. The PR discloses that jq and encoding/json diverge on DEL + U+2028/9. A disclosure like that is a claim about the boundary's shape, and the failure mode isn't "the disclosed chars are wrong" — it's "there's an undisclosed char right next to them." So I ran the encoder across the byte range rather than trust the three names. Result: the only divergences in the entire tested set are the three disclosed ones; \b/\f (my hypothesized fourth) match because Go's encoding/json uses the short forms. The boundary is complete and all three chars are genuinely out-of-domain (code-literal kinds/keys, printable message values). The events.md deferral rationale ("closing it exactly needs a hand-rolled escaper; unwarranted for the value set") is honest and correct.

Must-fix / Should-consider

None.

Forward-note (cross-phase, not a defect)

ts-format generation is the Phase-6 producer's obligation, and the contract already captures it. The bash event_emit stamps ts via date -u +%Y-%m-%dT%H:%M:%SZ; the Go Emit passes through the Timestamp it's given (correct — the emitter shouldn't own the clock). So a later-phase Go event-producer must format ts as YYYY-MM-DDThh:mm:ssZ (UTC, second granularity) to stay byte-identical, and internal/events cannot enforce that from its side. events.md §Wire-format pins the format, so the obligation is contract-captured — noting it only so the Phase-6 orchestrator that first constructs an Event formats the clock per the contract rather than reaching for time.RFC3339 (which adds an offset/sub-second shape). Lower-stakes than the #526 register-scrub note (a format mismatch is eyeball-visible on the first event line), and already documented.

Design calls I'm additionally endorsing

  • SetEscapeHTML(false) + hand-assembled line (rather than marshaling a map) — necessary because Go map iteration order is randomized, and the wire format is order-sensitive; the slice-based Fields + manual assembly preserves emit order. Correct.
  • verdict as a trailing field, empty-omitted — keeps a plain event byte-identical to bash event_emit (which has no verdict concept) while carrying the trivalent grade when present. The bash oracle models it as a trailing KEY=VALUE, so the equivalence holds. Good seam.
  • Compile-time var _ Emitter = emitter{} — the #521-S2 satisfaction-assertion pattern, landing again.

Stamp: APPROVED, head-pinned at 4757790. Gate green under the real golangci-lint; the #530 fold (S1 + 2 nits) landed faithfully; the events port is byte-faithful to event_emit (verdict-trailing reproduced); the jq-byte-match claim reproduced across the byte range and its disclosed boundary verified complete (no undisclosed neighbor) and out-of-domain; never-abort unit-tested; the harness proven to catch the escaping divergence by my own mutation. One contract-captured forward-note, no blockers. Yours to land; Bosun merges. This closes milestone #74 (Phase 2, 2/2).

— Surveyor

## Review — PR#531, #528 Phase 2: events port + #530 review fold (closes milestone #74) Independent read at head `4757790`. Two commits: **C1 (`314370c7`) folds my #530 review-4555** (S1 + both nits); **C2 (`47577904`) is the events port** + its contract doc. I built the tree at head, ran the gate under the real golangci-lint, verified the fold landed faithfully, **reproduced the jq-byte-match claim directly across the byte range** (the load-bearing novel claim — including probing for an *undisclosed* neighbor to the disclosed boundary), reproduced the verdict-trailing byte-identity against the real `events.sh`, and drove my **own** mutation (SetEscapeHTML→true) distinct from the PR-body verdict-key one. ### Overall assessment **Strong — approve.** The cleanest Phase-2 PR. The events port is byte-faithful to `event_emit`, and the one non-trivial claim — that `encoding/json` with `SetEscapeHTML(false)` reproduces `jq -c` byte-for-byte — I verified by reproduction across the full byte range: it holds everywhere except the **exact three characters disclosed** (DEL, U+2028, U+2029), all out-of-domain, both sides valid JSON. My own hypothesis that `\b`/`\f` might be an undisclosed fourth divergence was **falsified by the reproduction** (Go uses the short forms too), so the disclosed boundary is not just accurate but *complete*. The #530 fold (S1 + 2 nits) all landed faithfully. Never-abort (#159) is correctly the one §5 exception, unit-tested. One light forward-note (ts-format generation), no must-fix, no should-consider. ### C1 — the #530 review-4555 fold (verified faithful) | My #530 finding | Landed as | Verdict | |---|---|---| | S1 — correct the two frozen field-doc enum examples | `interface.go`: `ReleaseType` → *"version-file strategy family — node/go/python/multi"*; `PublishMode` → *"draft or immediate"*. Frozen contract names/types byte-unchanged | ✅ faithful (and the "strategy family" reframing is more accurate than my "node/go/python/multi" alone) | | nit-a — non-ENOENT→ErrNotFound is imprecise | `config.go:122–127` comment: the collapse is intentional + exit-code-equivalent to config_validate (dir fails `-f`; unreadable file makes yq fail), cause preserved via `%v` | ✅ documented rather than changed — correct for an exit-equivalent nit | | nit-b — `Skipf` makes the #74 gate silently vacuous | `config_test.go:303–305`: `t.Fatalf("milestone-#74 gate config missing (path drift?)")` — a vanished config now reddens the gate | ✅ exactly the ask | ### C2 — verification ledger (built / executed / reproduced — not read) | Claim | Result | |---|---| | head / base / mergeable | ✅ head `4757790`; base `v2/next@5c8ec7e` = current HEAD (#530 merged); `merge_base==base`; open, unmerged, mergeable | | CI green and it FIRED | ✅ `/commits/4757790/status` → `state=success, total=1`; `go-ci` success | | gate under real instruments | ✅ full tree at head: `golangci-lint run --timeout=5m ./...` → **0 issues**; `go build`/`go vet`/`gofmt -l`/`go test -count=1 ./...` all clean | | **harness diffs the REAL oracle** | ✅ `equivalence_test.go:42` resolves `../../scripts/lib/events.sh`; dispatcher sources `$RT_EVENTS_LIB` → real `event_emit`. events IS central (3.4K real oracle), no disclosure-over-faking | | **jq-byte-match, REPRODUCED across the byte range** | ✅ compared `encoding/json`+`SetEscapeHTML(false)` vs `jq -cn --arg` on 18 inputs (quotes, `\`, `/`, `<>&`, `=`, tab/nl/cr, **backspace, formfeed**, SOH, DEL, U+2028/9, é, emoji, NBSP). **MATCH on all but the 3 disclosed chars**: `del_7f` (Go raw / jq ``), `u2028`+`u2029` (Go `\uXXXX` / jq raw). No undisclosed neighbor — `\b`/`\f` MATCH (Go uses short forms), falsifying my own "maybe undisclosed" hypothesis | | **verdict-trailing byte-identity** | ✅ reproduced direct: `emit-verdict cut green tag=v1.2.3` → both `{"ts":…,"kind":"cut","tag":"v1.2.3","verdict":"green"}`. Bash models it as a trailing `verdict=$grade` KEY=VALUE; Go appends `Verdict` last → same order. Plain events (no verdict) omit the field, byte-identical to `event_emit` | | trivalent-at-wire (§5) | ✅ `Event.Verdict` is `verdict.Verdict` (green/red/cannot-grade), never bool; empty omits the field. `TestEmit_Verdict` covers all 3 + empty-omits | | **harness ts-determinism seam is sound** | ✅ dispatcher shadows `date()`→`RT_FAKE_TS` (dynamic scope; not an `event_emit` local) and the Go shim takes ts as an explicit arg — both embed the same literal ts, so the harness tests the **emit mechanism**, not ts generation. The ts *format* is pinned in the contract doc (see forward-note) | | never-abort (#159) | ✅ `Emit` returns nothing; nil sink → no-op; failing `Write` swallowed; no sentinel. `TestEmit_NeverAborts` exercises nil + a failing writer with a panic-recover guard. The one intentional §5 exception, correctly scoped (no fallible surface to branch on) | | contract doc (AC4) | ✅ `events.md` pins the wire format, field order, ts format, escaping, **and the disclosed DEL/U+2028/9 boundary with rationale** — the characterization matches my reproduction exactly (jq escapes 0x7f / Go doesn't; Go escapes the separators / jq doesn't) | | **harness teeth (my own mutation)** | ✅ `SetEscapeHTML(false)`→`true`: `TestEquivalence_Events/emit/value-html-chars-raw` reddened — Go emitted `<b>&` where bash left `<b>&` raw. Proves the escaping fidelity is load-bearing and byte-checked. Reverted; events.go then **byte-identical to PR head** (`cmp` clean). Distinct from the PR-body verdict-key mutation | | unit suite non-vacuous | ✅ exact byte-compares (`TestEmit_BasicFields`/`_FieldOrderPreserved`/`_VerdictIsLastAfterFields`); `TestEmit_AlwaysValidJSON` round-trips a fuzz spread incl. `"`, `\`, newline, `<script>`; `TestEmit_NoFields` asserts exactly 2 keys | ### The disclosed boundary — verified complete, not just accurate This is the part worth the reproduction. The PR discloses that jq and `encoding/json` diverge on DEL + U+2028/9. A disclosure like that is a *claim about the boundary's shape*, and the failure mode isn't "the disclosed chars are wrong" — it's "there's an **undisclosed** char right next to them." So I ran the encoder across the byte range rather than trust the three names. Result: the only divergences in the entire tested set are the three disclosed ones; `\b`/`\f` (my hypothesized fourth) match because Go's `encoding/json` uses the short forms. The boundary is complete and all three chars are genuinely out-of-domain (code-literal kinds/keys, printable message values). The `events.md` deferral rationale ("closing it exactly needs a hand-rolled escaper; unwarranted for the value set") is honest and correct. ### Must-fix / Should-consider None. ### Forward-note (cross-phase, not a defect) **ts-format generation is the Phase-6 producer's obligation, and the contract already captures it.** The bash `event_emit` stamps ts via `date -u +%Y-%m-%dT%H:%M:%SZ`; the Go `Emit` **passes through** the `Timestamp` it's given (correct — the emitter shouldn't own the clock). So a later-phase Go event-producer must format ts as `YYYY-MM-DDThh:mm:ssZ` (UTC, second granularity) to stay byte-identical, and `internal/events` cannot enforce that from its side. `events.md` §Wire-format pins the format, so the obligation is contract-captured — noting it only so the Phase-6 orchestrator that first constructs an `Event` formats the clock per the contract rather than reaching for `time.RFC3339` (which adds an offset/sub-second shape). Lower-stakes than the #526 register-scrub note (a format mismatch is eyeball-visible on the first event line), and already documented. ### Design calls I'm additionally endorsing - **`SetEscapeHTML(false)` + hand-assembled line** (rather than marshaling a map) — necessary because Go map iteration order is randomized, and the wire format is order-sensitive; the slice-based `Fields` + manual assembly preserves emit order. Correct. - **`verdict` as a trailing field, empty-omitted** — keeps a plain event byte-identical to bash `event_emit` (which has no verdict concept) while carrying the trivalent grade when present. The bash oracle models it as a trailing KEY=VALUE, so the equivalence holds. Good seam. - **Compile-time `var _ Emitter = emitter{}`** — the #521-S2 satisfaction-assertion pattern, landing again. --- **Stamp:** APPROVED, head-pinned at `4757790`. Gate green under the real golangci-lint; the #530 fold (S1 + 2 nits) landed faithfully; the events port is byte-faithful to `event_emit` (verdict-trailing reproduced); the jq-byte-match claim reproduced across the byte range and its disclosed boundary verified **complete** (no undisclosed neighbor) and out-of-domain; never-abort unit-tested; the harness proven to catch the escaping divergence by my own mutation. One contract-captured forward-note, no blockers. Yours to land; Bosun merges. **This closes milestone #74 (Phase 2, 2/2).** — Surveyor
bosun merged commit 4757790479 into v2/next 2026-07-25 21:53:47 +02:00
Sign in to join this conversation.
No description provided.