fix(scheduler): record who initiated an execution (#1970)

[dolho]

Problem

schedule_executions has carried five execution-origin columns for audit since AUDIT-001, and src/backend/database.py populates them on every path the backend owns. The scheduler is a separate service with its own DB module, and its create_execution() listed none of them:

INSERT INTO schedule_executions (
    id, schedule_id, agent_name, status, started_at, message, triggered_by,
    model_used, attempt_number, retry_of_execution_id
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)

Nor did the signature accept them, so there was nowhere to put a caller even if one had been forwarded. Every scheduler-created row was written with all five NULL — observed across 100+ runs of an actively used schedule.

triggered_by='manual' recorded that a human ran something and never who. The attribution existed only in backend / MCP-server logs, bounded by log retention, so past a few weeks the durable record could not answer "did anyone trigger this run, and who?".

Not a vulnerability — nothing authorizes on these columns.

The identity was dropped at three points, not one

#	Where	What was missing
1	routers/schedules.py::trigger_schedule	The delegating POST to the scheduler sent no body at all. The authenticated caller was in scope right there and never crossed the hop.
2	scheduler/main.py::_trigger_handler	No parameter to receive an identity.
3	scheduler/database.py::create_execution	Nowhere to put it.

Fixing only #3 — the line the issue points at — changes nothing observable, because nothing upstream would have had anything to pass.

Fix

An ExecutionOrigin value object threaded backend → _trigger_handler → _execute_manual_trigger → _execute_schedule_with_lock → create_execution.

One object rather than five parallel parameters at four call depths: five positional siblings is exactly how one of them silently stops being forwarded, which is the failure mode already on display here.

Cron ticks stay NULL. Attributing an autonomous fire to, say, the schedule's owner would make the column actively misleading — a blank reads as "unknown", a wrong name does not. Pinned by a test, because it is the half of the behaviour that must not change.

Two more paths the issue's fix would have left blank

Neither is mentioned in the issue, and both would have stayed anonymous after a literal reading of it:

• Retry (triggered_by='retry') inherits the original run's origin. A retry has no caller of its own, but it exists only because someone started the original; a chain that drops the initiator makes attempt 1 the only attributable attempt. The read is fail-open — an audit lookup must not be able to stop a retry from running.
• Reminder (triggered_by='reminder') inherits the provenance feat: agent self-reminders — agent-callable one-shot deferred self-trigger ("remind me to do X later") #1296 already persisted (owner_id, created_by_email, source_agent_name, source_mcp_key_id). This needed those four fields surfaced on the scheduler's Reminder model; the mapper guards them the way _row_to_execution guards its optional columns, so a table predating them doesn't take the fire path down.

Hardening the issue didn't ask for

Boundary validation of the trigger body. The scheduler's trigger endpoint takes an untrusted JSON body (as it already did for triggered_by). source_user_id is dropped rather than coerced when it is not an int — bool IS an int in Python, so True would have persisted as user 1: a real account attributed to a run it had nothing to do with. Strings are length-capped and blank-to-None, so "" and NULL are not two spellings of "unknown". A malformed body costs attribution, never the run.

Precedence flipped versus chat.py. The backend prefers the validated current_user.agent_name over the raw X-Source-Agent header:

"source_agent_name": current_user.agent_name or x_source_agent,

Header-first is fine for a collaboration hint; in an audit column it would let an agent-scoped caller pin its run on a sibling agent. The header now fills only the case where it is the sole signal (a user-scoped key, whose agent_name is None).

MCP third surface (Invariant #13). trigger_agent_schedule now forwards the same origin headers chat() already sends. Without it an MCP-triggered run attributes to the key owner but not to which key or which agent fired it — the part that identifies the actor when one human owns many of both. The generic extraHeaders hook this required explicitly refuses Authorization, so an attribution convenience can't become an auth-substitution surface.

Compatibility

Backward compatible in both rolling-deploy directions:

• old scheduler + new backend → the extra body fields are ignored, triggered_by still read as before;
• new scheduler + old backend → a bodyless POST parses to an empty origin, i.e. today's behaviour.

All five parameters are keyword-optional, so an un-migrated create_execution call site still inserts a valid row.

Verification

tests/unit/test_1970_execution_origin.py — 27 checks, 25 of which fail against the pre-fix tree:

$ git stash push -- src/ && pytest unit/test_1970_execution_origin.py -q
25 failed, 2 passed
$ git stash pop && pytest unit/test_1970_execution_origin.py -q
27 passed

The 2 that pass either way are deliberate: the cron-stays-NULL guard and the optional-parameter back-compat guard both assert behaviour the fix must preserve, so passing before and after is the correct result for them.

The DB assertions run against a real temp SQLite file whose table declares the five columns — a mock would have happily accepted the pre-fix signature, and a table without the columns would let a regressed INSERT pass by having nothing to fill.

Also green: tsc --noEmit on the MCP server, and the scheduler-adjacent suites (test_1808, test_schedule_status_observability, test_1945, test_1713 — 70 tests).

Acceptance criteria

• create_execution() accepts and writes all five source_* columns
• The backend forwards the authenticated caller across the scheduler hop
• _trigger_handler reads it and passes it down
• Cron-triggered runs keep passing NULL, which is correct for them

Sibling issues

#1968 (execution_id: undefined) and #1969 (lock-denied cron tick leaves no record) are untouched here. The issue notes both are "fixed most cleanly by creating the execution record synchronously in _trigger_handler()" — a restructure of the trigger path that this PR deliberately does not attempt, so the attribution fix can land without carrying that risk. The ExecutionOrigin object is already in scope at _trigger_handler, so it composes with that change rather than competing with it.

Related to #1970

🤖 Generated with Claude Code

Fixes #1970

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[dolho]

/review — self-review (PR #1974, #1970)

Branch: fix/1970-scheduler-execution-origin → dev · 9 files, +834/−23
Scope: CLEAN. The issue's suggested fix named 3 files; the diff touches 7 source files, all justified — the two extra non-cron paths (retry, reminder) and the MCP third surface (Invariant #13). No unrelated changes.
Plan completion: 4 AC DONE, 0 partial.

Self-review, so discount accordingly. Nothing blocking found.

[I1] The retry-origin read is a new DB call on the retry path (Confidence: 8/10)

original = self.db.get_execution(original_execution_id)

Trading "the retry is unattributed" for "the retry never runs" would be a bad deal, so this is wrapped fail-open with a WARNING. Worth naming because the first version I wrote had the if original guard but no try — a missing row was handled, a failing read was not. Those are different failures.

[I2] Attribution fields are caller-supplied at the scheduler boundary (Confidence: 7/10)

/api/schedules/{id}/trigger has no auth (platform network only), so a caller on that network could post a false source_user_id. Not a new exposure — triggered_by has been caller-supplied on this same endpoint all along — and nothing authorizes on these columns. Mitigated at the boundary: wrong types are dropped rather than coerced (bool IS an int in Python, so True would otherwise persist as real user 1) and strings are length-capped. Recording it explicitly so "audit column" is not later mistaken for "trustworthy identity".

[I3] The 255-char cap is not derived from the column (Confidence: 6/10)

source_user_email/source_mcp_key_name are TEXT — no DB constraint to match. 255 is a judgement call, not a derived bound. Fine, but it is a magic number.

Clean categories

• SQL safety — the INSERT stays qmark-parameterised; the _PgCursor ?→%s rewrite covers the 5 new placeholders.
• Race conditions — no new shared state; ExecutionOrigin is a frozen-in-practice value object passed by argument, never mutated.
• Auth boundaries — no new endpoint. The backend endpoint keeps AuthorizedAgent; the three X-* headers are attribution only, and precedence deliberately prefers the validated current_user.agent_name over the raw header (the reverse of chat.py) so a caller cannot pin its run on a sibling agent.
• Credential exposure — no values logged; the scheduler log line carries the user id, not the email, because it lands in Vector-captured platform logs.
• Enum completeness — no new enum/status values.
• Migrations — none needed; all five columns already exist (AUDIT-001).

Summary

• Critical: 0
• Informational: 3 — none blocking
• Scope: clean

CI: 18/18 green.

[dolho]

/edge-cases — 3 real bugs found across today's PRs (2 in this one)

Boundary-value analysis + Hypothesis over ExecutionOrigin.from_payload, the validation boundary for the scheduler's unauthenticated manual-trigger endpoint. Both bugs below were in code that already had 100% statement and branch coverage from test_1970_execution_origin.py — full coverage says every line ran, not that every value class was tried.

[B1] Range was never checked — a payload field could silently lose a run

from_payload dropped wrong-typed values and capped string length, but accepted any int. source_user_id lands in an INTEGER column, and SQLite raises rather than truncating:

OverflowError: Python int too large to convert to SQLite INTEGER

Verified end-to-end against a real create_execution. On this PR's path the exception lands in _execute_manual_trigger's blanket except — so the run is logged and silently lost, after the endpoint already answered "triggered". (On #1976's path it is caught and 500s.)

The range check is the missing sibling of a guard that was otherwise present — the function already drops unusable input for exactly this reason.

[B2] is_empty() used truthiness — user_id=0 read as no attribution

not any((self.user_id, ...)) — 0 is falsy. Hypothesis shrank it to {"source_user_id": 0}.

Latent: nothing in src/ calls it yet. That is exactly why it was worth fixing now, rather than leaving a public helper that lies about a valid value for the first caller to gate on.

Fixed here

Both, in 8c8f0b1, with tests/unit/test_execution_origin_properties.py — 31 boundary rows + 6 properties (total function, output types match the columns, strings stripped/capped/never-blank, is_empty agrees with the fields, parsing idempotent and non-mutating).

Branch coverage of src/scheduler/models.py: 100% (158 stmts, 6 branches, 0 partial) — before and after, which is the point.

Method notes

• Mutation gate not run (--mutate is off by default); the coverage-vs-bugs result above already makes the honest case better than a mutation score would.
• hypothesis==6.161.5 is already a pinned dep in tests/requirements-test.txt; the local venv was just stale. No new dependency.

[vybe]

Validated via /validate-pr — complete three-surface fix (backend router + scheduler + MCP client/tool), 25/27 regression tests fail pre-fix plus Hypothesis properties, retry inheritance and rolling-deploy compatibility verified both directions, extraHeaders hook refuses Authorization. I added the missing 'Fixes #1970' keyword and resolved the registry.json conflict (rebuild: dev + this PR's two entries); scheduler/service.py auto-merged cleanly against #1975 as predicted. Full CI green post-rebase. Note: stacked #1976 auto-retargets to dev on branch delete. Known follow-up: architecture.md schedule_executions DDL omits the source_* columns (pre-existing drift).