Skip to content

fix(service-automation): populate trigger.recordId and persist run trigger attribution to sys_automation_run - #7595

Merged
os-help merged 1 commit into
mainfrom
claude/issue-7533-run-trigger-attribution
Aug 11, 2026
Merged

fix(service-automation): populate trigger.recordId and persist run trigger attribution to sys_automation_run#7595
os-help merged 1 commit into
mainfrom
claude/issue-7533-run-trigger-attribution

Conversation

@os-help

Copy link
Copy Markdown
Collaborator

Fixes#7533

Both gaps land, so this closes the card. See "Boundary" below — the schema half turned out not to be the shared-contract surface the dispatch expected.

What was broken

Gap 1 — trigger.recordId was never populated on record_change runs. The field is declared in ExecutionLogSchema and was written by nothing. The trigger block was spelled as an object literal at ten separate places a run gets logged (three on execute, three on resume, two on the retry path, one on failSuspendedRun, one on cancelRun), and every one of them populated type / userId / object and omitted recordId. For the platform's most common trigger kind the run log could not answer "which record caused this run?", nor its reverse.

Gap 2 — the persisted sys_automation_run row carried no trigger block at all. The in-memory run recorded its runtime kind; recordLog copied only trigger.userId onto the RunRecord handed to the store, and runRecordToLogEntry rehydrated a hardcoded trigger: { type: '' }. After a restart a scheduled run, a webhook intake and a record change were indistinguishable rows — the durable copy of the history was strictly less informative than the volatile one.

What changed

  • One chokepoint, not ten literals.buildRunTrigger(context) is now the single construction point for ExecutionLogEntry.trigger, and it populates recordId. Ten literals is ten chances to omit the next field too; one function is one. recordId is read off context.record.idrecord is the declared carrier of the triggering row (populated by the record-change trigger's buildContext) and id is the platform primary key. Nullish and empty-string ids are dropped rather than stored, mirroring expandDeclaredLookups's guard on the same read.
  • RunRecord carries the trigger block (triggerType / triggerObject / triggerRecordId), flattened the way userId — itself a trigger field — already was.
  • sys_automation_run gains three COLUMNS: trigger_type, trigger_object, trigger_record_id, plus an index on (trigger_object, trigger_record_id). Columns rather than a JSON blob for the reason Surface flow run summaries (selected / acted / skipped) — a scheduled flow that does nothing is currently indistinguishable from one with nothing to do #4354 already recorded in this object for the count fields: both questions here are queries, not readings of one row. "Which runs did this record provoke?" is a filter; "was last night's failure storm scheduled or record-driven?" is a group-by. context_json could not serve either — it is not even written on terminal history rows.
  • Written on terminal history rows and live paused rows. A paused row is a sys_automation_run row too; leaving it null would make the reverse-correlation query answer for finished runs while silently omitting in-flight ones — a partial answer that reads as a complete one.

Rows written before this change carry no trigger columns and rehydrate exactly as they did, with an empty trigger type. Absent means "not recorded", never "no trigger" — no backfill is attempted.

Boundary: the schema half was NOT the spec seat's surface

The dispatch set a hard stop if gap 2 required adding a field to sys_automation_run's declared schema in packages/platform-objects or packages/spec. Measured on origin/main, it requires neither — the object is defined inside this package, at packages/services/service-automation/src/sys-automation-run.object.ts, and registered from its own plugin.ts. Outside the package sys_automation_run appears only as a name: one entry in platform-object-names.ts, a name/label/managedBy triple in the conversions registry, and prose. Tracing the precedent columns confirms the shape — selected_count (#4354) and node_type live only in this package's object file, store and tests. So the column addition is package-local and inside the claimed file surface, and trigger.recordId needed no spec change at all: ExecutionLogSchema has declared recordId, object and type from the start. Nothing here touches packages/spec or packages/platform-objects.

Tests

21 new cases; the package suite goes 890 to 911, all green.

  • src/run-trigger-attribution.test.ts (new) — the engine half. Content pins on the exact record id (not "some id"), on completed and failed runs; two runs of one flow distinguished by their causing records; the six trigger kinds still distinguishable after a cold restart; the schedule regression pin the card asked for; absent-stays-absent for record-less kinds; a pre-change history row still rehydrating.
  • src/suspended-run-store.test.ts (extended) — the persisted cell. These read engine.rows.get(...) rather than only what loadTerminal returns, because the defect was information dropped on the way to the row: a mapper that never wrote the column but reconstructed the value would pass the round-trip and fail the cell. Includes the reverse-correlation read driven through the fake engine's own find, and NULL-not-empty-string for record-less kinds.

They were added to the existing store suite deliberately, reusing its already-baselined fake engine rather than introducing a new double — check:engine-double-contract stays in sync at 148 pinned / 133 DEBT / 2 exempt.

Reverse verification

Direction predicted before running, two reverts, both via git checkout origin/main -- FILE (never git stash — shared stack):

  1. Revert engine.ts only → predicted the recordId and restart pins red, store-cell tests green (they feed RunRecord directly; the mapping is untouched). Measured: exactly 7 red, all in the new file, store block green.
  2. Revert suspended-run-store.ts only → 6 red, all six store-cell cases.

One honest deviation from prediction 2: I expected the engine-level restart tests to go red too, and they did not. InMemorySuspendedRunStore JSON-clones the whole RunRecord and never touches the column mapping, so the engine restart tests cannot see a broken mapping. That is the reason the persisted-cell layer exists as separate coverage rather than being folded into the engine tests — the deviation argues for the two-layer split, it does not undermine it.

Three cases stayed green under both reverts by design — the ones asserting absence (record-less kind, empty-string id, legacy row), which was already true before the change.

Verification

  • Build closure @objectstack/service-automation^... then the package build (DTS emit is this package's typecheck — it has no typecheck script): green.
  • pnpm --filter @objectstack/service-automation test: 911 passed (74 files).
  • Downstream consumer sweep, prefix filter ...@objectstack/service-automation (15 consumer packages) build: green. packages/verify first failed on TS2307 for @objectstack/runtime, rest, plugin-auth and four others — none of them touched here; it was the consumer filter not building verify's own dependencies. Rebuilt as @objectstack/verify... (its own closure): clean. No package outside this one references RunRecord or SysAutomationRun at all.
  • Adjacent packages: trigger-record-change 56 passed (it produces the context recordId is read from), plugin-approvals 452 passed (it reads this store).
  • Gates: check:nul-bytes OK (7016 files), check:changeset-fixed OK, check:engine-double-contract OK, check:docs-audit-scope OK.

Changeset added (minor, user-visible: new columns and a newly-populated field).


Generated by Claude Code

…igger attribution (#7533)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
@vercel

vercelBot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 11, 2026 6:45am

Request Review

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-automation.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx(via @objectstack/service-automation)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/service-automation)
  • content/docs/plugins/packages.mdx(via @objectstack/service-automation)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/service-automation)
  • content/docs/releases/v9.mdx(via @objectstack/service-automation)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-help@claude