fix(OMN-19813): reject an empty-string agent_id on claude_hook_events / claude_agent_spans by jonahgabriel · Pull Request #2968 · OmniNode-ai/omnimarket

jonahgabriel · 2026-09-26T20:03:54Z

OMN-19813

Hostile-review follow-up on omnibase_infra#4169. ck_claude_hook_events_subagent_flag CHECK (is_subagent = (agent_id IS NOT NULL)), and the Python validator that mirrors it, both read an empty string the same as any other non-null value: agent_id = '' with is_subagent = true passes both today and would be stored as a subagent row with no real id behind it.

Landing order — this PR must land AFTER omnimarket#2956 (which creates the two tables) and after omnimarket#2958 (ownership). It is not pushed to #2956, #2958 or omnibase_infra#4169 — those are in a fixed landing order a drain lane is running, and this branch is cut from #2956's tip rather than from dev HEAD for exactly that reason (the tables and models this PR touches do not exist on dev yet). Until #2956 merges, this diff will show #2956's and #2958's commits too; that is expected for a stacked PR in this repo, not a merge conflict to resolve.

What changed

min_length=1 on the three node_projection_claude_hook_events models that carry agent_id: ModelClaudeHookLineageWire (the wire boundary), ModelClaudeHookEventRow (what the writer inserts), ModelKnownParentToolCall (the writer's own parent lookup). parent_agent_id fields are out of scope — same class of risk, not part of this finding.
migrations/0002_reject_empty_agent_id.sql: a CHECK on both claude_hook_events and claude_agent_spans rejecting agent_id = '', plus a named post-condition assertion for the pre-existing ck_claude_hook_events_subagent_flag CHECK, which 0000 declares but never asserts exists (a second, below-quorum note from the same hostile review). Idempotent via DROP CONSTRAINT IF EXISTS + ADD, since an anonymous DO block guard is exactly the dynamic SQL the OMN-15361 application-database gate rejects for this schema.
Two new unit tests pinning the refusal at the model layer.

Proof

Local: uv run pytest tests/test_omn19513_claude_hook_events_projection.py -q (56 passed, including the 2 new), uv run ruff format --check / ruff check clean, uv run mypy --strict clean on the changed files.

Lab: proved on the .201 dev lane Postgres (omnibase-infra-postgres, db omnibase_infra) inside BEGIN ... ROLLBACK:

0000 + 0001 + 0002 apply clean; 0002 re-applied a second time in the same transaction to prove the drop/re-add idempotency.
Positive control: a real non-empty agent_id insert succeeds on both tables.
Negative control: an empty-string agent_id insert is refused on both tables with the new CHECK (check_violation on ck_claude_hook_events_agent_id_not_empty / ck_claude_agent_spans_agent_id_not_empty).
After ROLLBACK, to_regclass on both tables returns nothing — nothing was persisted to the lab database.

[x] Title contains OMN-XXXX
[x] Body cites OMN-XXXX ticket with dod_evidence (OMN-19813, AC1/AC2 above; probe pytest command in the ticket)
[x] uv run ruff format src/ tests/ && uv run ruff check --fix src/ tests/
[x] Focused local tests for changed behavior (tests/test_omn19513_claude_hook_events_projection.py, 56 passed)
[ ] pre-commit run --all-files — the local pre-commit hook chain ran and passed at commit time (governed pre-push in this repo is mypy --strict only, per this repo's CLAUDE.md OMN-18162)

Evidence-Ticket: OMN-19813
Evidence-Source: OCC#11635

…ode hook, with subagent lineage The consumer half of the all-hooks capture contract (omniclaude#2370). One subscribe topic, onex.evt.omniclaude.hook-event.v1, carries all 33 Claude Code hook types with lineage on every event. This node projects it into omninode_internal.claude_hook_events (one row per event, session_id as a real column, agent_id NULL for the main thread) and folds subagent lineage into omninode_internal.claude_agent_spans (parent_agent_id plus parent_resolution: main_thread, resolved, workflow or unknown). Rule 7a pair: HandlerProjectionClaudeHookEvents is the pure definition-B fold; ClaudeHookEventsProjectionWriter is the in-process writer that reads the spawning PreToolUse, calls the fold and persists. Idempotent by construction: event rows insert ON CONFLICT (event_id) DO NOTHING, the span tool_call_count is recounted from the event table rather than incremented, and a known parent replaces unknown but never the reverse. A child written before its spawning call is repaired when the call arrives. Only the writer is a routed entry. The contract's event has a top-level payload mapping and an event_id, which the runtime's typed arm reads as a transport envelope and unwraps through (OMN-16050); a routed fold would receive the per-hook payload without its lineage and dead-letter every event. The wire model refuses a payload key the capture contract does not declare for the metadata topic, so content that reaches this topic by a producer regression is dead-lettered, not stored. It drops, by name, the five top-level stamps the emit seam adds (measured on a live lab record). Tests reproduce the contract's expected_projection rows for the nested subagent, workflow-agent and orphan-agent scenarios through the writer's runtime entry, pin the hook list, payload keys and projection columns to the contract, and prove the SQL against real Postgres (skips without a DB).

…ps by name The wire model for onex.evt.omniclaude.hook-event.v1 forbids extra top-level keys and drops transport stamps by name. It previously dropped only five stamps: correlation_id, causation_id, entity_id, session_id, and redaction_state. The real appender (omniclaude hook_emit_append.py) also merges lane, lane_source, lane_ticket, workspace_path, and turn_id into every record. Because these five keys were not in the drop set, a fixture event journaled through the real appender was rejected with extra_forbidden, so every real hook event would have been dead-lettered. The fix adds the five appender keys to the named stamp set so they are dropped instead of rejected. The event's lineage.turn_id remains authoritative over the appender's top-level turn_id. The seam test now carries the appender stamps with a different top-level turn_id and asserts the stored turn_id is the lineage one. A new test proves an unnamed top-level key such as prompt is still refused.

…nership ahead of their vendoring Step 1 of the three-PR order scripts/application-relation-ownership.yaml documents, as omnimarket#2937 did for session_content. omnibase_infra#4169 vendors the node_projection_claude_hook_events migrations and its Application Database Domain Enforcement gate resolves ownership from this file at omnimarket dev tip, so the declaration has to land first; omnimarket#2956 then lands the node and its migration. No SQL, no exemption. The relation inventory is regenerated: source_declared_tables 81 to 83, source_created_tables unchanged at 72.

… stamp by name The omniclaude emit drainer stamps hook_fired_at on every published record, so every real drained hook event failed validation and was dead-lettered. This adds hook_fired_at to TRANSPORT_STAMP_KEYS, so the wire model drops it by name while still refusing unknown top-level keys. Tests carry the new key and assert it is accepted while an unknown key is still rejected. Onex-Lane: all-hooks-contract-83 Onex-Session: 9a8f8ac2b564447cbcbd3142788e28a1

…ook-events-ownership' into jonah/omn-19513-claude-hook-events-projection

…osters CI on the first push failed three roster tests this node moves. ClaudeHookEventsProjectionWriter joins the pinned exact set of in-process-dispatch writers as the eighth, on the same reviewed terms (its pure fold does not declare the capability). The relation inventory is regenerated after merging the omnimarket#2958 ownership declaration: source_created_tables 72 to 74 and the arithmetic unreconciled bound 14 to 12, declared unchanged here. The dogfood inventory counts node_dirs 425 to 426 and entry_points 416 to 417.

…ts-projection' into jonah/omn-19513-claude-hook-events-projection

… sequences Two changes carried here so this node package stays byte-equal to what omnibase_infra#4169 vendors and to what omnimarket#2958 declares: - 0001 now asserts the omninode_runtime USAGE grant on schema omninode_internal with has_schema_privilege, answering the Hostile Reviewer MAJOR finding on omnibase_infra#4169 that it was the one issued privilege left unasserted. The grant set itself is unchanged: it is already exactly the writer scope. The file is byte-identical to the vendored copy on omnibase_infra#4169 (cmp). - scripts/application-relation-ownership.yaml gains the two database_objects sequence entries, identical to omnimarket#2958, because the OMN-15361 SQL gate refused both sequences by name with only the tables declared. The ownership test from #2958 is carried too, so the hunks are identical and merge cleanly in either order. Lab (.201 dev lane, omnibase-infra-postgres, database omnidash_analytics): both files applied inside BEGIN ... ROLLBACK with ON_ERROR_STOP=1, clean; with the schema USAGE revoked inside a rolled-back transaction the new assertion raised division by zero. Post-state unchanged (USAGE true, 21 event rows, 4 span rows).

The is_subagent = (agent_id IS NOT NULL) CHECK on claude_hook_events, and the Python lineage validator that mirrors it, both read an empty string the same as any other non-null value. An agent_id of '' would pass both and be stored as a subagent row: is_subagent=true with no real id behind it, indistinguishable from the main thread's events except by an empty string. - min_length=1 on the three node_projection_claude_hook_events models that carry agent_id (ModelClaudeHookLineageWire, the wire boundary; ModelClaudeHookEventRow, what the writer inserts; ModelKnownParentToolCall, the writer's own parent lookup). parent_agent_id is out of scope: same class of risk, not part of this finding. - migrations/0002_reject_empty_agent_id.sql: a CHECK on both claude_hook_events and claude_agent_spans rejecting agent_id = '', plus a named post-condition assertion for the pre-existing ck_claude_hook_events_subagent_flag CHECK from 0000, which 0000 declares but never asserts exists (the hostile-reviewer non-blocking note on omnibase_infra#4169). Idempotent via DROP CONSTRAINT IF EXISTS + ADD, since a DO block guard is exactly the dynamic SQL the OMN-15361 gate rejects for this schema. - two new unit tests: the wire model refuses '' with is_subagent=true and nothing is stored; ModelKnownParentToolCall refuses '' too. Proved on the .201 dev lane Postgres inside BEGIN/ROLLBACK: 0000+0001+0002 apply clean (0002 re-applied a second time in the same transaction to prove idempotency), a real non-empty agent_id insert succeeds on both tables (positive control), an empty-string agent_id is refused on both tables with the new CHECK (negative control), and after ROLLBACK neither table exists -- nothing persisted to the lab database. Base branch is dev; this PR must land after omnimarket#2956 (which creates the tables this migration alters) and after omnimarket#2958 (ownership). Not pushed to #2956, #2958 or omnibase_infra#4169 -- those are in a fixed landing order a drain lane is running. OMN-19513

github-actions · 2026-09-26T20:04:15Z

✅ Architectural Review — PASSED

Errors: 0
Warnings: 0

What this checks

Rule	Description
ARCH-TOPIC-001	No hardcoded Kafka topic strings in handler code
ARCH-DI-001	No `event_bus=None` bypass
ARCH-DI-002	No direct Handler instantiation outside workflow_runner/adapters
ARCH-DI-003	No reinvented DI containers
ARCH-TOPIC-002	contract.yaml topics follow `onex.{cmd

No architectural violations found.

Static architectural lint — no model inference (OMN-14176).

…nimarket#2968 The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited. Ticket: OMN-16859 (AC3b) Onex-Workflow: OCC Receipt Runner Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/36268208058

github-actions · 2026-09-26T20:07:55Z

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 9
Models succeeded: qwen3-review,gpt-oss-review

Gate semantics

Verdict	Meaning	Blocks merge?
`passed`	>=2 models succeeded, no critical/error findings	No
`blocked`	CRITICAL or ERROR findings found	Yes
`degraded`	Fewer than 2 models succeeded (infra unavailable/timeout)	Yes (OMN-15110)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

jonahgabriel · 2026-09-26T20:53:24Z

Independent re-verification of the lab proof (lane empty-agent-id-83, 2026-09-26T20:xx Z), since the PR-body edit path is gated here and cannot be used for a correction:

Correction: the tables live in omnidash_analytics on omnibase-infra-postgres, not database omnibase_infra as the Proof section states. select to_regclass('omninode_internal.claude_hook_events') returns null in omnibase_infra and returns the relation in omnidash_analytics. Re-ran the full proof there.
migrations/0002_reject_empty_agent_id.sql applied clean inside BEGIN...ROLLBACK against the real tables. All three named post-condition assertions passed (ck_claude_hook_events_subagent_flag_exists_assertion, ck_claude_hook_events_agent_id_not_empty_exists_assertion, ck_claude_agent_spans_agent_id_not_empty_exists_assertion), and pg_constraint.convalidated = t for all three.
Positive control: insert with agent_id='proof-omn19813-agent' succeeded on both claude_hook_events and claude_agent_spans.
Negative control: insert with agent_id='' on both tables raised check_violation (ck_claude_hook_events_agent_id_not_empty / ck_claude_agent_spans_agent_id_not_empty).
Re-applied the migration file a second time in the same transaction: clean idempotent DROP CONSTRAINT IF EXISTS + ADD, no error.
After ROLLBACK: select count(*) ... where session_id='proof-omn19813-session' = 0 on both tables, and the new constraint name has 0 rows in pg_constraint — nothing this proof added persisted. The tables themselves (created by the earlier docs(OMN-17427): remove 33 unreferenced docs files after archiving them #4169 lab pass) are untouched, as expected: this migration only adds constraints to pre-existing tables, it does not create or drop them.

No writes to dev, no touches to #2956/#2958/omnibase_infra#4169, no merge/arm/CI-rerun. Scratch files removed from the .201 container and host after the run.

Resolved three census conflicts by adding this PR's increments on top of dev's figures after OMN-19716 landed. Dogfood node_dirs moves from 427 to 428 and entry_points from 418 to 419. The relation inventory's source_created_tables moves from 73 to 75 for claude_hook_events and claude_agent_spans. source_declared_tables stays at dev's 84 because the declarations landed with omnimarket#2958, and minimum_unreconciled_live_base_tables drops from 13 to 11. The inventory JSON was regenerated with the generator's --write mode, and 120 focused tests pass with 2 skipped (real Postgres). Onex-Lane: all-hooks-contract-83 Onex-Session: 9a8f8ac2b564447cbcbd3142788e28a1

…imarket#2968 (#11522) * evidence(OMN-19813): author OCC companion for OmniNode-ai/omnimarket#2968 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 5dc118ed99a2c41d3bd7c28e844e51664d05e3e7. * evidence(OMN-19813): self-bind OCC#11522 + rebind contract_sha256 * evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#2968 The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited. Ticket: OMN-16859 (AC3b) Onex-Workflow: OCC Receipt Runner Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/36268208058 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai> Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com> Co-authored-by: Jonah Gray <jonah@omninode.ai>

…evmerge # Conflicts: # tests/test_market_node_runtime_dogfood.py

…ts-projection' into jonah/omn-19513-agent-id-empty-check

…id-empty-check-84h # Conflicts: # docs/evidence/OMN-15423-relation-inventory.json # src/omnimarket/nodes/node_projection_claude_hook_events/models/model_claude_hook_event_wire.py # src/omnimarket/nodes/node_projection_claude_hook_events/models/model_claude_hook_projection_request.py # src/omnimarket/nodes/node_projection_claude_hook_events/models/model_claude_hook_projection_result.py # tests/test_market_node_runtime_dogfood.py # tests/test_omn16874_consumer_flow_writer_dispatch.py # tests/test_omn19513_claude_hook_events_projection.py # tests/unit/scripts/test_application_relation_inventory.py

onexbot-occ-writer · 2026-09-29T00:04:12Z

OCC autobind rebound this PR's evidence-source stamp line to OCC#11635.

The receipt gate's own eligibility validator, run against the change-control tree the gate pins for OCC#11635, returned eligible for head 00ac625057efa7ea5a473b1a4cfe868162bf921d. The body now carries exactly one stamp line. Displaced: OCC#11522.

Reported by occ_companion_emitter (OMN-18853).

…nibase_infra copy The vendored copy landed in omnibase_infra dev first (#4209) with a comment that reads 'not yet merged to dev'. The node-migration-vendor-parity gate requires identical bytes, so adopt the vendored comment text. This commit also carries OMN-19813 in its message so the identity-binding gates find the Evidence-Ticket.

jonahgabriel · 2026-09-29T02:06:48Z

PR #2968 is closed as superseded by #3071 because the branch name contained omn-19513 while the evidence ticket was OMN-19813, which failed the lint check at head 20d4ea9. The same net diff involving 5 files and 56 passing tests was re-homed onto the new branch jonah/omn-19813-reject-empty-agent-id cut from dev 29dc6ae. No work was lost in this transition, and the landing order parents omnimarket#2956 and omnimarket#2958 are already merged.

jonahgabriel added 9 commits September 26, 2026 09:47

Merge remote-tracking branch 'origin/jonah/omn-19513-declare-claude-h…

5655f31

…ook-events-ownership' into jonah/omn-19513-claude-hook-events-projection

Merge remote-tracking branch 'origin/jonah/omn-19513-claude-hook-even…

3467e4f

…ts-projection' into jonah/omn-19513-claude-hook-events-projection

onexbot-occ-writer Bot mentioned this pull request Sep 26, 2026

evidence(OMN-19813): OCC Evidence-Source autobind for OmniNode-ai/omnimarket#2968 OmniNode-ai/onex_change_control#11522

Merged

jonahgabriel enabled auto-merge (squash) September 26, 2026 20:06

github-advanced-security Bot found potential problems Sep 26, 2026

View reviewed changes

Comment thread tests/test_omn19513_claude_hook_events_real_postgres.py Fixed

jonahgabriel added 2 commits September 27, 2026 08:21

Merge remote-tracking branch 'origin/dev' into drain/omn-19513-2956-d…

63121ee

…evmerge # Conflicts: # tests/test_market_node_runtime_dogfood.py

Merge remote-tracking branch 'origin/jonah/omn-19513-claude-hook-even…

a59af91

…ts-projection' into jonah/omn-19513-agent-id-empty-check

jonahgabriel mentioned this pull request Sep 27, 2026

feat(OMN-19888): scheduled heals update release-window reds and re-run vendor-parity reds once their cause is gone #3026

Merged

onexbot-occ-writer Bot mentioned this pull request Sep 28, 2026

evidence(OMN-17943): backfill diff-derived behavior proofs onto pre-minter contracts OmniNode-ai/onex_change_control#11713

Merged

jonahgabriel closed this Sep 29, 2026

auto-merge was automatically disabled September 29, 2026 02:06
Pull request was closed

Navigation Menu

Repository navigation

fix(OMN-19813): reject an empty-string agent_id on claude_hook_events / claude_agent_spans - #2968

fix(OMN-19813): reject an empty-string agent_id on claude_hook_events / claude_agent_spans#2968
jonahgabriel wants to merge 14 commits into
devfrom
jonah/omn-19513-agent-id-empty-check

jonahgabriel commented Sep 26, 2026 •

edited by onexbot-occ-writer Bot

Loading

Uh oh!

github-actions Bot commented Sep 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

jonahgabriel commented Sep 26, 2026

Uh oh!

onexbot-occ-writer Bot commented Sep 29, 2026

Uh oh!

jonahgabriel commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jonahgabriel commented Sep 26, 2026 • edited by onexbot-occ-writer Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

OMN-19813

What changed

Proof

Uh oh!

github-actions Bot commented Sep 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Architectural Review — PASSED

What this checks

Uh oh!

github-actions Bot commented Sep 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Hostile Reviewer — PASSED

Uh oh!

Uh oh!

jonahgabriel commented Sep 26, 2026

Uh oh!

onexbot-occ-writer Bot commented Sep 29, 2026

Uh oh!

jonahgabriel commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahgabriel commented Sep 26, 2026 •

edited by onexbot-occ-writer Bot

Loading

github-actions Bot commented Sep 26, 2026 •

edited

Loading

github-actions Bot commented Sep 26, 2026 •

edited

Loading