Skip to content

trigger.recordId is never populated on record_change runs, and the persisted sys_automation_run row carries no trigger block at all — runs cannot be correlated to their triggering record, and trigger kinds are lost across restart #7533

Closed

Description

@huangyiirene

Symptom

Two related gaps in what an automation run records about why it ran.

1. trigger.recordId is never populated on record_change runs. A flow fired by a record change produces a run whose trigger block has no recordId, so there is no way to correlate a run back to the record that triggered it from the run log. Given a suspicious run you cannot answer "which record caused this"; given a suspicious record you cannot find the runs it provoked. For the single most common trigger kind on the platform, the run log is not usable as an audit trail.

2. The persisted sys_automation_run history row carries no trigger block at all. In-memory runs do record their runtime trigger kind — that part works and was verified for all six kinds. But the persisted history row has no trigger information whatsoever, so the trigger kind is lost across a process restart. After a restart the run history can tell you a flow ran and whether it succeeded, but not what fired it: a scheduled run, a webhook intake, and a record change become indistinguishable rows.

These compound. The runs that most need attribution (record changes) lose their record id immediately, and every run loses its kind as soon as the process cycles — which for a long-lived deployment means the durable history is the less informative copy.

Everything else in the trigger surface is solid and was verified in the same run: all 6 trigger kinds fire and record their runtime kind; webhook HMAC intake answers 202 with payload interpolation, and both bad-signature and missing-signature are refused 401 with no task created; anonymous /trigger is denied 401 with no run row; the schedule fired 12 consecutive runs exactly 60s apart. The mechanism is right; the record of it is incomplete.

Root cause

Not root-caused to a line in this run. The behavioural split is informative though: the in-memory run object does carry the runtime trigger kind while the persisted row carries none, which points at the persistence mapping rather than at trigger dispatch — the information exists at the moment of the run and is dropped on the way to sys_automation_run.

recordId is a separate omission: it is absent even in-memory on record_change runs, so that one is upstream, where the record-change trigger builds the run's trigger context.

Both live in the flow engine / automation service — packages/services/service-automation.

Reproduction

  1. Boot the showcase on an isolated file DB and port.
  2. Trigger a record_change flow by writing a matching record.
  3. Read the run from the automation run log and inspect its trigger block → no recordId. There is no field correlating the run to the record just written.
  4. Read the persisted sys_automation_run row for the same run → no trigger block at all.
  5. Cold-restart the process against the same file DB.
  6. Re-read the run history → the runs are still there, but nothing records what kind of trigger fired each one. Repeat across the other trigger kinds (schedule, webhook, manual) — all indistinguishable after the restart.

Source

Extracted from the QA run #7516 (framework a86db17). Checklist item trigger-type-matrix (PARTIAL) — these are the two gaps that held it back from a pass.

Activity

  1. added
    bugSomething isn't working
    pm:queueReady for the PM dispatch loop
    domain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landed
    on Aug 11, 2026
  2. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 11, 2026
  3. self-assigned this
    on Aug 11, 2026
  4. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    Claim: PM loop, services lane, wave 2 round 1
    Session: session_015fkdTyGmMD5s8ZtEifvuGy
    Branch: claude/issue-7533-run-trigger-attribution
    Worktree: objectstack-issue-7533
    Domain: domain:services
    File surface: packages/services/service-automation/src/ (the record-change trigger's run-context builder for gap 1; the sys_automation_run persistence mapping for gap 2) + tests in the same package. Stop on breach; if the persisted-row shape turns out to require a packages/spec or packages/platform-objects schema change (a new column / trigger block on sys_automation_run), STOP and report — that is the shared-contract single-owner boundary, not this claim.
    Container & model: M (two related gaps, persistence-shape judgment involved), mode:subagent, model: opus — kept in-container rather than cloud despite M sizing because the verification radius is one package and the maintainer has asked for throughput this wave; flagged here for auditability.
    Serial constraints cleared: no open PR or in-flight claim touches service-automation — #7446 (wait-node, same package) merged 08-10 and is in the base. Batch-mates #7243 (service-datasource) and #7278 (plugin-approvals) are file-disjoint.


    Generated by Claude Code

  5. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    ACCEPT — PR #7595, reviewed against GitHub rather than the report's own claims. Both gaps land; Fixes is correct.

    The dispatch's boundary assumption was falsified, and the falsification is verified. The dispatch set a hard stop if gap 2 needed a packages/platform-objects / packages/spec schema change. The dev measured — and this seat re-measured independently on origin/main — that sys_automation_run's object definition lives inside this package (service-automation/src/sys-automation-run.object.ts, registered from its own plugin.ts); outside it the name appears only in platform-object-names.ts, the conversions registry, and prose. Precedent columns (selected_count, #4354) live only in-package too. So the three new columns are package-local, inside the claimed surface, and no cross-seat boundary was touched. A dev disproving the PM's mechanism assumption with measurement is a good run — endorsed here in the open.

    What shipped, verified in the diff (6 files, all in-package + changeset):

    • Gap 1 at a chokepoint, not a patch. ExecutionLogEntry.trigger was spelled as an object literal at every logging site, and every copy omitted recordId. It is now built by one function (buildRunTrigger), which reads context.record.id, drops nullish/empty ids rather than storing blanks, and is used at every site — one place to omit the next field instead of many.
    • Gap 2 as columns with a query rationale. trigger_type / trigger_object / trigger_record_id plus an index on (trigger_object, trigger_record_id) — columns rather than a JSON blob because both target questions are queries (a filter and a group-by), the same Surface flow run summaries (selected / acted / skipped) — a scheduled flow that does nothing is currently indistinguishable from one with nothing to do #4354 reasoning already recorded in this object. Written on terminal rows and paused rows (a paused run is a sys_automation_run row too; omitting it would make reverse-correlation silently exclude in-flight runs).
    • Tests read the persisted cell. The store suite asserts engine.rows.get(...) directly — a mapper that reconstructed the value without writing the column would pass a round-trip and fail these. Content pins on the exact record id; two runs distinguished by their causing records; the six-kind restart matrix (new Set(seen).size === 6 — the card's exact symptom was six empty strings); NULL-not-empty-string; legacy rows rehydrate as "not recorded".
    • Reverse verification with an honestly reported deviation: engine-only revert reds exactly the 7 engine pins with the store block green; store-only revert reds the 6 cell cases. The dev predicted engine restart tests would also red on the store revert and reports they did not (the in-memory store JSON-clones past the mapping) — which is the argument for the two-layer split, stated instead of smoothed over.
    • Compat: pre-existing rows carry no columns and rehydrate exactly as before ('' type — the value this path always returned); no backfill.
    • Consumer sweep with direction stated (prefix filter, 15 packages, the verify false-alarm diagnosed as an unbuilt-own-closure artifact and re-run correctly); adjacent producers/readers (trigger-record-change 56, plugin-approvals 452) green; check:engine-double-contract in sync by reusing the already-baselined fake engine instead of introducing a new double.

    CI at review time: in progress. Driving to landing: gate conclusions → ready → arm → queue-branch verify.


    Generated by Claude Code

  6. os-help commented on Aug 11, 2026

    @os-help
    Collaborator

    os-dev-report

    {
      "issue": 7533,
      "status": "done",
      "branch": "claude/issue-7533-run-trigger-attribution",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/7595",
      "premise_still_valid": true,
      "summary": "Both gaps implemented. Gap 1: the trigger block was an object literal copied at TEN recordLog sites (not the ~8 I first counted — the retry path has two more), every copy populating type/userId/object and omitting the long-declared `recordId`; replaced by one chokepoint `buildRunTrigger(context)` that reads the id off `context.record.id` (the declared carrier, per the record-change trigger's buildContext) and drops nullish/empty ids. Gap 2: `recordLog` copied only `trigger.userId` onto the RunRecord handed to the store and `runRecordToLogEntry` rehydrated a hardcoded `trigger: {type: ''}`; RunRecord now carries triggerType/triggerObject/triggerRecordId and `sys_automation_run` gains trigger_type/trigger_object/trigger_record_id as COLUMNS (indexed on (trigger_object, trigger_record_id)), written on terminal history AND live paused rows. THE PM's HARD BOUNDARY DID NOT BIND, and this is the one mechanism assumption I falsified: the `sys_automation_run` schema does NOT live in packages/spec or packages/platform-objects — it is defined in this very package at packages/services/service-automation/src/sys-automation-run.object.ts and registered from its own plugin.ts. Outside the package the object appears only as a NAME (one entry in spec's platform-object-names.ts, a name/label/managedBy triple in the conversions registry, prose). Tracing the precedent columns confirms it: selected_count (#4354) and node_type exist only in this package's object file, store and tests. So the column addition is package-local and inside the claimed file surface, and gap 1 needed no spec change either — ExecutionLogSchema has declared trigger.recordId/object/type from the start. Nothing touches packages/spec or packages/platform-objects, hence `Fixes #7533` rather than `Part of`. Chose columns over folding into the existing `context_json` because both questions the card names are QUERIES (filter by record, group-by kind), the same argument #4354 already recorded in this object for the count columns — and `context_json` is not even written on terminal rows.",
      "tests": "21 new cases; package suite 890 -> 911, all green. `pnpm --filter @objectstack/service-automation test`: 'Test Files 74 passed (74) / Tests 911 passed (911)'. Build closure `@objectstack/service-automation^...` green first; package build green ('DTS Build success in 4480ms') — that IS this package's typecheck, it has no typecheck script (ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT). New: src/run-trigger-attribution.test.ts (engine half — exact-record-id content pins on completed+failed runs, two runs distinguished by cause, six kinds still distinguishable after cold restart, the schedule regression pin the card asked for, absent-stays-absent, pre-change row still rehydrating). Extended src/suspended-run-store.test.ts with the persisted-CELL assertions per the card's clause — they read `engine.rows.get(...)`, not just loadTerminal, since the defect was information dropped on the way to the row; added there deliberately to reuse the already-baselined fake engine rather than mint a new double, so check:engine-double-contract stays in sync (OK — 148 pinned, 133 DEBT, 2 exempt). REVERSE VERIFICATION, direction predicted first, via `git checkout origin/main -- FILE` (never git stash): (1) revert engine.ts -> predicted recordId+restart pins red and store-cell tests GREEN (they feed RunRecord directly, mapping untouched); measured exactly that, '7 failed | 904 passed', all 7 in the new file. (2) revert suspended-run-store.ts -> '6 failed | 905 passed', all six store-cell cases. HONEST DEVIATION on (2): I predicted the engine restart tests would ALSO go red and they did not — InMemorySuspendedRunStore JSON-clones the whole RunRecord and never touches the column mapping, so engine-level tests structurally cannot see a broken mapping. That argues FOR the two-layer split rather than against it. Three cases stayed green under both reverts by design (the absence-asserting ones). Gates: check:nul-bytes OK (7016 files), check:changeset-fixed OK, check:engine-double-contract OK, check:docs-audit-scope OK. Downstream sweep used the PREFIX filter `...@objectstack/service-automation` (15 consumer packages) build: green — packages/verify first failed TS2307 on @objectstack/runtime/rest/plugin-auth and four more, none touched here; that was the consumer filter not building verify's OWN deps, and `@objectstack/verify...` (its own closure) is clean. No package outside this one references RunRecord or SysAutomationRun at all. Adjacent: trigger-record-change 56 passed (producer of the context recordId is read from), plugin-approvals 452 passed (reads this store). CI at report time: honestly IN PROGRESS — ESLint and TypeScript Type Check both `in_progress`, no conclusion yet; already-green: filter, Check PR Size, Console Pin Freshness, 'No other open PR may claim the same issue', Vercel Preview Comments. Reported at draft-PR time per dispatch instruction not to idle-poll CI, so the family gates inside ESLint are UNVERIFIED remotely.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Notes for the PM, none blocking:

    • skip-changeset does not apply — this PR ships a changeset (minor: new columns plus a newly-populated field), so the label would be wrong. Auto Label was still in_progress at report time; no label write of mine.
    • The dispatch's load-bearing question ("can the persisted row carry trigger info WITHOUT a schema change") resolved as: no schema change is needed outside this package, and the in-package object file is the right owner. If the spec seat would nonetheless prefer these three columns reviewed as a shared-contract change, that is a review call on the object file — not a blocker I hit.
    • Worktree removed after the push.

    Generated by Claude Code


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:servicesRetired 2026-10-22, alias of domain:server (services surface); in-flight cards keep it until landedpm:dispatchedDispatched to a dev agent by /pm-dispatch

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions