Repository navigation
fix(events): the write core refuses an issue-scoped event with an empty issue (FAFF-1353) - #1094
Merged
alechill merged 7 commits intoOct 20, 2026
Conversation
…event with an empty issue Signed-off-by: Alec Hill <alec@shftwst.dev>
…ty issue (FAFF-1353) appendRecordUnderLock now applies the issue-scoped rule to every minted record, including when the payload check is skipped (a non-delivery profile or an unavailable schema bundle). The rule lives in one predicate, issueScopedViolations, which eventViolations also calls at its old position. The core resolves the governance profile once, before the lock, and passes it to logRecordChecker and to the mint callback. corrective.js uses that profile instead of resolving its own inside the lock. Signed-off-by: Alec Hill <alec@shftwst.dev>
Signed-off-by: Alec Hill <alec@shftwst.dev>
Contributor
|
Ready to land. Run this locally in a real terminal: |
Signed-off-by: Alec Hill <alec@shftwst.dev>
alechill
marked this pull request as draft
October 20, 2026 16:47
…ters cannot hang on Node 24 (FAFF-1353) Every typecheck run on Node 24.22.0 hit the job's 20-minute timeout, each time in a test that runs `faff validate-adapters` on the real tree; every run on 24.21.0 passed. validate-adapters spawns eval/prefix-planner.mjs, which ended with process.exit(). On Node 24, process.exit() can deadlock while a V8 background job is parked (nodejs/node#64274, fix pending in #66171), and the spawn had no timeout. The planner's main() now returns its exit code and the entry sets process.exitCode, as bin/faff already does. validate-adapters gives the planner call a 60-second timeout, so a hung planner fails that lint by name instead of stalling the caller. Signed-off-by: Alec Hill <alec@shftwst.dev>
… marker Signed-off-by: Alec Hill <alec@shftwst.dev>
…ang fix and review round 2 Signed-off-by: Alec Hill <alec@shftwst.dev>
alechill
marked this pull request as ready for review
October 20, 2026 17:39
alechill
deleted the
FAFF-1353-write-core-refuses-issue-scoped-event-with-empty-issue
branch
October 20, 2026 17:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A per-issue gate kernel that captures a decision under a live
$FAFF_RUN_DIR, with no correlation id or issue exported, wrote adecision-capturerow withissue: "". The events write core accepted it, becauseappendRecordUnderLockonly ran the v0.3 payload schema, which allows an empty string.faff events validatethen rejected the row, andmintIssueAnchorcopied it into every per-issue anchor of the run. Fourteen such rows sit inrun-20261018-174517-beepboop-list-2710a1.The write core now refuses an issue-scoped event with an empty
issue, including when the payload check is skipped because of a governance-profile override or an unavailable schema bundle.This PR also fixes the
typecheckjob hanging on Node 24.22, which blocked this PR's CI.Closes FAFF‑1353. Slice 1 of 2 under FAFF‑1348; FAFF‑1354 makes the refusal visible on the capture path.
Changes
issueScopedViolations(record, profile)inplugin/skills/faff/bin/lib/events.js, exported. It holds the rule thateventViolationsapplied inline, with the same violation string.eventViolationscalls it at the old position, after the phase and type checks and before the outcome check, so violation order is unchanged.appendRecordUnderLockresolves the governance profile once, before the lock. It passes that profile tologRecordChecker, to the mint callback as a trailing argument, and toissueScopedViolations, which runs on every minted record after the null-record abort. A record that fails raises oneEVENT_SCHEMAerror listing the issue violation first, then any payload violations. The error's code, message prefix and string array are unchanged.appendRecordUnderLockno longer says an unavailable bundle "degrades the write to eventViolations alone", which was false.logRecordCheckerinevidence.jstakes a trailingprofileparameter that defaults toactiveProfile(). Its twodeclared-effects.jsonlcallers are unchanged.corrective.jspasses the profile it receives to its existingeventViolationscall, so it no longer reads the override file whileevents.jsonl.lockis held.eventViolationsvocabulary. Structural writes such asrun-start,ledger-writeandsentry-tripstill go through under a profile override whose vocabulary does not name them.records/specs/.Node 24
typecheckhangEvery
typecheckrun on Node 24.22.0 hit the job's 20-minute timeout: four on FAFF‑1332's PR and three on this one. Every run on 24.21.0 passed. Each stall was in a test that runsfaff validate-adapterson the real tree. That command spawnseval/prefix-planner.mjs, which ended withprocess.exit(). On Node 24,process.exit()can deadlock while a V8 background job is parked (nodejs/node#64274; fix pending in nodejs/node#66171), and the spawn had no timeout.eval/prefix-planner.mjs:main()returns its exit code and the entry setsprocess.exitCode, asbin/faffalready does. Exit codes are unchanged.validate-adapters.js: the planner call has a 60-second timeout, so a hung planner fails that lint by name instead of stalling the caller.Verification
test/faff-1353-write-core-issue-scoped.test.mjsand one regression case intest/faff-989-decision-capture-driver.test.mjs. Most fail on the base commit. The corrective lock-ordering probe records three reads of the override file on the base, the third while the lock exists, and two reads with no lock after the change.faff events validate --fileover all 421 trackedevents.jsonlfiles, with$FAFF_GOVERNANCE_PROFILEunset, gives identical output and exit codes on the base commit and on this branch.typecheckon Node 24.22.0 after the planner fix: 7033 tests, 0 failures, in 9.5 minutes.Review
Two rounds. Standard review: no findings either round. An adversarial second opinion from
z-ai/glm-5.3-flashraised two findings in round 1 and carried both into round 2. Both were refuted against the source:validateProfileShaperequiresissue_scoped_types, and the schema-unavailable marker records only writes that happened. The test now says why no marker is expected. Round 2 raised nothing on the planner fix. Details are on the issue.Acceptance Criteria
faff-1353#done:1Appending adecision-capturerecord withissue: ""throughappendEventRecordthrowsEVENT_SCHEMA, its violations name the issue-scoped rule, andevents.jsonlis byte-identical before and after the call.Verified: Default-profile refusal tests pass: EVENT_SCHEMA names the rule, events.jsonl byte-identical.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::default profile: a decision-capture append with issue '' is refused and writes nothingMethod: test
test/faff-1353-write-core-issue-scoped.test.mjs::default profile: a decision-capture append with no issue key is refused and writes nothingfaff-1353#done:2The same append is refused, withevents.jsonlbyte-identical, when$FAFF_GOVERNANCE_PROFILEpoints at a shape-valid profile file whoseissue_scoped_typesincludes the type. The path wherelogRecordCheckerreturns null no longer lets the row through.Verified: Override declaring the type issue-scoped refuses the append; file byte-identical.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::profile override declaring the type issue-scoped: the append is refused and writes nothingfaff-1353#done:3The same append is refused, withevents.jsonlbyte-identical, when the schema bundle is unavailable, set up the waytest/evidence-payload-writers.test.mjs:191does it withpluginCopyplusmintRun. Theevidence-schema-unavailable.jsonmarker and stderr line do not let the write through.Verified: Plugin copy with run-event.schema.json removed still refuses; file byte-identical.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::unavailable schema bundle: the append is still refused and writes nothingfaff-1353#done:4git diffagainst the base names no file underverification/evidence/v0.3/orplugin/skills/faff/contracts/evidence-v0.3/, and does not namescripts/evidence-v0.3-compose.mjs.Verified: git diff cb1a9f1..HEAD names only events.js, evidence.js, corrective.js, two tests and the spec.
Method: command
git diff --name-only cb1a9f15..HEADfaff-1353#done:5The issue-scoped rule exists once in the tree, as a predicate over a record and a resolved profile, called by botheventViolationsand the write core. No second copy of its violation string exists in production code underplugin/skills/faff/bin/lib/(EVENT_ISSUE_SCOPEDatevents.js:96is a delivery-keyed set used only bytest/park-reconsider.test.mjs:190; it carries no violation string and is not a second implementation).eventViolationscalls it after thephaseandtypechecks and before the outcome check, so for everyevents.jsonltracked in git (git ls-files '*events.jsonl', 420 files atHEAD, mostly under.faff/anchors/),faff events validate --file, run with$FAFF_GOVERNANCE_PROFILEunset, prints byte-identical stdout and stderr, and exits with the same code, before and after the change.Verified: One predicate, issueScopedViolations; the violation string appears once in bin/lib; validate output identical over 421 tracked events.jsonl files.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::issueScopedViolations is the rule eventViolations applies, between the type check and the outcome checkMethod: command
grep -rn "is issue-scoped but missing required field" plugin/skills/faff/bin/lib/Method: command
faff events validate --file <f> at cb1a9f15 vs HEAD for every git ls-files '*events.jsonl' (FAFF_GOVERNANCE_PROFILE unset)faff-1353#done:6Under a$FAFF_GOVERNANCE_PROFILEoverride whoseevent_phasesandevent_typesare disjoint from delivery's,emitGenesisRunStartstill appends itsrun-start, andfaff run-ledger init-interactiverefused with exit 3 because a live L3/L4 run is already resolved (thesentry-tripsite atrun-ledger.js:315) still appends itssentry-triprow to that live run'sevents.jsonl. That append sits in a best-effortcatch, so the exit code alone proves nothing: the criterion is the row's presence. Neither write throwsEVENT_SCHEMA. The core refuses no record over a phase or type its dialect does not name.Verified: Under a disjoint override, genesis run-start and the init-interactive exit-3 sentry-trip rows land.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::disjoint override: emitGenesisRunStart still appends its run-startMethod: test
test/faff-1353-write-core-issue-scoped.test.mjs::disjoint override: a refused init-interactive against a live L4 run still appends its sentry-tripfaff-1353#done:7Under that same override, the core still accepts aledger-writerecord with nodata.ledger_sha256, because the core never runs envelope mode. That rule stays enforced only where it is enforced today.Verified: Under a disjoint override, ledger-write with no ledger_sha256 is accepted.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::disjoint override: the core accepts a ledger-write with no ledger_sha256 because it never runs envelope modefaff-1353#done:8During one in-processappendEventRecordcall under a$FAFF_GOVERNANCE_PROFILEoverride, the override file is read exactly once, and<run dir>/events.jsonl.lockdoes not exist at the moment of that read.Verified: appendEventRecord reads the override once, lock absent.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::appendEventRecord under an override reads the override once, with no lock heldfaff-1353#done:9During one in-processcmdCorrectiveAuthorcall (faff corrective author --op park-with-cause --cites-signal scope-drift --issue FAFF‑1 --cause "probe" --run-dir <R>;--issueis required atcorrective.js:431andpark-with-causeneeds a non-empty--causeat:435) under a$FAFF_GOVERNANCE_PROFILEoverride pointing at a JSON copy ofDELIVERY_PROFILE:Verified: cmdCorrectiveAuthor reads the override twice, never under the lock, event_appended true; probe fails at the base (3 reads, third under lock).
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::faff corrective author under an override reads it twice, never with the lock held, and appends the eventfaff-1353#done:10With$FAFF_GOVERNANCE_PROFILEpointing at an unreadable or malformed file,appendEventRecordthrowsGovernanceProfileErrorand writes nothing, as that call site does today.Verified: Missing and malformed overrides throw GovernanceProfileError; nothing written.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::an unreadable or malformed override makes appendEventRecord throw GovernanceProfileError and write nothingfaff-1353#done:11A record that fails both halves raises oneEVENT_SCHEMA. Itsviolationsarray holds the issue-scoped violation followed by the payload violations, all plain strings, and itscodeand message prefix are unchanged.Verified: One EVENT_SCHEMA, issue violation first, plain strings, message prefix unchanged.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::a record failing both halves raises one EVENT_SCHEMA listing the issue violation first, as plain stringsfaff-1353#done:12faff events appendof{"phase":"build","type":"build-start"}with noissuestill exits 1, prints one stderr line per violation, and creates noevents.jsonl.test/events.test.mjs:139and:244pass without modification.Verified: faff events append pre-check exits 1, no events.jsonl; test/events.test.mjs passes unmodified.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::faff events append of an issue-scoped type with no issue exits 1 and creates no events.jsonlMethod: command
node --test test/events.test.mjsfaff-1353#done:13All eightappendRecordUnderLockcall sites,emitGenesisRunStartincluded, still append under the default profile. No existingtest/*.test.mjsfile fails withEVENT_SCHEMAon a path that passed before. The diff leaves the seven mint callbacks other thancorrective.js's unchanged.Verified: Full suite passes (4 CI shards: 7031 pass, 0 fail, 6 skipped); diff leaves the seven other mint callbacks unchanged.
Method: command
node --import ./test/hermetic-env.mjs --test --test-shard=<n>/4Method: command
faff gates run --jsonfaff-1353#done:14The comment aboveappendRecordUnderLockno longer says an unavailable bundle "degrades the write to eventViolations alone". It states that the issue-scoped rule runs on every minted record, and that an unavailable bundle or a non-delivery profile skips only the payload check.Verified: Comment rewritten; the old 'degrades the write to eventViolations alone' text is gone.
Method: command
grep -n "degrades the write to eventViolations alone" plugin/skills/faff/bin/lib/events.jsfaff-1353#done:15Under a$FAFF_GOVERNANCE_PROFILEoverride shaped likeSECOND_PROFILE,faff corrective author --jsonstill writes its artifact and reportsevent_appended: false, andevents.jsonlgains nocorrective-authoredrow. The retained in-minteventViolationscall still refuses the record against the override's type list, as it does atHEAD.Verified: SECOND_PROFILE-shaped override: artifact written, event_appended false, no row.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::faff corrective author under a SECOND_PROFILE-shaped override writes its artifact but appends no eventfaff-1353#done:16Using apluginCopywhoserun-event.schema.jsonaddsprobe_requiredto theissue-outcomemapping'sdata.required, run the copy'sfaff run-ledger record-outcome --issue FAFF‑1 --outcome shipped --run-dir <R>against a run minted withrun-ledger init-interactive --issue FAFF‑1. The core refuses theissue-outcomeappend,run-ledger.jsonholds the new outcome, the stderr line names the refusal and endsthe ledger IS written, the verb exits 0 as it does on success, andevents.jsonlgains noissue-outcomerow.Verified: record-outcome refusal: ledger written, stderr ends 'the ledger IS written', exit 0, no issue-outcome row.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::record-outcome: a core refusal leaves the ledger written, exits 0 and adds no issue-outcome rowfaff-1353#done:17Using apluginCopywhoserun-event.schema.jsonaddsprobe_requiredto theevidence-terminal-onlymapping'sdata.required, run the copy'sfaff evidence write review-verdict --terminal-only --run-dir <R> --issue ISSUE‑1with{"signal":"pass","findings":[]}on stdin, from a fixture built liketest/evidence-write.test.mjs's. The verb exits 2, stderr carriescannot append the evidence-terminal-only event; nothing written, noreview-verdict.jsonis written, andevents.jsonlis byte-identical before and after.Verified: terminal-only refusal: exit 2, named stderr, no verdict, events.jsonl byte-identical.
Method: test
test/faff-1353-write-core-issue-scoped.test.mjs::evidence write review-verdict --terminal-only: a core refusal exits 2, writes no verdict and leaves events.jsonl untouchedfaff-1353#done:18In a scratch git repo withcapture.decision_kernel: onand a genesisrun-startminted viafaff events append, runFAFF_RUN_DIR=<abs run dir> faff next --status todo --spec highwith neitherFAFF_DECISION_CORRELATION_IDnorFAFF_DECISION_ISSUEexported.faff nextexits 0 and prints its verdict,events.jsonlgains nodecision-capturerow, andfaff events validate --file <that events.jsonl>exits 0.Verified: faff next under FAFF_RUN_DIR with no correlation id or issue exits 0, no decision-capture row, validate and verify exit 0.
Method: test
test/faff-989-decision-capture-driver.test.mjs::FAFF‑1353: a kernel run with no correlation id and no issue writes no decision-capture row, and the events file still validatesCloses FAFF-1353