jpmschweitzer/scheduler
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 5 Wiki Activity

Compare commits

base: jpmschweitzer/scheduler:v1.2.0
Branches Tags
jpmschweitzer/scheduler:main
jpmschweitzer/scheduler:v1.9.0 jpmschweitzer/scheduler:v1.8.0 jpmschweitzer/scheduler:v1.7.0 jpmschweitzer/scheduler:v1.6.0 jpmschweitzer/scheduler:v1.5.1 jpmschweitzer/scheduler:v1.5.0 jpmschweitzer/scheduler:v1.4.0 jpmschweitzer/scheduler:v1.3.0 jpmschweitzer/scheduler:v1.2.0 jpmschweitzer/scheduler:v1.1.3 jpmschweitzer/scheduler:v1.1.2 jpmschweitzer/scheduler:v1.1.1 jpmschweitzer/scheduler:v1.1.0 jpmschweitzer/scheduler:v1.0.4 jpmschweitzer/scheduler:v1.0.3 jpmschweitzer/scheduler:v1.0.2 jpmschweitzer/scheduler:v1.0.1 jpmschweitzer/scheduler:v1.0.0
...
compare: jpmschweitzer/scheduler:main
Branches Tags
jpmschweitzer/scheduler:main
jpmschweitzer/scheduler:v1.9.0 jpmschweitzer/scheduler:v1.8.0 jpmschweitzer/scheduler:v1.7.0 jpmschweitzer/scheduler:v1.6.0 jpmschweitzer/scheduler:v1.5.1 jpmschweitzer/scheduler:v1.5.0 jpmschweitzer/scheduler:v1.4.0 jpmschweitzer/scheduler:v1.3.0 jpmschweitzer/scheduler:v1.2.0 jpmschweitzer/scheduler:v1.1.3 jpmschweitzer/scheduler:v1.1.2 jpmschweitzer/scheduler:v1.1.1 jpmschweitzer/scheduler:v1.1.0 jpmschweitzer/scheduler:v1.0.4 jpmschweitzer/scheduler:v1.0.3 jpmschweitzer/scheduler:v1.0.2 jpmschweitzer/scheduler:v1.0.1 jpmschweitzer/scheduler:v1.0.0
41 Commits
v1.2.0 ... main
Author SHA1 Message Date
jpmschweitzer 613ecb9fb1 fix(permissions): narrow rm -rf deny globs to their exact forms 
The trailing wildcard on the three rm -rf deny entries spanned path
separators, so Bash(rm -rf /*) matched every absolute path on the
machine rather than the filesystem root, and the ~ and $HOME entries
had the same shape. Narrowed to the exact literal forms.

These rules match literal command text, so they still stop a typo on
rm -rf /, rm -rf ~ or rm -rf $HOME exactly, but they no longer stop a
recursive delete aimed at any other path. That reduced cover is
deliberate, not an oversight.
 2026-08-25 20:31:21 +02:00
jpmschweitzerandClaude b1cb3eb899 build(make): deselect the integration tests, and give them a target (T-55) 
Twelve tests in test_database_integration.py carry
@pytest.mark.integration and need a real Postgres. The marker was
registered in pytest.ini and used correctly on the file — but `make test`
never deselected it and no target ever selected it. So those tests
neither ran nor passed: they errored on every invocation, and `make test`
exited 2 permanently, which teaches a reader that the exit code means
nothing. That is worse than either running them or not having them.

`make test` is now hermetic per D-26 — 164 passed, 19 deselected, exit 0,
verified inside an unprivileged network namespace as well as outside.

WHY THE AUDIT MISSED THIS, which is the part worth keeping. T-55 measured
each suite with and without a network and treated identical results as
proof of no live dependency. These twelve fail identically both ways,
because postgres-shared is a Docker-internal name that a host process
cannot resolve in either condition. A namespace proves a test does not
reach the network; it cannot distinguish that from a test whose
dependency is unreachable regardless. The positive control was run
against a host that WAS reachable, so it never covered this case.

The new target refuses an empty selection: pytest exit 5 fails with its
own message and a collection error gets a different one, so "nothing to
run" can never be read as "everything passed". Mutation-checked — anchor
asserted unique, marker renamed, target failed at exit 2 with the
intended message, Makefile restored byte-identical.

It reports honestly when Postgres is absent rather than skipping: 6
passed, 1 skipped, 12 errors from this host, which is the true state.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-18 20:44:24 +02:00
jpmschweitzerandClaude 3572a45322 test: delete six tests for a method that never existed 
_should_run_now has no definition in src/ in any commit in this repo's
history — checked with `git log --all -S` across the whole tree, not just
the current worktree. Twelve assertion sites across six tests called it,
so these have never passed and never protected anything.

The behaviour they describe is real: cron-wildcard matching of minute,
hour, day_of_month, month and day_of_week. It lives inside the WHERE
clause of get_tasks_for_minute, not as a Python predicate, so there was
nothing to rename them onto.

KNOWN GAP, stated rather than left implied: that matching is now covered
by no test at all. Testing it means either asserting against the SQL and
params a mocked cursor receives, or extracting the predicate out of the
query — the second changes what decides, every minute, which scheduled
work runs, and is not a refactor to do casually. Deleting was chosen over
rewriting because a test that has never run is not coverage, and leaving
it in place claimed some.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-18 20:44:08 +02:00
jpmschweitzerandClaude 292c7de2bf test(redaction): assert the coarse behaviour the source actually has 
This asserted that _redact_sensitive recurses into a dict under a
sensitive key — redacting auth.token while leaving auth.type readable.
The source replaces the whole value the moment the KEY matches, so
redacted["config"]["auth"] is a string and indexing ["token"] into it
raises TypeError. Source and test arrived in the same commit, so this
was never drift: it was a disagreement nobody settled.

Settled in favour of the source. Fine-grained redaction has to know
which sub-keys carry a secret, which is a guess about the shape of data
nobody has inspected; matching on the key cannot be wrong that way. Real
configs here are {"auth": {"type": "bearer", "token": "${SOME_KEY}"}},
and the cost of guessing wrong is a credential in a log, which no later
fix undoes. The price is readability, and it is paid deliberately.

Adds a second assertion that the secret appears nowhere in the output by
any path, which is the property actually worth protecting.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-18 20:44:08 +02:00
jpmschweitzer 6915be31ca fix(tests): match postgres_host assertion to the fixture actually in effect 
tests/conftest.py sets os.environ["POSTGRES_HOST"] = "postgres-shared" at
module level (before `from src.main import app`), commented "Use real
postgres for integration tests". test_settings_loads_from_environment
asserted settings.postgres_host == "test-postgres", a value grep confirms
nothing in this suite has ever set — git log -p shows both the conftest
line and this assertion originate in the same single commit and neither has
changed since. Matched the assertion to the environment the suite actually
runs under.

Not a source change and not a claim that "postgres-shared" is the right
fixture value for a suite this ticket also found is not actually hermetic
where that value is concerned (see the 12 pre-existing DNS errors, tracked
separately from this fix) — only that the assertion should test what the
fixture sets, not an unset value.
 2026-08-18 15:50:45 +02:00
jpmschweitzer 3f987bcf64 fix(tests): repair test_create_list_delete_task_flow's two defects 
Same dependency_overrides fix as the other test_*.py commits for
get_task_executor (patch() cannot reach a route's already-registered
Depends() reference — see the conftest.py commit for the full explanation
and the empirical check that established it).

Once the mock actually intercepted the DB layer, a second, independent
defect surfaced: KeyError: 0. mock_cursor.fetchone.side_effect held 3 values
sized for "create, list, delete" (one fetchone() each), but list_tasks uses
fetchall(), not fetchone(), and delete_task has grown a second fetchone()
call since this test was written — c34db66 added the 409-on-history-loss
check, which does `row = cur.fetchone()` for the task id and then a
separate `cur.fetchone()[0]` for its execution count. A real create->list->
delete flow through this endpoint set needs 1 (create) + 2 (delete) = 3
fetchone() values in that order, not one per named step. Re-sequenced so
delete_task sees a real id and a zero execution count, reaching the intended
200 rather than an unhandled KeyError.

Source unchanged; delete_task's behavior is deliberate (c34db66) and this
integration test had not been updated to track it.
 2026-08-18 15:50:29 +02:00
jpmschweitzer 68bea0cceb fix(tests): correct two independent AttributeErrors in test_task_executor 
1. `executor.max_concurrent` has never existed. Concurrency is capped by the
   module-level MAX_CONCURRENT_TASKS constant via asyncio.Semaphore(
   MAX_CONCURRENT_TASKS) in TaskExecutor.__init__ — confirmed with git log -p
   across this file's whole history (three commits), the name has always
   been the module constant, never an instance attribute.
   test_executor_initialization now asserts MAX_CONCURRENT_TASKS == 5 and the
   semaphore's initial count, instead of a name the class never had.
   test_concurrent_task_limit asserted `mock_execute.call_count <=
   executor.max_concurrent`, which — separately from the AttributeError — was
   asserting the wrong observable: process_minute() awaits the full batch via
   asyncio.gather before returning, so by the time the assertion runs all 10
   scheduled tasks have executed; the semaphore bounds how many run
   concurrently mid-flight, not the eventual call_count. Reworded to assert
   all scheduled tasks still run (call_count == len(tasks)); a concurrency-
   in-flight assertion would need a task that can be observed mid-execution,
   which the AsyncMock stand-in does not provide.

2. _run_executor (src/tasks/executor.py) loads the executor module with the
   __import__ builtin directly (`__import__(module_path, fromlist=
   ['execute'])`), not importlib.import_module — this repo's own CLAUDE.md
   documents it as "the thing that will mislead you" about this module.
   importlib is never imported there, so patch('src.tasks.executor.importlib.
   import_module') failed at patch setup, before the three
   test_execute_task_* bodies ran at all. Switched to patch('builtins.
   __import__', side_effect=...) with a routing function that falls through
   to the real import for anything other than the target module — verified
   the call-recording shape empirically first (call('name', fromlist=[...])).

Source is unchanged in both cases; both are test-only defects present since
this file's initial commit.
 2026-08-18 15:49:37 +02:00
jpmschweitzer 874f9f9711 fix(tests): repair three mock-wiring bugs in doc_sync_executor tests 
test_executor_successful_sync_entire_repo: _get_git_commit is patched
separately and never calls the real _run_command, so it does not consume a
slot in mock_run.side_effect. The list reserved one anyway (labelled "git
rev-parse HEAD"), which shifted "M  README.md\n" one call late — git status
--porcelain saw "" (no changes) instead, so execute() took the no-changes
branch and the test asserted "Successfully synced" against "already up to
date". Removed the phantom slot. Also gave the iterdir() mock items real
string .name attributes: MagicMock(name="X") sets the mock's repr, not the
.name attribute read by execute()'s `item.name != '.git'` check, so every
item was being treated as non-.git regardless of the intended value.

test_executor_sync_specific_paths, test_executor_handles_no_changes:
mock_upstream_dir/mock_gitea_dir were built but never wired to
mock_path.return_value, so `work_dir = Path(...)` and `upstream_dir =
work_dir / "upstream"` resolved to a different, unconfigured auto-generated
MagicMock. `source.name` on that mock is itself a MagicMock, not a string,
so `', '.join(copied_paths)` raised TypeError. Wired mock_path.return_value
to a work_dir mock whose __truediv__ yields the intended upstream/gitea
mocks, matching the pattern the first test already used correctly.

All three are test-side: doc_sync_executor.py is unchanged. Confirmed via
git log -p that this file and its test have exactly one commit in this
repo's history (the initial extraction), so there is no prior passing
version to regress from — these tests appear to have never passed.
 2026-08-18 15:49:20 +02:00
jpmschweitzer 5ccfb83f2f fix(tests): correct two independent test-api defects found while auditing T-55 
1. get_task_executor mocking: same dependency_overrides fix as the previous
   commit, applied to this file's remaining patch('src.main.get_task_executor')
   call sites (test_valid_api_key_allows_access, test_create_task_with_valid_data,
   test_trigger_task_endpoint, test_stats_endpoint_returns_metrics,
   test_executions_endpoint_returns_history, test_executions_filter_by_task_name).

2. test_create_task_missing_fields_returns_400 asserted a status this endpoint
   cannot return. `task: TaskCreate` in src/main.py is a plain Pydantic request
   body with no custom validation for missing fields — FastAPI's own
   dependency-resolution layer rejects the request before create_task's body
   runs, and that layer always answers 422, not 400. There is no code path in
   this repo, at any point in its git history, that produces 400 for this
   request. Renamed to test_create_task_missing_fields_returns_422 and the
   assertion updated to match; source unchanged.

Kept together in one commit because both live in the same small file and were
found in the same pass, rather than risk a manual hunk split on tightly
interleaved diff context.
 2026-08-18 15:49:08 +02:00
jpmschweitzer e39234b436 fix(tests): use dependency_overrides for get_task_executor, not patch() 
FastAPI resolves Depends(get_task_executor) against the function object it
captured when each route was decorated, at import time. patch('src.main.
get_task_executor') therefore never reaches an already-registered route —
confirmed empirically with a minimal FastAPI app before touching this repo's
tests. Every one of these tests fell through to the real dependency, which
called the real psycopg2.connect() and failed on postgres-shared's DNS (a
Docker-network name unreachable from a host process either way, so this
reproduced identically with and without host networking).

app.dependency_overrides is FastAPI's own supported mechanism for this, and
was already used correctly elsewhere in this suite (test_task_delete.py).
Added a shared override_task_executor fixture in conftest.py and switched
every get_task_executor patch() call site in test_api_comprehensive.py to it.

Source (src/main.py) is unchanged — Depends() is the idiomatic, correct
pattern; the tests were using an ineffective substitute for it.
 2026-08-18 15:48:29 +02:00
jpmschweitzerandClaude f9e1409898 build(setup): prove the venv actually works instead of trusting pip's exit code 
`make setup` created the venv and ran pip install, then exited 0 whether or
not the result was usable — exactly D-24's failure shape, and the one this
repo hit hardest: on 2026-08-09 there was no venv at all here while CLAUDE.md
documented .venv/bin/python -m pytest as the way to run tests, and nothing in
setup would have caught that state before a person did.

Add a `pytest tests/ --collect-only -q` step at the end. It imports every
test module and everything each one pulls in from src/, without running a
single test, and fails the target on a broken interpreter or a broken
dependency graph alike.

Verified directly:
- absent interpreter -> exit 127, target halts
- apscheduler (declared, imported by src/main.py) uninstalled -> collect-only
  exits 4 with ModuleNotFoundError, target halts
- make setup afterwards reinstalls it and collect-only exits 0, 189 tests
  collected, same count as before the mutation
- idempotent: a warm rerun on an already-correct venv changes nothing and
  collects the same 189 tests, just faster (7s vs 30s cold)

T-47 (workspace).

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-17 12:11:52 +02:00
jpmschweitzerandClaude 5dbc2c38a4 release v1.9.0
Build and Push / release (push) Successful in 3s
Details
Build and Push / build (push) Successful in 1m14s
Details 
Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 14:51:32 +02:00
jpmschweitzerandClaude 4e68767771 refactor(health-report): put summary where the other writer puts it 
check_history has two producers. sysmon-go writes `summary` at the top level
beside `status`; this module wrote it under `metrics`. So a reader had to know
which producer wrote a row before it could find out what the row said, and a
query written the obvious way found one and silently missed the other.

That is the T-36 failure repeating. There, per-domain queries returned rows from
August and looked like a system that had stopped reporting, because the data was
nested under a composite row nobody had mentioned. Nothing was missing; the
query was asking the wrong shape. verify.sh had already grown a coalesce over
both spellings, which is the tell: a compatibility shim that hides a schema
disagreement rather than resolving it.

D-33 made this table a contract between producers, and a contract needs one
spelling.

Summary is now a required parameter with no default. sysmon-go enforces the same
thing through Domain.Run's signature, and the reason is identical: a row whose
substance is missing looks exactly like a row whose check found nothing to say.
Both call sites pass it; the failure path passes the exception rather than
leaving the field to the metrics blob.

Old rows keep the nested spelling and verify.sh keeps reading both, because
rewriting history to match a new convention is a worse trade than a fallback
with a reason attached.

Also drops "Three consequences" from the module docstring, which by then listed
five. A hardcoded count beside the thing it counts is the same defect as
install.sh printing "wrote 8 keys" while writing ten — this morning's bug, in
prose instead of code.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 14:51:32 +02:00
jpmschweitzerandClaude 7b12ce8f0a release v1.8.0
Build and Push / release (push) Successful in 2s
Details
Build and Push / build (push) Successful in 1m13s
Details 
Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 12:29:18 +02:00
jpmschweitzer c34db66f51 fix(api): refuse to delete a task's history by accident, with 409 and ?purge 
DELETE /tasks/{name} issued a bare DELETE against scheduled_tasks. Any task
that had ever run owns rows in task_executions, so the foreign key rejected it
and the caller got:

  psycopg2.errors.ForeignKeyViolation: update or delete on table
  "scheduled_tasks" violates foreign key constraint
  "task_executions_task_id_fkey" on table "task_executions"

surfaced as a bare 500 with nothing naming history as the obstacle. It read as
the service being broken rather than the request being refusable, and since
every task that has ever fired has history, the endpoint effectively worked
only for tasks that had never run. Found while removing a temporary probe task,
which then had to be deleted with hand-written SQL across two tables.

Refusing rather than cascading, because the outcomes are not equally
recoverable: a task definition can be recreated from the API in one call, its
execution history cannot be recreated at all. Defaulting to the destructive
reading of an ambiguous request is how audit trails disappear quietly.

The 409 carries what the caller needs to act -- how many records are at stake,
the flag that proceeds anyway, and PUT enabled=false, which is usually what was
actually wanted: it stops the task running and keeps the record. A bare
"conflict" would be little better than the 500 it replaces.

Purge deletes history and task in one transaction. Split across two, a failure
between them leaves the audit trail gone and the task alive -- the worst of both.

Mutation-checked: removing the guard fails the refusal tests. A test also pins
that a refused delete issues no DELETE at all, and that ?purge=true on a missing
task is still 404 rather than a success.
 2026-08-11 12:29:18 +02:00
jpmschweitzerandClaude 4911c94e48 release v1.7.0
Build and Push / release (push) Successful in 3s
Details
Build and Push / build (push) Successful in 1m14s
Details 
Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 12:22:47 +02:00
jpmschweitzerandClaude 538bfe5944 feat(health-report): name the task that produced each check_history row 
`source` names the code that wrote a row. It cannot name the schedule that
invoked it, and two tasks may share one executor -- so a row could not answer
the question a health record mostly exists to answer: which job broke?

Concretely, on 2026-08-11 a 425 MB probe task and the 5 GB nightly backup both
ran through config_backup_executor. The probe failed and wrote

  source: scheduler/config_backup_executor
  status: critical
  error:  Backup file was not created

which is byte-for-byte what a nightly backup failure would have written. The
row was true and unattributable, and the reflex it invited -- delete the
inconvenient row -- was correctly refused. Attribution is the actual fix: the
record stays intact and starts saying who it is about.

Carried in a ContextVar rather than an argument. Executors are invoked as
execute(config, settings) and there are ten of them, several dormant -- existing
only as a string in a database row and becoming live the moment someone inserts
a task naming them. A signature change would leave those broken in a way nothing
imports, greps or tests would reveal. Injecting the name into `config` was the
other option and is worse: `config` is what a human wrote in the task
definition, and an executor is entitled to reject keys it does not recognise.

Two properties make the ContextVar safe, both verified in the deployed runtime
rather than reasoned about:

  - asyncio.to_thread propagates the context, so reporting still sees the task
    after T-74 moved executor bodies into worker threads. Had it not, every row
    from a real executor would have quietly lost its task while unit tests kept
    passing -- so there is a test that specifically goes through report_async.
  - Each asyncio Task gets its own copy, so the five concurrent executions
    MAX_CONCURRENT_TASKS permits cannot read each other's value. The isolation
    test yields mid-execution to force interleaving; without that it would pass
    even against a shared global.

A plain await does NOT get its own copy and leaks the value to the caller, which
the runtime check showed. Both real entry points go through create_task, but
task_scope resets via token rather than depending on that.

The field is omitted, not nulled, when there is no task: report() is callable
from a script, and a null would claim a task existed with no name.

Mutation-checked: removing the scope from the engine fails both isolation tests.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 12:22:47 +02:00
jpmschweitzerandClaude 1c861e2fe1 release v1.6.0
Build and Push / release (push) Successful in 4s
Details
Build and Push / build (push) Successful in 1m15s
Details 
Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 12:09:35 +02:00
jpmschweitzerandClaude e45de4fec7 fix(orchestrator): record the terminal states that were never written 
Two defects with one shape: an execution reaches a terminal condition and the
orchestrator fails to write it down, so the system's own record disagrees with
what happened. Neither produced an error. Both produced silence.

T-2 -- a restart mid-task unscheduled that task forever.

get_tasks_for_minute excludes any task holding a task_executions row with
status='running'. The row is written before the executor runs and updated
after, so a process dying in between left it 'running' permanently, and the
task was then excluded from every future minute with no error, no alarm and no
log line. It did not fail; it went quiet.

test_example_task had held such a row since 2025-12-07 -- 5916 hours. It is
disabled, so nothing was broken by that instance; the mechanism is the point,
and the exposure is daily, because Watchtower restarts this container at 4 AM
while the config backup starts at 03:05 and runs ~21 minutes.

Startup now reconciles them, where the reasoning is sound by construction: this
process has just begun, so nothing it can see is genuinely running.

Marked 'orphaned', not 'failed'. When the process dies mid-task the work may
well have completed -- a backup that finished and never got to update its row
is indistinguishable from one that died halfway -- and 'failed' would assert an
outcome nobody observed. Same error as the health-report diagnostic fixed in
18be804: naming a cause you did not witness.

Not extended to a duration-based sweep. While this process lives, execute_task's
finally clause always closes the row, so a stale row implies a dead owner. A
time-based rule would have to tell a slow task from a dead one, and getting that
wrong closes the record of a task still working.

T-3 -- the 'timeout' status was unreachable.

execute_task has an `except asyncio.TimeoutError` branch that records
status='timeout'. It could never run: _run_executor wrapped the awaited call in
`except Exception`, and since 3.11 asyncio.TimeoutError IS the builtin
TimeoutError (OSError -> Exception), so the broad handler caught it first and
converted it to an ordinary error tuple. Confirmed in the deployed runtime and
against the history -- 18,785 executions since 2025-12-07, of which 'timeout'
rows: zero. Every timeout in eight months was filed as a generic failure,
erasing the distinction between "too slow for its window" and "broken".

A narrower except after a broader one is dead code, and no linter is configured
here to say so.

One trap in fixing it: while the branch was unreachable a timeout travelled the
normal path, which DOES update scheduled_tasks. Making the branch reachable
without that write would have traded a wrong status for a stale one, so
_update_task_outcome now mirrors terminal outcomes onto the parent row.

The timeout message also states that the work may still be running -- after
T-74 executors are handed to asyncio.to_thread, and a thread cannot be
cancelled, so wait_for frees the loop while the work continues.

Both fixes are mutation-checked: removing the startup call fails the ordering
test, removing the narrow except clause fails the propagation test. Suite goes
118 -> 126 passing with the same 36 pre-existing failures.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 12:09:35 +02:00
jpmschweitzerandClaude d91d0f2c63 release v1.5.1
Build and Push / release (push) Successful in 2s
Details
Build and Push / build (push) Successful in 1m14s
Details 
Also collapses a duplicated changelog section. A second "## [Unreleased]"
heading has existed at the foot of the file since 2026-08-07, above the
roadmap list -- and because it matched first, my own v1.5.0 and T-69 commits
wrote their entries into both copies. The roadmap is now "## Planned", which is
what it always was, so the two cannot collide again.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 11:35:29 +02:00
jpmschweitzerandClaude b62024b21d fix(executors): run backups off the event loop 
The scheduler serves its own REST API from the same loop that runs executors,
and the config backup spends ~21 minutes inside tarfile and zlib. Called inline
that starves the loop for the whole window: the service was unreachable
03:05-03:25 every night, and again at 07:39 today when the job was triggered by
hand to prove the T-69 report path. asyncio.to_thread is the fix -- zlib
releases the GIL while compressing, so the loop is scheduled normally.

The outage was invisible for as long as it existed. The hourly health check
fires at :35 and the outage runs 03:05-03:25, so no sample ever landed inside
it. A fixed-phase hourly probe cannot see a 20-minute event; that is aliasing,
not bad luck, and it would have stayed hidden indefinitely.

health_report.report_async joins it: psycopg2 is a blocking driver, so
reporting from the loop held it for the connect and insert -- up to
connect_timeout seconds precisely when the database is unreachable, which is
when a report matters most. Both backup executors use it now.

Caveat worth knowing: the engine wraps executors in asyncio.wait_for and a
thread cannot be cancelled, so on timeout the task is recorded failed while the
tar runs to completion. Still strictly better than blocking everything, and the
configured 3600s is well clear of the observed 1263s.

Seven tests in this file had been red since the initial commit -- they came
over with the portainer-core extraction, patched the Path class wholesale,
asserted "backed up" against a function returning "Backup completed: ...", and
one wrapped its call in except Exception: pass with its only assertion
commented out. There is no CI test gate here, so nothing reported it. Replaced
with tests that build real archives in tmp_path and assert on their contents.

The new loop test is the one that matters and it is mutation-checked: with
to_thread reverted it counts 0 heartbeat ticks, with it ~40.

Their structural demands (_create_tar_filter, a sync _cleanup_old_backups) were
adopted because threading wanted that shape anyway. Their exclude semantics
were not. The tests assert fnmatch behaviour and the deployed config is written
against substring matching -- it excludes logs as ".log", and paths as
unanchored fragments like "ollama/models/*" against members named
"docker-data/ollama/...". Under fnmatch neither matches, and the nightly
archive would silently gain many GB of model blobs instead of shrinking. That
is now pinned by tests naming the consequence, so the "improvement" fails loudly.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 11:35:29 +02:00
jpmschweitzerandClaude 18be804ba0 fix: report the database's refusal instead of guessing its cause 
The health report's InsufficientPrivilege handler said 'the scheduler's
database user lacks INSERT on check_history'. That grant was in place. What was
actually missing was USAGE on check_history_id_seq — the sequence behind the
table's serial id — so an INSERT was refused for a reason the message did not
mention and actively contradicted.

Verified by attempting the insert directly as scheduler_user:
  ERROR: permission denied for sequence check_history_id_seq

A diagnostic that names a cause it did not observe is worse than a generic one:
it sends the reader to a fix that is already applied, and reads as evidence the
grant did not work. The message now prints psycopg2's own first line.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 10:04:41 +02:00
jpmschweitzerandClaude 1012f6f374 release v1.5.0
Build and Push / release (push) Successful in 3s
Details
Build and Push / build (push) Successful in 1m14s
Details 
Backup executors report their own outcome to check_history. The grant that
makes it functional — INSERT for scheduler_user on the sysmon database — was
applied 2026-08-11, so this deploy is the step that closes the gap: the backup
domain has had no writer since the file-age poll was retired.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-11 09:17:27 +02:00
jpmschweitzer d9cbaee1fc feat: backup executors report their own outcome to check_history 
The health record used to learn about backups by polling the mtime of the
newest archive, hourly, against a 48-hour threshold — for a job that runs once
a day. Forty-seven of every forty-eight runs could not produce a new answer.

Worse, file age cannot distinguish a failed backup from one that has not run
yet: when tonight's job dies, yesterday's archive is 24 hours old and still
reads healthy, and keeps reading healthy until hour 48. A failure stayed
invisible for two days to the check whose only job was noticing it. These
executors know at 03:05.

Reporting wraps the work rather than living inside it, so the failure path
cannot be forgotten — an exception is reported as critical and re-raised,
leaving the task's own status untouched. Reporting only success would reproduce
exactly the blind spot this replaces.

A reporting failure never fails the backup. Everything in health_report is
caught and logged, which means the absence of rows is the only symptom a broken
reporter produces — so monitor for rows, not for errors.

Not yet functional in production: scheduler_user holds DELETE and SELECT on
check_history (it prunes the table nightly) but not INSERT. Until that grant is
made the report logs a refusal and skips. The ticket predicted this as the step
that would fail silently, which is why it is named in the changelog and handled
as its own exception rather than folded into a generic catch.

Verified against baseline: tests/test_config_backup_executor.py and
tests/test_portainer_backup_executor.py report 7 failed / 17 passed both with
and without this change, so the pre-existing failures are untouched.

Workspace D-33, T-69.
 2026-08-11 00:11:52 +02:00
jpmschweitzerandClaude 0c2199667f build(ci): move the pre-push gate into the Makefile 
The hook carried ~50 lines of gitleaks logic and a comment explaining it was
self-contained because "this repo has no Makefile". It has one now, so the
reason is gone and the arrangement is backwards: a hook is a trigger, and
logic belongs where it can be read, run by hand, and changed under review.

.githooks/pre-push is now a byte-identical shim onto `make pre-push` in every
repo in the workspace. The scan itself moves to ci/secrets.sh unchanged, and
`make secrets` runs it on its own.

The call surface is identical everywhere; what it runs is not, and should not
be — each repo gates what it actually has. That is the point of standardising
the name rather than the contents: nobody has to read a repo to find out how
to check it.

secrets runs first, deliberately. It is the only failure here that cannot be
undone by fixing it afterwards — a failed lint costs another commit, a pushed
credential is cached and indexed whether or not it is later deleted.

Some of these gates fail today, on lint debt that predates them, and they are
left wired anyway. The board was measured once and written down in T-56
instead of being worked around here. Narrowing each gate to whatever already
passes would produce a gate that reports success for doing nothing, which is
the failure this workspace keeps rediscovering.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 18:57:22 +02:00
jpmschweitzerandClaude 054974c3fb ci(make): reserve exit 69 for "could not run" (D-26) 
Environment guards now exit 69 rather than 1, so a caller can tell a suite
that could not start from one that ran and failed. The first toj test sweep
reported "3 repositories failed" and none of the three had executed a test —
two could not find go, one had no venv. That points the reader at the tests
when the fault is in the environment.

Only the environment guards change. A gitleaks finding, a failed test run and
a vulncheck hit still exit 1, because those did run and did fail.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 15:56:21 +02:00
jpmschweitzerandClaude b712caefb1 build: add the Makefile command surface (D-27) 
Every repo gets one at the root: help, plus test and lint where those exist.
The point is that a target name means the same thing in every repo, so an
agent or a person can act without reading the repo first.

Paths resolve here rather than in callers (D-10). python3 on this host is 3.8
and cannot parse these sources, and a bare pytest or ruff resolves only in a
login shell — so both are named explicitly through the venv, and a missing
venv fails with the command to fix it rather than a bare no-such-file.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 15:10:33 +02:00
jpmschweitzerandClaude 5b4814a3cd chore(claude): pin PQL_VAULT per project so cwd stops choosing the vault 
pql is now a bare word on PATH, which removed the long incantation that had
been forcing --vault into every call by habit. Convenience lowered the cost
of the wrong thing without lowering the cost of the right one: a three-word
pql ticket new targets whichever vault the cwd happens to sit in, and there
are nine of them with colliding id sequences.

PQL_VAULT in each project settings file makes the vault a property of the
session rather than of the working directory — the same lesson Rule 3 records
for git -C, applied to pql. Verified the env var overrides cwd discovery,
that an explicit --vault still beats the env var, and that the harness
hot-reloads it without a restart.

This does not make provenance visible: no output says which vault answered,
so a forgotten --vault still returns a well-formed answer about the wrong
dataset. That remains T-37.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 13:49:00 +02:00
jpmschweitzerandClaude 9eb9e4b32b chore(claude): deny toj in the sub-repos 
toj is now on the global PATH as /usr/local/bin/toj, so its scope boundary
had to stop being "the absolute path is inconvenient to type" and start
being a rule. Its repo and settings verbs operate on the workspace root; run
from inside this repo they answer about the wrong tree.

Both spellings are denied, bare and absolute, because a deny with one
spelling left open is decorative.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 13:42:06 +02:00
jpmschweitzerandClaude c64412d9b3 ci: gate pushes on a gitleaks scan of the outgoing commits 
No repo here scanned for committed credentials. The hook is self-contained
rather than delegating to a Makefile, because this repo has none and a hook
reaching into a sibling repo breaks the moment this one is cloned elsewhere.

Scans the outgoing range rather than full history: history carries settled
findings — test fixtures, vendored third-party code — and a gate that fails
on something unfixable gets bypassed within a week.

Setting core.hooksPath means pql init must replant its replication shims into
.githooks, which is why they are gitignored here alongside the tracked
pre-push. Same layout pql itself uses.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 12:48:54 +02:00
jpmschweitzerandClaude 4cbfe7a6f9 docs: qualify workspace decision ids cited from this repo 
Decision ids are per-vault sequences, so they collide by construction
once there is more than one vault -- and every repo now has one. A bare
D-15 here will mean this repo's D-15 the moment this repo records one.
Cross-vault references are therefore qualified: workspace D-15.

Not hypothetical: pql holds D-1 through D-31 while the workspace holds
D-1 through D-21, so every workspace id currently collides with an
unrelated pql one. A bare id is not wrong the day it is written -- it
decays into wrong as the other vault grows, and nothing flags it.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 04:17:07 +02:00
jpmschweitzerandClaude e975f5b720 docs: replace AGENTS.md with a repo-specific CLAUDE.md 
One agent doc per repo, and it is CLAUDE.md. Written fresh rather than
reformatted. The old file's feature-branch mandate and its `git add -A`
release snippet are both gone, and its health-check URL pointed at port
8000, which is tatlock -- this service is 8090.

The section worth reading is on executors, because neither of the usual
ways to establish what code is live works here. src/executors/*.py are
never statically imported: src/tasks/executor.py builds the module path
from a scheduled_tasks row and calls __import__ at execution time. So a
grep finds no importer, and a cold sys.modules snapshot shows none of
them loaded. The authoritative source is the database, and the doc
carries the query.

That distinction matters for gcs_backup_executor, which has zero rows
today. It is dormant, not dead: it becomes live the moment someone
inserts a row naming it, with no code change and no deploy.

Also records that the table is scheduled_tasks even though the API path
is /tasks, so the obvious query fails with UndefinedTable, and that the
empty src/config/ directory does not shadow src/config.py -- verified in
the container, a regular module wins over a namespace package.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 03:15:15 +02:00
jpmschweitzerandClaude 55fe0f9083 chore: adopt the workspace agent-config baseline 
Commits a .claude/settings.json rather than leaving permissions to
per-developer local state, and initialises a pql vault for this repo's
tickets and internal decisions.

Every git deny rule appears in both the `git <verb>` and `git * <verb>`
forms. Only the second catches `git -C <path>`, and without it the whole
deny list is decorative -- it looks like a policy and stops nothing.

The allow list carries pql's absolute path alongside the bare name.
pql is installed to ~/.local/bin, which is on the login PATH but not the
one a non-interactive shell gets, so the bare-name rules match nothing on
their own and every call would prompt anyway.

.gitignore now covers .claude/settings.local.json, which is machine-local
and must never be shared. `pql init` contributed the .pql/* rules with an
exception for the changelog, which is the replication log of record and
has to be committed for tickets to travel with a clone.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-09 03:14:59 +02:00
jpmschweitzerandClaude 0865763597 chore: release v1.4.0
Build and Push / release (push) Successful in 3s
Details
Build and Push / build (push) Successful in 32s
Details 
Ships the Portainer backup executor.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-08 19:56:44 +02:00
jpmschweitzerandClaude 933196c2a2 feat(executors): back up Portainer's own state 
Portainer keeps every stack definition, endpoint, user and access-control
rule in a BoltDB inside the portainer_data Docker volume. That volume
sits under /var/lib/docker/volumes/, and the daily config backup covers
~/docker-data and code-server-config only — so the thing that defines all
24 stacks was the one thing not backed up.

Calls Portainer's /api/backup rather than tarring the volume. BoltDB is a
single memory-mapped file, so copying it while Portainer writes can
capture a torn page; the API serialises a consistent snapshot.

A 200 whose body is not a readable archive is treated as failure. An
archive that will not open is worse than a missing one, because it looks
like a backup until the day it is needed. Writing that check found a real
gap in it: a truncated tar.gz raises EOFError, which is neither TarError
nor OSError, so the first version of the guard let it through.

Archives contain TLS certificates and private keys and are written 0600.
Retention only ever deletes files matching the exact name this executor
writes, so an unrelated archive left in the same directory survives.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-08 19:56:44 +02:00
jpmschweitzerandClaude 23cd5ddca8 chore: release v1.3.0
Build and Push / release (push) Successful in 4s
Details
Build and Push / build (push) Successful in 2m7s
Details 
Carries the two new pruning executors and the POST /tasks fix, which has
been on main unreleased since the image only rebuilds on a version tag.

Also backfills the missing 1.2.0 changelog entry: that version was tagged
and shipped without one.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-08 11:01:52 +02:00
jpmschweitzerandClaude cfabe1b4d1 docs: correct the executor list in TASK_REGISTRATION 
The "Other Executors" section advertised shell, python and docker
executors that were never implemented, and omitted every executor that
was. The missing shell executor in particular sent a recent piece of
work down the wrong path before the gap was noticed.

Lists the modules that actually exist and documents the config for the
two new ones.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-08 11:00:42 +02:00
jpmschweitzerandClaude 7b80e30691 feat(executors): add docker prune executor 
Non-interactive equivalent of system-admin-toj's prune-docker.sh, which
prompts per stage and so cannot run from cron.

Only the stages that discard regenerable data run by default: build
cache and dangling images. Unused images and volumes are opt-in, because
docker volume prune removes volumes belonging to merely-stopped
containers rather than only orphaned ones, which on this host is a
plausible way to lose a database.

A failing stage is reported and the remaining stages still run, since a
partial reclaim beats none, but the task still ends up failed so the
error is not swallowed.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-08 11:00:42 +02:00
jpmschweitzerandClaude 788c03514a feat(executors): add postgres retention executor 
Deletes rows past a retention window from a table on the shared Postgres
server. Written for sysmon's check_history, which grows with every
monitoring check and had no retention at all despite the docs promising
a 30-day rolling window.

Connects with the Scheduler's own credentials and overrides only the
database name, so no second set of secrets enters the stack. The target
database grants scheduler_user just SELECT and DELETE on the table, so a
bug here can drop old rows but cannot corrupt or forge history.

Table and column names cannot be bound as query parameters, so both are
validated against a strict identifier pattern before interpolation, and
a retention window below 1 day is refused rather than silently emptying
the table.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-08 11:00:42 +02:00
jpmschweitzerandClaude ae4f9e6a20 fix(api): return created task instead of 500 on POST /tasks 
TaskResponse declared created_at and updated_at as str, but both are
timestamp columns and psycopg2 returns datetime objects. Pydantic
rejected every response, so the endpoint raised ResponseValidationError
after the INSERT had already committed.

Every task creation therefore looked like a failure, and the natural
retry failed again with a genuine duplicate-key violation, making it
appear the first attempt had done nothing.

Declaring them as datetime leaves the JSON on the wire unchanged
(FastAPI serialises to ISO 8601) and matches what GET /tasks/{task_name}
already returned.

Co-Authored-By: Claude <noreply@anthropic.com>
 2026-08-07 17:14:16 +02:00
jpmschweitzerandClaude Fable 5 c1fbc1cdb0 chore(ci): push images via git.schweitz.net registry 
The .internal registry domain is being retired; git.schweitz.net now
serves the registry without SSO on /v2/.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
 2026-07-19 11:10:08 +02:00
44 changed files with 4391 additions and 827 deletions
Expand all files Collapse all files
.claude/settings.json
+70
View File
@@ -0,0 +1,70 @@
{
"env": {
"PQL_VAULT": "/mnt/media/Projects/scheduler"
},
"permissions": {
"allow": [
"Bash(pql)",
"Bash(pql *)",
"Bash(/home/jpmschweitzer/.local/bin/pql:*)",
"Bash(git status:*)",
"Bash(git log:*)",
"Bash(git diff:*)",
"Bash(git branch:*)",
"Bash(.venv/bin/python -m pytest:*)",
"Bash(.venv/bin/pytest:*)",
"Bash(pytest:*)",
"Bash(docker logs scheduler:*)",
"Bash(curl -s http://localhost:8090/*)"
],
"deny": [
"Bash(/mnt/media/Projects/cladmin/ops/bin/toj)",
"Bash(/mnt/media/Projects/cladmin/ops/bin/toj:*)",
"Bash(chmod -R 777 *)",
"Bash(chmod 777 *)",
"Bash(dd if=*)",
"Bash(find * -delete*)",
"Bash(find * -exec*)",
"Bash(git * add --all*)",
"Bash(git * add -A*)",
"Bash(git * add .)",
"Bash(git * branch -D *)",
"Bash(git * checkout -- *)",
"Bash(git * clean -fd*)",
"Bash(git * clean -fdx*)",
"Bash(git * commit --no-verify*)",
"Bash(git * merge --no-ff*)",
"Bash(git * push --force*)",
"Bash(git * push -f*)",
"Bash(git * reset --hard*)",
"Bash(git * restore .*)",
"Bash(git add --all*)",
"Bash(git add -A*)",
"Bash(git add .)",
"Bash(git branch -D *)",
"Bash(git checkout -- *)",
"Bash(git clean -fd*)",
"Bash(git clean -fdx*)",
"Bash(git commit --no-verify*)",
"Bash(git merge --no-ff*)",
"Bash(git push --force*)",
"Bash(git push -f*)",
"Bash(git reset --hard*)",
"Bash(git restore .*)",
"Bash(mkfs*)",
"Bash(psql * -c DELETE FROM scheduled_tasks*)",
"Bash(psql * -c DROP*)",
"Bash(psql * DROP DATABASE*)",
"Bash(psql * TRUNCATE*)",
"Bash(redis-cli * FLUSHALL*)",
"Bash(redis-cli * FLUSHDB*)",
"Bash(rm -rf $HOME)",
"Bash(rm -rf /)",
"Bash(rm -rf ~)",
"Bash(su *)",
"Bash(sudo *)",
"Bash(toj)",
"Bash(toj:*)"
]
}
}
.gitattributes
+1
View File
@@ -0,0 +1 @@
.pql/changelog/*.sql merge=union
.gitea/workflows/build.yml
+3 -3
View File
@@ -26,7 +26,7 @@ jobs:
- name: Login to Gitea Registry
uses: docker/login-action@v3
with:
registry: git.schweitz.internal
registry: git.schweitz.net
username: ${{ secrets.REGISTRY_USER }}
password: ${{ secrets.REGISTRY_PASSWORD }}
@@ -36,8 +36,8 @@ jobs:
context: .
push: true
tags: |
git.schweitz.internal/jpmschweitzer/scheduler:latest
git.schweitz.internal/jpmschweitzer/scheduler:${{ github.ref_name }}
git.schweitz.net/jpmschweitzer/scheduler:latest
git.schweitz.net/jpmschweitzer/scheduler:${{ github.ref_name }}
- name: Trigger Watchtower update
if: success()
.githooks/pre-push
Executable
+13
View File
@@ -0,0 +1,13 @@
#!/usr/bin/env bash
# Trigger only. The checks live in the Makefile, where they can be read, run by
# hand (`make pre-push`), and changed under review.
#
# This file is identical in every repo in this workspace, deliberately: the call
# surface is the same everywhere even though what each gate runs is not, so
# nobody has to read a repo to find out how to check it (D-27).
#
# Enable per clone with: git config core.hooksPath .githooks
# Never bypass with --no-verify. Suppress a specific finding deliberately
# instead, with a reason — see `make pre-push`.
set -euo pipefail
exec make -C "$(git rev-parse --show-toplevel)" pre-push
.gitignore
+13
View File
@@ -87,3 +87,16 @@ cython_debug/
# Project-specific
logs/
task-data/
# Claude Code user-specific settings
.claude/settings.local.json
.pql/*
!.pql/changelog/
# pql shims planted by `pql init` into the dir core.hooksPath points at.
# Per-clone: each embeds the absolute path of the pql binary that planted it.
# Only .githooks/pre-push is shared.
.githooks/pre-commit
.githooks/post-merge
.githooks/post-checkout
.githooks/post-rewrite
.pql/changelog/0000-format.sql
+11
View File
@@ -0,0 +1,11 @@
-- Changelog format marker, written by pql. Comments only: this file
-- is never executed — Import descends into the per-table directories
-- and does not read the changelog root.
--
-- A changelog carrying no marker is format 1, the shape that existed
-- before formats were versioned. An older format is migrated forward
-- by `pql plan upgrade` (and automatically from the post-merge hook);
-- a newer one is refused rather than replayed under rules this binary
-- does not know. See D-28 and docs/versions.md.
-- pql:changelog_format: 2.0.0
-- pql:written_by: 2.2.0
.pql/changelog/ticket_deps/0000-schema.sql
+139
View File
@@ -0,0 +1,139 @@
-- Auto-generated by pql init. CREATE TABLE statements
-- for the planning schema; per-table dir keeps the changelog
-- self-describing per D-15. CREATE TABLE IF NOT EXISTS is
-- idempotent so running schema files from each directory in
-- replay order is harmless.
--
-- Importer parses the markers below to detect schema drift
-- between the producing pql version and the local one — a
-- bumped canonical_version means projection rules changed
-- and replay must refuse rather than silently corrupt state.
-- pql:created_by: 2.2.0
-- pql:canonical_version: 2
CREATE TABLE IF NOT EXISTS decisions (
id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('confirmed','question','rejected')),
domain TEXT NOT NULL,
title TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'active'
CHECK(status IN ('active','superseded','resolved','open')),
date TEXT,
file_path TEXT NOT NULL,
synced_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS decision_refs (
source_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
target_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
ref_type TEXT NOT NULL
CHECK(ref_type IN ('supersedes','references','resolves','depends_on','amends')),
note TEXT,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (source_id, target_id, ref_type)
);
-- Identity split (D-26): a ticket's stable, collision-proof identity is its
-- record_id (a locally-generated ULID, planning.NewRecordID); the friendly
-- T-NNN label lives in ticket_idmap and may be reconciled. Every structural
-- reference (parent, deps, history, labels) targets record_id, so a label
-- clash never corrupts the graph — only ticket_idmap needs a relabel.
CREATE TABLE IF NOT EXISTS tickets (
record_id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('initiative','epic','story','task','bug')),
parent_record_id TEXT REFERENCES tickets(record_id),
title TEXT NOT NULL,
description TEXT,
-- No CHECK enumeration: the ticket status vocabulary is per-vault
-- configurable (ticket_statuses in .pql/config.yaml). Validation lives
-- in Go (planning.StatusSet), so adding/renaming statuses needs no
-- schema change. The DEFAULT is a harmless fallback — CreateTicket
-- always inserts the configured default explicitly.
status TEXT NOT NULL DEFAULT 'backlog',
priority TEXT DEFAULT 'medium'
CHECK(priority IN ('critical','high','medium','low')),
assigned_to TEXT,
team TEXT,
decision_ref TEXT REFERENCES decisions(id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
-- ticket_idmap maps a record_id to its current friendly label (T-NNN).
-- ticket_id is intentionally NOT globally unique: two uncoordinated clones
-- can mint the same label, which surfaces as a duplicate-label collision
-- (detected at replay) and is fixed with "pql ticket relabel".
CREATE TABLE IF NOT EXISTS ticket_idmap (
record_id TEXT PRIMARY KEY REFERENCES tickets(record_id),
ticket_id TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_deps (
blocker_record_id TEXT NOT NULL REFERENCES tickets(record_id),
blocked_record_id TEXT NOT NULL REFERENCES tickets(record_id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (blocker_record_id, blocked_record_id)
);
CREATE TABLE IF NOT EXISTS ticket_history (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
field TEXT NOT NULL,
old_value TEXT,
new_value TEXT,
changed_by TEXT,
changed_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT UNIQUE,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_labels (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
label TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (ticket_record_id, label)
);
CREATE TABLE IF NOT EXISTS meta (
key TEXT PRIMARY KEY,
value TEXT NOT NULL,
updated_at TEXT NOT NULL DEFAULT (datetime('now'))
);
CREATE INDEX IF NOT EXISTS idx_tickets_status ON tickets(status);
CREATE INDEX IF NOT EXISTS idx_tickets_team ON tickets(team);
CREATE INDEX IF NOT EXISTS idx_tickets_decision_ref ON tickets(decision_ref);
CREATE INDEX IF NOT EXISTS idx_tickets_assigned ON tickets(assigned_to);
CREATE INDEX IF NOT EXISTS idx_tickets_parent ON tickets(parent_record_id);
CREATE INDEX IF NOT EXISTS idx_ticket_idmap_label ON ticket_idmap(ticket_id);
CREATE INDEX IF NOT EXISTS idx_decisions_domain ON decisions(domain);
CREATE INDEX IF NOT EXISTS idx_decisions_type ON decisions(type);
CREATE INDEX IF NOT EXISTS idx_decision_refs_target ON decision_refs(target_id);
.pql/changelog/ticket_history/0000-schema.sql
+139
View File
@@ -0,0 +1,139 @@
-- Auto-generated by pql init. CREATE TABLE statements
-- for the planning schema; per-table dir keeps the changelog
-- self-describing per D-15. CREATE TABLE IF NOT EXISTS is
-- idempotent so running schema files from each directory in
-- replay order is harmless.
--
-- Importer parses the markers below to detect schema drift
-- between the producing pql version and the local one — a
-- bumped canonical_version means projection rules changed
-- and replay must refuse rather than silently corrupt state.
-- pql:created_by: 2.2.0
-- pql:canonical_version: 2
CREATE TABLE IF NOT EXISTS decisions (
id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('confirmed','question','rejected')),
domain TEXT NOT NULL,
title TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'active'
CHECK(status IN ('active','superseded','resolved','open')),
date TEXT,
file_path TEXT NOT NULL,
synced_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS decision_refs (
source_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
target_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
ref_type TEXT NOT NULL
CHECK(ref_type IN ('supersedes','references','resolves','depends_on','amends')),
note TEXT,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (source_id, target_id, ref_type)
);
-- Identity split (D-26): a ticket's stable, collision-proof identity is its
-- record_id (a locally-generated ULID, planning.NewRecordID); the friendly
-- T-NNN label lives in ticket_idmap and may be reconciled. Every structural
-- reference (parent, deps, history, labels) targets record_id, so a label
-- clash never corrupts the graph — only ticket_idmap needs a relabel.
CREATE TABLE IF NOT EXISTS tickets (
record_id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('initiative','epic','story','task','bug')),
parent_record_id TEXT REFERENCES tickets(record_id),
title TEXT NOT NULL,
description TEXT,
-- No CHECK enumeration: the ticket status vocabulary is per-vault
-- configurable (ticket_statuses in .pql/config.yaml). Validation lives
-- in Go (planning.StatusSet), so adding/renaming statuses needs no
-- schema change. The DEFAULT is a harmless fallback — CreateTicket
-- always inserts the configured default explicitly.
status TEXT NOT NULL DEFAULT 'backlog',
priority TEXT DEFAULT 'medium'
CHECK(priority IN ('critical','high','medium','low')),
assigned_to TEXT,
team TEXT,
decision_ref TEXT REFERENCES decisions(id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
-- ticket_idmap maps a record_id to its current friendly label (T-NNN).
-- ticket_id is intentionally NOT globally unique: two uncoordinated clones
-- can mint the same label, which surfaces as a duplicate-label collision
-- (detected at replay) and is fixed with "pql ticket relabel".
CREATE TABLE IF NOT EXISTS ticket_idmap (
record_id TEXT PRIMARY KEY REFERENCES tickets(record_id),
ticket_id TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_deps (
blocker_record_id TEXT NOT NULL REFERENCES tickets(record_id),
blocked_record_id TEXT NOT NULL REFERENCES tickets(record_id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (blocker_record_id, blocked_record_id)
);
CREATE TABLE IF NOT EXISTS ticket_history (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
field TEXT NOT NULL,
old_value TEXT,
new_value TEXT,
changed_by TEXT,
changed_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT UNIQUE,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_labels (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
label TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (ticket_record_id, label)
);
CREATE TABLE IF NOT EXISTS meta (
key TEXT PRIMARY KEY,
value TEXT NOT NULL,
updated_at TEXT NOT NULL DEFAULT (datetime('now'))
);
CREATE INDEX IF NOT EXISTS idx_tickets_status ON tickets(status);
CREATE INDEX IF NOT EXISTS idx_tickets_team ON tickets(team);
CREATE INDEX IF NOT EXISTS idx_tickets_decision_ref ON tickets(decision_ref);
CREATE INDEX IF NOT EXISTS idx_tickets_assigned ON tickets(assigned_to);
CREATE INDEX IF NOT EXISTS idx_tickets_parent ON tickets(parent_record_id);
CREATE INDEX IF NOT EXISTS idx_ticket_idmap_label ON ticket_idmap(ticket_id);
CREATE INDEX IF NOT EXISTS idx_decisions_domain ON decisions(domain);
CREATE INDEX IF NOT EXISTS idx_decisions_type ON decisions(type);
CREATE INDEX IF NOT EXISTS idx_decision_refs_target ON decision_refs(target_id);
.pql/changelog/ticket_history/2026-08.sql
+206
View File
@@ -0,0 +1,206 @@
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'description', NULL, 'DELETE /tasks/{task_name} returns 500 whenever the task has at least one row in task_executions:
psycopg2.errors.ForeignKeyViolation: update or delete on table "scheduled_tasks"
violates foreign key constraint "task_executions_task_id_fkey" on table "task_executions"
DETAIL: Key (id)=(46) is still referenced from table "task_executions".
src/main.py:355. Since every task that has ever fired has execution history, the endpoint works only for tasks that have never run — which is close to none of them. Found on 2026-08-11 while cleaning up a temporary probe task created for T-74; it had to be removed with hand-written SQL against two tables, which is not something the API should require.
The caller gets a bare "Internal Server Error" with no indication that history is the obstacle, so it reads as the service being broken rather than the request being refusable.
Deciding what delete should MEAN is the actual work here, and it should not be guessed:
- cascade — drop the execution history with the task. Simple, and silently destroys the audit trail for a task someone deletes by mistake.
- soft delete — mark it deleted and keep the history. Keeps the audit trail, adds a state every query then has to filter on.
- refuse with 409 and a real message ("task has N executions; pass ?purge=true"). Explicit, and makes the destructive variant a deliberate act.
The third is the smallest correct change and matches how the rest of this system treats destructive operations. Whichever is chosen, a 500 on a foreseeable, well-defined condition is the part that is simply wrong.', NULL, '2026-08-11 09:53:56', '2026-08-11 09:53:56.969', '2026-08-11 09:53:56.969', NULL, '8b2dfce5342a7cd89bbe6d04c03157f1', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'description', NULL, 'get_tasks_for_minute (src/tasks/executor.py:70) excludes any task holding a task_executions row with status=''running'':
AND id NOT IN (SELECT task_id FROM task_executions WHERE status = ''running'')
Nothing ever reconciles that row. It is written before the executor runs and updated after, so a process that dies in between leaves it ''running'' permanently — and the task is then excluded from every future minute, forever, with no error, no alarm and no log line. The task simply stops existing as far as the scheduler is concerned.
This is not hypothetical. test_example_task has held a ''running'' row since 2025-12-07 — 5916 hours. It happens to be disabled, so nothing is broken today; the mechanism is what matters, not this instance.
The exposure is real and daily: Watchtower restarts this container every morning at 4 AM. Any task still running at that moment is permanently unscheduled by it. The config backup runs 03:05 and takes ~21 minutes, which is not far off.
The failure mode is the one this system keeps producing: it looks like nothing. A backup that stops running forever produces no failure — it produces silence, and silence reads as health.
FIX: reconcile at startup. Any row left ''running'' when the process starts cannot be running, because the process that owned it is gone. Mark those ''orphaned'' (or ''failed'' with a message naming the restart) during app startup, before the scheduler begins its first minute. Consider also a stale-row guard for rows older than the task''s timeout_seconds, which covers a killed worker without a restart.
The audit trail is preserved either way — this is a status correction, not a deletion.', NULL, '2026-08-11 09:59:05', '2026-08-11 09:59:05.288', '2026-08-11 09:59:05.288', NULL, '1b14bc4af4acc6df0dc2d21675edf31f', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'description', NULL, 'execute_task has an `except asyncio.TimeoutError` handler (src/tasks/executor.py:179) that records status=''timeout''. It can never run.
_run_executor wraps the awaited call in `except Exception` (line 219) and returns the error as a value. Since Python 3.11 asyncio.TimeoutError IS the builtin TimeoutError, which inherits OSError -> Exception, so the broad handler catches it first and converts it into an ordinary (None, error) tuple. execute_task then sees a non-None error and files status=''failed''.
Verified in the deployed runtime:
asyncio.TimeoutError is TimeoutError: True
MRO: TimeoutError -> OSError -> Exception -> BaseException
And confirmed against 8 months of history — 18,785 executions, and the count of status=''timeout'' rows is zero:
success 16690 | failed 2093 | running 2 | timeout 0
So every timeout since 2025-12-07 has been recorded as a generic failure carrying a traceback. Operationally that erases the distinction that matters most when a job misbehaves: "this job is too slow for its window" and "this job is broken" need different responses, and right now they look identical in the history.
FIX: catch asyncio.TimeoutError explicitly in _run_executor, ahead of the broad handler, and let it propagate (or return a marker execute_task can distinguish). Note the ordering is the whole bug — a narrower except after a broader one is dead code, and there is no linter configured here to say so.
Related: a thread started by asyncio.to_thread cannot be cancelled, so after T-74 a timeout frees the loop while the work continues to completion. Whatever ''timeout'' comes to mean should say so rather than implying the work stopped.', NULL, '2026-08-11 09:59:05', '2026-08-11 09:59:05.545', '2026-08-11 09:59:05.545', NULL, 'af08842306d6326036b1865b8cbf7c58', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'description', 'get_tasks_for_minute (src/tasks/executor.py:70) excludes any task holding a task_executions row with status=''running'':
AND id NOT IN (SELECT task_id FROM task_executions WHERE status = ''running'')
Nothing ever reconciles that row. It is written before the executor runs and updated after, so a process that dies in between leaves it ''running'' permanently — and the task is then excluded from every future minute, forever, with no error, no alarm and no log line. The task simply stops existing as far as the scheduler is concerned.
This is not hypothetical. test_example_task has held a ''running'' row since 2025-12-07 — 5916 hours. It happens to be disabled, so nothing is broken today; the mechanism is what matters, not this instance.
The exposure is real and daily: Watchtower restarts this container every morning at 4 AM. Any task still running at that moment is permanently unscheduled by it. The config backup runs 03:05 and takes ~21 minutes, which is not far off.
The failure mode is the one this system keeps producing: it looks like nothing. A backup that stops running forever produces no failure — it produces silence, and silence reads as health.
FIX: reconcile at startup. Any row left ''running'' when the process starts cannot be running, because the process that owned it is gone. Mark those ''orphaned'' (or ''failed'' with a message naming the restart) during app startup, before the scheduler begins its first minute. Consider also a stale-row guard for rows older than the task''s timeout_seconds, which covers a killed worker without a restart.
The audit trail is preserved either way — this is a status correction, not a deletion.', 'get_tasks_for_minute (src/tasks/executor.py:70) excludes any task holding a task_executions row with status=''running'':
AND id NOT IN (SELECT task_id FROM task_executions WHERE status = ''running'')
Nothing ever reconciles that row. It is written before the executor runs and updated after, so a process that dies in between leaves it ''running'' permanently — and the task is then excluded from every future minute, forever, with no error, no alarm and no log line. The task simply stops existing as far as the scheduler is concerned.
This is not hypothetical. test_example_task has held a ''running'' row since 2025-12-07 — 5916 hours. It happens to be disabled, so nothing is broken today; the mechanism is what matters, not this instance.
The exposure is real and daily: Watchtower restarts this container every morning at 4 AM. Any task still running at that moment is permanently unscheduled by it. The config backup runs 03:05 and takes ~21 minutes, which is not far off.
The failure mode is the one this system keeps producing: it looks like nothing. A backup that stops running forever produces no failure — it produces silence, and silence reads as health.
FIX: reconcile at startup. Any row left ''running'' when the process starts cannot be running, because the process that owned it is gone. Mark those ''orphaned'' (or ''failed'' with a message naming the restart) during app startup, before the scheduler begins its first minute. Consider also a stale-row guard for rows older than the task''s timeout_seconds, which covers a killed worker without a restart.
The audit trail is preserved either way — this is a status correction, not a deletion.
FIXED in v1.6.0 (e45de4f). TaskExecutor.reconcile_orphaned_executions() runs in the lifespan before the scheduler starts.
Proven on the real system rather than in a test. The row that had been ''running'' since 2025-12-07 22:44 was released at startup:
WARNING - Orphaned execution recovered: test_example_task was left ''running''
since 2025-12-07 22:44:00.046695. That task had been excluded from
scheduling until now.
Rows still ''running'' afterwards: 0. Status is ''orphaned'', error names the restart.
''orphaned'' rather than ''failed'' because the outcome is genuinely unknown — a task that completed and never got to update its row looks exactly like one that died halfway, and ''failed'' would assert something nobody observed.
Ordering is part of the fix and is mutation-checked: reconciliation must complete before the first minute is processed, or the first tick still sees the stale rows. Removing the startup call fails the test.
Not extended to a duration-based sweep, deliberately. While the process lives, execute_task''s finally always closes the row out, so a stale row implies a dead owner; a time-based rule would have to distinguish a slow task from a dead one and would eventually close the record of a task that is still working.', NULL, '2026-08-11 10:11:39', '2026-08-11 10:11:39.058', '2026-08-11 10:11:39.058', NULL, '0cec030639664cf075a113be75218e8d', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'status', 'backlog', 'done', NULL, '2026-08-11 10:11:39', '2026-08-11 10:11:39.178', '2026-08-11 10:11:39.178', NULL, 'cd26a078365d91d44e6d284e80da39f5', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'description', 'execute_task has an `except asyncio.TimeoutError` handler (src/tasks/executor.py:179) that records status=''timeout''. It can never run.
_run_executor wraps the awaited call in `except Exception` (line 219) and returns the error as a value. Since Python 3.11 asyncio.TimeoutError IS the builtin TimeoutError, which inherits OSError -> Exception, so the broad handler catches it first and converts it into an ordinary (None, error) tuple. execute_task then sees a non-None error and files status=''failed''.
Verified in the deployed runtime:
asyncio.TimeoutError is TimeoutError: True
MRO: TimeoutError -> OSError -> Exception -> BaseException
And confirmed against 8 months of history — 18,785 executions, and the count of status=''timeout'' rows is zero:
success 16690 | failed 2093 | running 2 | timeout 0
So every timeout since 2025-12-07 has been recorded as a generic failure carrying a traceback. Operationally that erases the distinction that matters most when a job misbehaves: "this job is too slow for its window" and "this job is broken" need different responses, and right now they look identical in the history.
FIX: catch asyncio.TimeoutError explicitly in _run_executor, ahead of the broad handler, and let it propagate (or return a marker execute_task can distinguish). Note the ordering is the whole bug — a narrower except after a broader one is dead code, and there is no linter configured here to say so.
Related: a thread started by asyncio.to_thread cannot be cancelled, so after T-74 a timeout frees the loop while the work continues to completion. Whatever ''timeout'' comes to mean should say so rather than implying the work stopped.', 'execute_task has an `except asyncio.TimeoutError` handler (src/tasks/executor.py:179) that records status=''timeout''. It can never run.
_run_executor wraps the awaited call in `except Exception` (line 219) and returns the error as a value. Since Python 3.11 asyncio.TimeoutError IS the builtin TimeoutError, which inherits OSError -> Exception, so the broad handler catches it first and converts it into an ordinary (None, error) tuple. execute_task then sees a non-None error and files status=''failed''.
Verified in the deployed runtime:
asyncio.TimeoutError is TimeoutError: True
MRO: TimeoutError -> OSError -> Exception -> BaseException
And confirmed against 8 months of history — 18,785 executions, and the count of status=''timeout'' rows is zero:
success 16690 | failed 2093 | running 2 | timeout 0
So every timeout since 2025-12-07 has been recorded as a generic failure carrying a traceback. Operationally that erases the distinction that matters most when a job misbehaves: "this job is too slow for its window" and "this job is broken" need different responses, and right now they look identical in the history.
FIX: catch asyncio.TimeoutError explicitly in _run_executor, ahead of the broad handler, and let it propagate (or return a marker execute_task can distinguish). Note the ordering is the whole bug — a narrower except after a broader one is dead code, and there is no linter configured here to say so.
Related: a thread started by asyncio.to_thread cannot be cancelled, so after T-74 a timeout frees the loop while the work continues to completion. Whatever ''timeout'' comes to mean should say so rather than implying the work stopped.
FIXED in v1.6.0 (e45de4f). _run_executor now catches asyncio.TimeoutError ahead of the broad handler and re-raises, so execute_task''s timeout branch is reachable for the first time.
A trap surfaced while fixing it, and it is the interesting part. While the branch was unreachable, a timeout travelled the NORMAL path — which does update scheduled_tasks. Making the branch reachable without adding that write would have swapped a wrong status (''failed'') for a stale one (last_status showing the previous run''s outcome indefinitely). _update_task_outcome now mirrors terminal outcomes onto the parent row. A fix that repairs the reported symptom while breaking something adjacent is worse than the bug.
The timeout message also records that the work may still be running: after T-74 executors are handed to asyncio.to_thread, and a thread cannot be cancelled, so wait_for frees the loop while the work continues to completion. Saying "timed out" without that would imply the work stopped.
Mutation-checked: removing the narrow except clause fails the propagation test. A third test pins that ordinary exceptions are still returned rather than raised, so the narrow clause cannot start swallowing anything else.', NULL, '2026-08-11 10:11:39', '2026-08-11 10:11:39.291', '2026-08-11 10:11:39.291', NULL, '8bdb93fdcfbc234e18f3d034ddf25277', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'status', 'backlog', 'done', NULL, '2026-08-11 10:11:39', '2026-08-11 10:11:39.421', '2026-08-11 10:11:39.421', NULL, '7bc24ec03ed40ccacd71d7c7269d05af', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0PNFBDB743QJQ9MF38MPJ8', 'description', NULL, 'DONE in v1.7.0 (538bfe5). Verified on a real row:
12:24:27 ok task=backup_portainer_daily source=scheduler/portainer_backup_executor
WHY, and why not the other fix. A row written by a failing probe on 2026-08-11 was byte-for-byte what a nightly backup failure would have written — same source, same domain, nothing naming the task. The reflex was to delete the inconvenient row; that was refused as audit tampering and the refusal was right twice over, because the row is TRUE (a run did fail) and the defect is that it cannot say which run. Attribution keeps the record intact and makes it answer the question.
MECHANISM. A ContextVar (src/task_context.py) set by the engine around executor invocation, read by health_report. Not an argument: executors are invoked as execute(config, settings), there are ten, and several are dormant — they exist only as a string in a database row and become live the moment someone inserts a task naming them. A signature change would leave those broken in a way no import, grep or test would reveal. Not injected into `config` either: config is what a human wrote in the task definition and an executor may legitimately reject unknown keys.
TWO PROPERTIES VERIFIED IN THE DEPLOYED RUNTIME rather than assumed:
- asyncio.to_thread propagates context, so attribution survives the worker thread T-74 introduced. Had it not, every row from a real executor would have lost its task while unit tests kept passing — hence a test that goes through report_async specifically.
- Each asyncio Task gets its own copy, so five concurrent executions cannot read each other''s. The isolation test yields mid-execution to force interleaving; without that it would pass against a shared global.
- A plain await does NOT get its own copy and leaks to the caller. Both entry points use create_task, but task_scope resets by token rather than relying on it.
Omitted rather than nulled when absent — report() is callable from a script, and a null would claim a task existed with no name.
The one pre-existing critical row still carries no task, which now identifies it: it is the only backup row without the field, so it is legible as pre-attribution rather than ambiguous.', NULL, '2026-08-11 10:25:02', '2026-08-11 10:25:02.952', '2026-08-11 10:25:02.952', NULL, 'ede9b41ea9849e08202b7aae16e9a54c', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0PNFBDB743QJQ9MF38MPJ8', 'status', 'backlog', 'done', NULL, '2026-08-11 10:25:03', '2026-08-11 10:25:03.085', '2026-08-11 10:25:03.085', NULL, 'b45fb340bf78c6c7e4287df001d01456', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'description', 'DELETE /tasks/{task_name} returns 500 whenever the task has at least one row in task_executions:
psycopg2.errors.ForeignKeyViolation: update or delete on table "scheduled_tasks"
violates foreign key constraint "task_executions_task_id_fkey" on table "task_executions"
DETAIL: Key (id)=(46) is still referenced from table "task_executions".
src/main.py:355. Since every task that has ever fired has execution history, the endpoint works only for tasks that have never run — which is close to none of them. Found on 2026-08-11 while cleaning up a temporary probe task created for T-74; it had to be removed with hand-written SQL against two tables, which is not something the API should require.
The caller gets a bare "Internal Server Error" with no indication that history is the obstacle, so it reads as the service being broken rather than the request being refusable.
Deciding what delete should MEAN is the actual work here, and it should not be guessed:
- cascade — drop the execution history with the task. Simple, and silently destroys the audit trail for a task someone deletes by mistake.
- soft delete — mark it deleted and keep the history. Keeps the audit trail, adds a state every query then has to filter on.
- refuse with 409 and a real message ("task has N executions; pass ?purge=true"). Explicit, and makes the destructive variant a deliberate act.
The third is the smallest correct change and matches how the rest of this system treats destructive operations. Whichever is chosen, a 500 on a foreseeable, well-defined condition is the part that is simply wrong.', 'DELETE /tasks/{task_name} returns 500 whenever the task has at least one row in task_executions:
psycopg2.errors.ForeignKeyViolation: update or delete on table "scheduled_tasks"
violates foreign key constraint "task_executions_task_id_fkey" on table "task_executions"
DETAIL: Key (id)=(46) is still referenced from table "task_executions".
src/main.py:355. Since every task that has ever fired has execution history, the endpoint works only for tasks that have never run — which is close to none of them. Found on 2026-08-11 while cleaning up a temporary probe task created for T-74; it had to be removed with hand-written SQL against two tables, which is not something the API should require.
The caller gets a bare "Internal Server Error" with no indication that history is the obstacle, so it reads as the service being broken rather than the request being refusable.
Deciding what delete should MEAN is the actual work here, and it should not be guessed:
- cascade — drop the execution history with the task. Simple, and silently destroys the audit trail for a task someone deletes by mistake.
- soft delete — mark it deleted and keep the history. Keeps the audit trail, adds a state every query then has to filter on.
- refuse with 409 and a real message ("task has N executions; pass ?purge=true"). Explicit, and makes the destructive variant a deliberate act.
The third is the smallest correct change and matches how the rest of this system treats destructive operations. Whichever is chosen, a 500 on a foreseeable, well-defined condition is the part that is simply wrong.
FIXED in v1.8.0 (c34db66). 409 with a message that can be acted on; ?purge=true proceeds.
Verified against the live service with a throwaway task that had one execution row:
DELETE /tasks/t1_delete_probe2 -> HTTP 409
Task ''t1_delete_probe2'' has 1 execution record(s). Deleting it would discard
that history. Re-send with ?purge=true to delete the task and its history
together, or PUT enabled=false to stop it running while keeping the record.
task still present afterwards: 1 (the refusal deleted nothing)
DELETE /tasks/t1_delete_probe2?purge=true -> HTTP 200
{"message":"...deleted successfully","executions_purged":1}
tasks: 24, probe execution rows: 0
REFUSE RATHER THAN CASCADE, chosen for asymmetry of recovery: a task definition can be recreated from the API in one call, its execution history cannot be recreated at all. Defaulting to the destructive reading of an ambiguous request is how audit trails disappear quietly.
The message carries the three things a caller needs — how much history is at stake, the flag that proceeds, and PUT enabled=false, which is usually what was actually wanted since it stops the task running and keeps the record. A bare "conflict" would be little better than the 500 it replaced.
History and task are deleted in ONE transaction. Split across two, a failure between them leaves the audit trail gone and the task alive: the worst of both outcomes.
Mutation-checked: removing the guard fails the refusal tests. Separate tests pin that a refused delete issues no DELETE at all, and that ?purge=true against a missing task is still 404 rather than a success.
NOTE: the commit for this work is missing its Co-Authored-By trailer — I wrote the message file without it. Already pushed, and fixing it would require rewriting published history on main, so it stands as-is.', NULL, '2026-08-11 10:31:46', '2026-08-11 10:31:46.465', '2026-08-11 10:31:46.465', NULL, 'efc9df5b52ac92d63e5ae5a60c2bdef8', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'status', 'backlog', 'done', NULL, '2026-08-11 10:31:46', '2026-08-11 10:31:46.598', '2026-08-11 10:31:46.598', NULL, '7bad13e92e65eb912519a498a882faee', 2) ON CONFLICT(hash) DO NOTHING;
.pql/changelog/ticket_idmap/0000-schema.sql
+139
View File
@@ -0,0 +1,139 @@
-- Auto-generated by pql init. CREATE TABLE statements
-- for the planning schema; per-table dir keeps the changelog
-- self-describing per D-15. CREATE TABLE IF NOT EXISTS is
-- idempotent so running schema files from each directory in
-- replay order is harmless.
--
-- Importer parses the markers below to detect schema drift
-- between the producing pql version and the local one — a
-- bumped canonical_version means projection rules changed
-- and replay must refuse rather than silently corrupt state.
-- pql:created_by: 2.2.0
-- pql:canonical_version: 2
CREATE TABLE IF NOT EXISTS decisions (
id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('confirmed','question','rejected')),
domain TEXT NOT NULL,
title TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'active'
CHECK(status IN ('active','superseded','resolved','open')),
date TEXT,
file_path TEXT NOT NULL,
synced_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS decision_refs (
source_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
target_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
ref_type TEXT NOT NULL
CHECK(ref_type IN ('supersedes','references','resolves','depends_on','amends')),
note TEXT,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (source_id, target_id, ref_type)
);
-- Identity split (D-26): a ticket's stable, collision-proof identity is its
-- record_id (a locally-generated ULID, planning.NewRecordID); the friendly
-- T-NNN label lives in ticket_idmap and may be reconciled. Every structural
-- reference (parent, deps, history, labels) targets record_id, so a label
-- clash never corrupts the graph — only ticket_idmap needs a relabel.
CREATE TABLE IF NOT EXISTS tickets (
record_id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('initiative','epic','story','task','bug')),
parent_record_id TEXT REFERENCES tickets(record_id),
title TEXT NOT NULL,
description TEXT,
-- No CHECK enumeration: the ticket status vocabulary is per-vault
-- configurable (ticket_statuses in .pql/config.yaml). Validation lives
-- in Go (planning.StatusSet), so adding/renaming statuses needs no
-- schema change. The DEFAULT is a harmless fallback — CreateTicket
-- always inserts the configured default explicitly.
status TEXT NOT NULL DEFAULT 'backlog',
priority TEXT DEFAULT 'medium'
CHECK(priority IN ('critical','high','medium','low')),
assigned_to TEXT,
team TEXT,
decision_ref TEXT REFERENCES decisions(id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
-- ticket_idmap maps a record_id to its current friendly label (T-NNN).
-- ticket_id is intentionally NOT globally unique: two uncoordinated clones
-- can mint the same label, which surfaces as a duplicate-label collision
-- (detected at replay) and is fixed with "pql ticket relabel".
CREATE TABLE IF NOT EXISTS ticket_idmap (
record_id TEXT PRIMARY KEY REFERENCES tickets(record_id),
ticket_id TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_deps (
blocker_record_id TEXT NOT NULL REFERENCES tickets(record_id),
blocked_record_id TEXT NOT NULL REFERENCES tickets(record_id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (blocker_record_id, blocked_record_id)
);
CREATE TABLE IF NOT EXISTS ticket_history (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
field TEXT NOT NULL,
old_value TEXT,
new_value TEXT,
changed_by TEXT,
changed_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT UNIQUE,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_labels (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
label TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (ticket_record_id, label)
);
CREATE TABLE IF NOT EXISTS meta (
key TEXT PRIMARY KEY,
value TEXT NOT NULL,
updated_at TEXT NOT NULL DEFAULT (datetime('now'))
);
CREATE INDEX IF NOT EXISTS idx_tickets_status ON tickets(status);
CREATE INDEX IF NOT EXISTS idx_tickets_team ON tickets(team);
CREATE INDEX IF NOT EXISTS idx_tickets_decision_ref ON tickets(decision_ref);
CREATE INDEX IF NOT EXISTS idx_tickets_assigned ON tickets(assigned_to);
CREATE INDEX IF NOT EXISTS idx_tickets_parent ON tickets(parent_record_id);
CREATE INDEX IF NOT EXISTS idx_ticket_idmap_label ON ticket_idmap(ticket_id);
CREATE INDEX IF NOT EXISTS idx_decisions_domain ON decisions(domain);
CREATE INDEX IF NOT EXISTS idx_decisions_type ON decisions(type);
CREATE INDEX IF NOT EXISTS idx_decision_refs_target ON decision_refs(target_id);
.pql/changelog/ticket_idmap/2026-08.sql
+4
View File
@@ -0,0 +1,4 @@
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'T-1', '2026-08-11 09:53:56.835', '2026-08-11 09:53:56.835', NULL, '3d980945e785a6bc7ca8fcaa8250e22b', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'T-2', '2026-08-11 09:59:05.153', '2026-08-11 09:59:05.153', NULL, 'c8655ec9601ba93fe822395329e62262', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'T-3', '2026-08-11 09:59:05.427', '2026-08-11 09:59:05.427', NULL, '3bce28c3d803d3f5036cfbb1ac11969c', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0PNFBDB743QJQ9MF38MPJ8', 'T-4', '2026-08-11 10:25:02.811', '2026-08-11 10:25:02.811', NULL, 'eb8c6e4088b39b797541c1be0ee313fd', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
.pql/changelog/ticket_labels/0000-schema.sql
+139
View File
@@ -0,0 +1,139 @@
-- Auto-generated by pql init. CREATE TABLE statements
-- for the planning schema; per-table dir keeps the changelog
-- self-describing per D-15. CREATE TABLE IF NOT EXISTS is
-- idempotent so running schema files from each directory in
-- replay order is harmless.
--
-- Importer parses the markers below to detect schema drift
-- between the producing pql version and the local one — a
-- bumped canonical_version means projection rules changed
-- and replay must refuse rather than silently corrupt state.
-- pql:created_by: 2.2.0
-- pql:canonical_version: 2
CREATE TABLE IF NOT EXISTS decisions (
id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('confirmed','question','rejected')),
domain TEXT NOT NULL,
title TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'active'
CHECK(status IN ('active','superseded','resolved','open')),
date TEXT,
file_path TEXT NOT NULL,
synced_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS decision_refs (
source_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
target_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
ref_type TEXT NOT NULL
CHECK(ref_type IN ('supersedes','references','resolves','depends_on','amends')),
note TEXT,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (source_id, target_id, ref_type)
);
-- Identity split (D-26): a ticket's stable, collision-proof identity is its
-- record_id (a locally-generated ULID, planning.NewRecordID); the friendly
-- T-NNN label lives in ticket_idmap and may be reconciled. Every structural
-- reference (parent, deps, history, labels) targets record_id, so a label
-- clash never corrupts the graph — only ticket_idmap needs a relabel.
CREATE TABLE IF NOT EXISTS tickets (
record_id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('initiative','epic','story','task','bug')),
parent_record_id TEXT REFERENCES tickets(record_id),
title TEXT NOT NULL,
description TEXT,
-- No CHECK enumeration: the ticket status vocabulary is per-vault
-- configurable (ticket_statuses in .pql/config.yaml). Validation lives
-- in Go (planning.StatusSet), so adding/renaming statuses needs no
-- schema change. The DEFAULT is a harmless fallback — CreateTicket
-- always inserts the configured default explicitly.
status TEXT NOT NULL DEFAULT 'backlog',
priority TEXT DEFAULT 'medium'
CHECK(priority IN ('critical','high','medium','low')),
assigned_to TEXT,
team TEXT,
decision_ref TEXT REFERENCES decisions(id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
-- ticket_idmap maps a record_id to its current friendly label (T-NNN).
-- ticket_id is intentionally NOT globally unique: two uncoordinated clones
-- can mint the same label, which surfaces as a duplicate-label collision
-- (detected at replay) and is fixed with "pql ticket relabel".
CREATE TABLE IF NOT EXISTS ticket_idmap (
record_id TEXT PRIMARY KEY REFERENCES tickets(record_id),
ticket_id TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_deps (
blocker_record_id TEXT NOT NULL REFERENCES tickets(record_id),
blocked_record_id TEXT NOT NULL REFERENCES tickets(record_id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (blocker_record_id, blocked_record_id)
);
CREATE TABLE IF NOT EXISTS ticket_history (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
field TEXT NOT NULL,
old_value TEXT,
new_value TEXT,
changed_by TEXT,
changed_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT UNIQUE,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_labels (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
label TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (ticket_record_id, label)
);
CREATE TABLE IF NOT EXISTS meta (
key TEXT PRIMARY KEY,
value TEXT NOT NULL,
updated_at TEXT NOT NULL DEFAULT (datetime('now'))
);
CREATE INDEX IF NOT EXISTS idx_tickets_status ON tickets(status);
CREATE INDEX IF NOT EXISTS idx_tickets_team ON tickets(team);
CREATE INDEX IF NOT EXISTS idx_tickets_decision_ref ON tickets(decision_ref);
CREATE INDEX IF NOT EXISTS idx_tickets_assigned ON tickets(assigned_to);
CREATE INDEX IF NOT EXISTS idx_tickets_parent ON tickets(parent_record_id);
CREATE INDEX IF NOT EXISTS idx_ticket_idmap_label ON ticket_idmap(ticket_id);
CREATE INDEX IF NOT EXISTS idx_decisions_domain ON decisions(domain);
CREATE INDEX IF NOT EXISTS idx_decisions_type ON decisions(type);
CREATE INDEX IF NOT EXISTS idx_decision_refs_target ON decision_refs(target_id);
.pql/changelog/tickets/0000-schema.sql
+139
View File
@@ -0,0 +1,139 @@
-- Auto-generated by pql init. CREATE TABLE statements
-- for the planning schema; per-table dir keeps the changelog
-- self-describing per D-15. CREATE TABLE IF NOT EXISTS is
-- idempotent so running schema files from each directory in
-- replay order is harmless.
--
-- Importer parses the markers below to detect schema drift
-- between the producing pql version and the local one — a
-- bumped canonical_version means projection rules changed
-- and replay must refuse rather than silently corrupt state.
-- pql:created_by: 2.2.0
-- pql:canonical_version: 2
CREATE TABLE IF NOT EXISTS decisions (
id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('confirmed','question','rejected')),
domain TEXT NOT NULL,
title TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'active'
CHECK(status IN ('active','superseded','resolved','open')),
date TEXT,
file_path TEXT NOT NULL,
synced_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS decision_refs (
source_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
target_id TEXT NOT NULL REFERENCES decisions(id) ON DELETE CASCADE,
ref_type TEXT NOT NULL
CHECK(ref_type IN ('supersedes','references','resolves','depends_on','amends')),
note TEXT,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (source_id, target_id, ref_type)
);
-- Identity split (D-26): a ticket's stable, collision-proof identity is its
-- record_id (a locally-generated ULID, planning.NewRecordID); the friendly
-- T-NNN label lives in ticket_idmap and may be reconciled. Every structural
-- reference (parent, deps, history, labels) targets record_id, so a label
-- clash never corrupts the graph — only ticket_idmap needs a relabel.
CREATE TABLE IF NOT EXISTS tickets (
record_id TEXT PRIMARY KEY,
type TEXT NOT NULL CHECK(type IN ('initiative','epic','story','task','bug')),
parent_record_id TEXT REFERENCES tickets(record_id),
title TEXT NOT NULL,
description TEXT,
-- No CHECK enumeration: the ticket status vocabulary is per-vault
-- configurable (ticket_statuses in .pql/config.yaml). Validation lives
-- in Go (planning.StatusSet), so adding/renaming statuses needs no
-- schema change. The DEFAULT is a harmless fallback — CreateTicket
-- always inserts the configured default explicitly.
status TEXT NOT NULL DEFAULT 'backlog',
priority TEXT DEFAULT 'medium'
CHECK(priority IN ('critical','high','medium','low')),
assigned_to TEXT,
team TEXT,
decision_ref TEXT REFERENCES decisions(id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
-- ticket_idmap maps a record_id to its current friendly label (T-NNN).
-- ticket_id is intentionally NOT globally unique: two uncoordinated clones
-- can mint the same label, which surfaces as a duplicate-label collision
-- (detected at replay) and is fixed with "pql ticket relabel".
CREATE TABLE IF NOT EXISTS ticket_idmap (
record_id TEXT PRIMARY KEY REFERENCES tickets(record_id),
ticket_id TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_deps (
blocker_record_id TEXT NOT NULL REFERENCES tickets(record_id),
blocked_record_id TEXT NOT NULL REFERENCES tickets(record_id),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (blocker_record_id, blocked_record_id)
);
CREATE TABLE IF NOT EXISTS ticket_history (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
field TEXT NOT NULL,
old_value TEXT,
new_value TEXT,
changed_by TEXT,
changed_at TEXT NOT NULL DEFAULT (datetime('now')),
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT UNIQUE,
canonical_version INTEGER
);
CREATE TABLE IF NOT EXISTS ticket_labels (
ticket_record_id TEXT NOT NULL REFERENCES tickets(record_id),
label TEXT NOT NULL,
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
deleted_at TEXT,
hash TEXT,
canonical_version INTEGER,
PRIMARY KEY (ticket_record_id, label)
);
CREATE TABLE IF NOT EXISTS meta (
key TEXT PRIMARY KEY,
value TEXT NOT NULL,
updated_at TEXT NOT NULL DEFAULT (datetime('now'))
);
CREATE INDEX IF NOT EXISTS idx_tickets_status ON tickets(status);
CREATE INDEX IF NOT EXISTS idx_tickets_team ON tickets(team);
CREATE INDEX IF NOT EXISTS idx_tickets_decision_ref ON tickets(decision_ref);
CREATE INDEX IF NOT EXISTS idx_tickets_assigned ON tickets(assigned_to);
CREATE INDEX IF NOT EXISTS idx_tickets_parent ON tickets(parent_record_id);
CREATE INDEX IF NOT EXISTS idx_ticket_idmap_label ON ticket_idmap(ticket_id);
CREATE INDEX IF NOT EXISTS idx_decisions_domain ON decisions(domain);
CREATE INDEX IF NOT EXISTS idx_decisions_type ON decisions(type);
CREATE INDEX IF NOT EXISTS idx_decision_refs_target ON decision_refs(target_id);
.pql/changelog/tickets/2026-08.sql
+273
View File
@@ -0,0 +1,273 @@
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'bug', NULL, 'DELETE /tasks/{name} 500s for any task that has ever run', NULL, 'backlog', 'high', NULL, NULL, NULL, '2026-08-11 09:53:56.826', '2026-08-11 09:53:56.826', NULL, '86310746d98f53b722c7336b0bab980a', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'bug', NULL, 'DELETE /tasks/{name} 500s for any task that has ever run', 'DELETE /tasks/{task_name} returns 500 whenever the task has at least one row in task_executions:
psycopg2.errors.ForeignKeyViolation: update or delete on table "scheduled_tasks"
violates foreign key constraint "task_executions_task_id_fkey" on table "task_executions"
DETAIL: Key (id)=(46) is still referenced from table "task_executions".
src/main.py:355. Since every task that has ever fired has execution history, the endpoint works only for tasks that have never run — which is close to none of them. Found on 2026-08-11 while cleaning up a temporary probe task created for T-74; it had to be removed with hand-written SQL against two tables, which is not something the API should require.
The caller gets a bare "Internal Server Error" with no indication that history is the obstacle, so it reads as the service being broken rather than the request being refusable.
Deciding what delete should MEAN is the actual work here, and it should not be guessed:
- cascade — drop the execution history with the task. Simple, and silently destroys the audit trail for a task someone deletes by mistake.
- soft delete — mark it deleted and keep the history. Keeps the audit trail, adds a state every query then has to filter on.
- refuse with 409 and a real message ("task has N executions; pass ?purge=true"). Explicit, and makes the destructive variant a deliberate act.
The third is the smallest correct change and matches how the rest of this system treats destructive operations. Whichever is chosen, a 500 on a foreseeable, well-defined condition is the part that is simply wrong.', 'backlog', 'high', NULL, NULL, NULL, '2026-08-11 09:53:56.826', '2026-08-11 09:53:56.969', NULL, '248bb53ca2bae0f12fabd9d88a1ca171', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'bug', NULL, 'A restart mid-task unschedules that task forever, silently', NULL, 'backlog', 'high', NULL, NULL, NULL, '2026-08-11 09:59:05.153', '2026-08-11 09:59:05.153', NULL, '0bda80455ba51ea4cb0cb91e302ac216', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'bug', NULL, 'A restart mid-task unschedules that task forever, silently', 'get_tasks_for_minute (src/tasks/executor.py:70) excludes any task holding a task_executions row with status=''running'':
AND id NOT IN (SELECT task_id FROM task_executions WHERE status = ''running'')
Nothing ever reconciles that row. It is written before the executor runs and updated after, so a process that dies in between leaves it ''running'' permanently — and the task is then excluded from every future minute, forever, with no error, no alarm and no log line. The task simply stops existing as far as the scheduler is concerned.
This is not hypothetical. test_example_task has held a ''running'' row since 2025-12-07 — 5916 hours. It happens to be disabled, so nothing is broken today; the mechanism is what matters, not this instance.
The exposure is real and daily: Watchtower restarts this container every morning at 4 AM. Any task still running at that moment is permanently unscheduled by it. The config backup runs 03:05 and takes ~21 minutes, which is not far off.
The failure mode is the one this system keeps producing: it looks like nothing. A backup that stops running forever produces no failure — it produces silence, and silence reads as health.
FIX: reconcile at startup. Any row left ''running'' when the process starts cannot be running, because the process that owned it is gone. Mark those ''orphaned'' (or ''failed'' with a message naming the restart) during app startup, before the scheduler begins its first minute. Consider also a stale-row guard for rows older than the task''s timeout_seconds, which covers a killed worker without a restart.
The audit trail is preserved either way — this is a status correction, not a deletion.', 'backlog', 'high', NULL, NULL, NULL, '2026-08-11 09:59:05.153', '2026-08-11 09:59:05.288', NULL, '23a72bff9ffc3592c796741df8a7232e', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'bug', NULL, 'The ''timeout'' execution status is unreachable; timeouts are filed as generic failures', NULL, 'backlog', 'medium', NULL, NULL, NULL, '2026-08-11 09:59:05.427', '2026-08-11 09:59:05.427', NULL, '31a32d6dce87263657892d48552d1f0a', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'bug', NULL, 'The ''timeout'' execution status is unreachable; timeouts are filed as generic failures', 'execute_task has an `except asyncio.TimeoutError` handler (src/tasks/executor.py:179) that records status=''timeout''. It can never run.
_run_executor wraps the awaited call in `except Exception` (line 219) and returns the error as a value. Since Python 3.11 asyncio.TimeoutError IS the builtin TimeoutError, which inherits OSError -> Exception, so the broad handler catches it first and converts it into an ordinary (None, error) tuple. execute_task then sees a non-None error and files status=''failed''.
Verified in the deployed runtime:
asyncio.TimeoutError is TimeoutError: True
MRO: TimeoutError -> OSError -> Exception -> BaseException
And confirmed against 8 months of history — 18,785 executions, and the count of status=''timeout'' rows is zero:
success 16690 | failed 2093 | running 2 | timeout 0
So every timeout since 2025-12-07 has been recorded as a generic failure carrying a traceback. Operationally that erases the distinction that matters most when a job misbehaves: "this job is too slow for its window" and "this job is broken" need different responses, and right now they look identical in the history.
FIX: catch asyncio.TimeoutError explicitly in _run_executor, ahead of the broad handler, and let it propagate (or return a marker execute_task can distinguish). Note the ordering is the whole bug — a narrower except after a broader one is dead code, and there is no linter configured here to say so.
Related: a thread started by asyncio.to_thread cannot be cancelled, so after T-74 a timeout frees the loop while the work continues to completion. Whatever ''timeout'' comes to mean should say so rather than implying the work stopped.', 'backlog', 'medium', NULL, NULL, NULL, '2026-08-11 09:59:05.427', '2026-08-11 09:59:05.545', NULL, 'ae0db866086e38b681b0ea32837df277', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'bug', NULL, 'A restart mid-task unschedules that task forever, silently', 'get_tasks_for_minute (src/tasks/executor.py:70) excludes any task holding a task_executions row with status=''running'':
AND id NOT IN (SELECT task_id FROM task_executions WHERE status = ''running'')
Nothing ever reconciles that row. It is written before the executor runs and updated after, so a process that dies in between leaves it ''running'' permanently — and the task is then excluded from every future minute, forever, with no error, no alarm and no log line. The task simply stops existing as far as the scheduler is concerned.
This is not hypothetical. test_example_task has held a ''running'' row since 2025-12-07 — 5916 hours. It happens to be disabled, so nothing is broken today; the mechanism is what matters, not this instance.
The exposure is real and daily: Watchtower restarts this container every morning at 4 AM. Any task still running at that moment is permanently unscheduled by it. The config backup runs 03:05 and takes ~21 minutes, which is not far off.
The failure mode is the one this system keeps producing: it looks like nothing. A backup that stops running forever produces no failure — it produces silence, and silence reads as health.
FIX: reconcile at startup. Any row left ''running'' when the process starts cannot be running, because the process that owned it is gone. Mark those ''orphaned'' (or ''failed'' with a message naming the restart) during app startup, before the scheduler begins its first minute. Consider also a stale-row guard for rows older than the task''s timeout_seconds, which covers a killed worker without a restart.
The audit trail is preserved either way — this is a status correction, not a deletion.
FIXED in v1.6.0 (e45de4f). TaskExecutor.reconcile_orphaned_executions() runs in the lifespan before the scheduler starts.
Proven on the real system rather than in a test. The row that had been ''running'' since 2025-12-07 22:44 was released at startup:
WARNING - Orphaned execution recovered: test_example_task was left ''running''
since 2025-12-07 22:44:00.046695. That task had been excluded from
scheduling until now.
Rows still ''running'' afterwards: 0. Status is ''orphaned'', error names the restart.
''orphaned'' rather than ''failed'' because the outcome is genuinely unknown — a task that completed and never got to update its row looks exactly like one that died halfway, and ''failed'' would assert something nobody observed.
Ordering is part of the fix and is mutation-checked: reconciliation must complete before the first minute is processed, or the first tick still sees the stale rows. Removing the startup call fails the test.
Not extended to a duration-based sweep, deliberately. While the process lives, execute_task''s finally always closes the row out, so a stale row implies a dead owner; a time-based rule would have to distinguish a slow task from a dead one and would eventually close the record of a task that is still working.', 'backlog', 'high', NULL, NULL, NULL, '2026-08-11 09:59:05.153', '2026-08-11 10:11:39.058', NULL, '8d88d4684f803319131765d97772e64d', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQAR5KXM3PTG0QT3R1PV8', 'bug', NULL, 'A restart mid-task unschedules that task forever, silently', 'get_tasks_for_minute (src/tasks/executor.py:70) excludes any task holding a task_executions row with status=''running'':
AND id NOT IN (SELECT task_id FROM task_executions WHERE status = ''running'')
Nothing ever reconciles that row. It is written before the executor runs and updated after, so a process that dies in between leaves it ''running'' permanently — and the task is then excluded from every future minute, forever, with no error, no alarm and no log line. The task simply stops existing as far as the scheduler is concerned.
This is not hypothetical. test_example_task has held a ''running'' row since 2025-12-07 — 5916 hours. It happens to be disabled, so nothing is broken today; the mechanism is what matters, not this instance.
The exposure is real and daily: Watchtower restarts this container every morning at 4 AM. Any task still running at that moment is permanently unscheduled by it. The config backup runs 03:05 and takes ~21 minutes, which is not far off.
The failure mode is the one this system keeps producing: it looks like nothing. A backup that stops running forever produces no failure — it produces silence, and silence reads as health.
FIX: reconcile at startup. Any row left ''running'' when the process starts cannot be running, because the process that owned it is gone. Mark those ''orphaned'' (or ''failed'' with a message naming the restart) during app startup, before the scheduler begins its first minute. Consider also a stale-row guard for rows older than the task''s timeout_seconds, which covers a killed worker without a restart.
The audit trail is preserved either way — this is a status correction, not a deletion.
FIXED in v1.6.0 (e45de4f). TaskExecutor.reconcile_orphaned_executions() runs in the lifespan before the scheduler starts.
Proven on the real system rather than in a test. The row that had been ''running'' since 2025-12-07 22:44 was released at startup:
WARNING - Orphaned execution recovered: test_example_task was left ''running''
since 2025-12-07 22:44:00.046695. That task had been excluded from
scheduling until now.
Rows still ''running'' afterwards: 0. Status is ''orphaned'', error names the restart.
''orphaned'' rather than ''failed'' because the outcome is genuinely unknown — a task that completed and never got to update its row looks exactly like one that died halfway, and ''failed'' would assert something nobody observed.
Ordering is part of the fix and is mutation-checked: reconciliation must complete before the first minute is processed, or the first tick still sees the stale rows. Removing the startup call fails the test.
Not extended to a duration-based sweep, deliberately. While the process lives, execute_task''s finally always closes the row out, so a stale row implies a dead owner; a time-based rule would have to distinguish a slow task from a dead one and would eventually close the record of a task that is still working.', 'done', 'high', NULL, NULL, NULL, '2026-08-11 09:59:05.153', '2026-08-11 10:11:39.177', NULL, '99284d5f4a53a35637ee10c7b6b66c6e', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'bug', NULL, 'The ''timeout'' execution status is unreachable; timeouts are filed as generic failures', 'execute_task has an `except asyncio.TimeoutError` handler (src/tasks/executor.py:179) that records status=''timeout''. It can never run.
_run_executor wraps the awaited call in `except Exception` (line 219) and returns the error as a value. Since Python 3.11 asyncio.TimeoutError IS the builtin TimeoutError, which inherits OSError -> Exception, so the broad handler catches it first and converts it into an ordinary (None, error) tuple. execute_task then sees a non-None error and files status=''failed''.
Verified in the deployed runtime:
asyncio.TimeoutError is TimeoutError: True
MRO: TimeoutError -> OSError -> Exception -> BaseException
And confirmed against 8 months of history — 18,785 executions, and the count of status=''timeout'' rows is zero:
success 16690 | failed 2093 | running 2 | timeout 0
So every timeout since 2025-12-07 has been recorded as a generic failure carrying a traceback. Operationally that erases the distinction that matters most when a job misbehaves: "this job is too slow for its window" and "this job is broken" need different responses, and right now they look identical in the history.
FIX: catch asyncio.TimeoutError explicitly in _run_executor, ahead of the broad handler, and let it propagate (or return a marker execute_task can distinguish). Note the ordering is the whole bug — a narrower except after a broader one is dead code, and there is no linter configured here to say so.
Related: a thread started by asyncio.to_thread cannot be cancelled, so after T-74 a timeout frees the loop while the work continues to completion. Whatever ''timeout'' comes to mean should say so rather than implying the work stopped.
FIXED in v1.6.0 (e45de4f). _run_executor now catches asyncio.TimeoutError ahead of the broad handler and re-raises, so execute_task''s timeout branch is reachable for the first time.
A trap surfaced while fixing it, and it is the interesting part. While the branch was unreachable, a timeout travelled the NORMAL path — which does update scheduled_tasks. Making the branch reachable without adding that write would have swapped a wrong status (''failed'') for a stale one (last_status showing the previous run''s outcome indefinitely). _update_task_outcome now mirrors terminal outcomes onto the parent row. A fix that repairs the reported symptom while breaking something adjacent is worse than the bug.
The timeout message also records that the work may still be running: after T-74 executors are handed to asyncio.to_thread, and a thread cannot be cancelled, so wait_for frees the loop while the work continues to completion. Saying "timed out" without that would imply the work stopped.
Mutation-checked: removing the narrow except clause fails the propagation test. A third test pins that ordinary exceptions are still returned rather than raised, so the narrow clause cannot start swallowing anything else.', 'backlog', 'medium', NULL, NULL, NULL, '2026-08-11 09:59:05.427', '2026-08-11 10:11:39.291', NULL, 'b7379fa662b40c6996f17c36b64010c0', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0GQBTCRCTXK6EJF0T8CS2C', 'bug', NULL, 'The ''timeout'' execution status is unreachable; timeouts are filed as generic failures', 'execute_task has an `except asyncio.TimeoutError` handler (src/tasks/executor.py:179) that records status=''timeout''. It can never run.
_run_executor wraps the awaited call in `except Exception` (line 219) and returns the error as a value. Since Python 3.11 asyncio.TimeoutError IS the builtin TimeoutError, which inherits OSError -> Exception, so the broad handler catches it first and converts it into an ordinary (None, error) tuple. execute_task then sees a non-None error and files status=''failed''.
Verified in the deployed runtime:
asyncio.TimeoutError is TimeoutError: True
MRO: TimeoutError -> OSError -> Exception -> BaseException
And confirmed against 8 months of history — 18,785 executions, and the count of status=''timeout'' rows is zero:
success 16690 | failed 2093 | running 2 | timeout 0
So every timeout since 2025-12-07 has been recorded as a generic failure carrying a traceback. Operationally that erases the distinction that matters most when a job misbehaves: "this job is too slow for its window" and "this job is broken" need different responses, and right now they look identical in the history.
FIX: catch asyncio.TimeoutError explicitly in _run_executor, ahead of the broad handler, and let it propagate (or return a marker execute_task can distinguish). Note the ordering is the whole bug — a narrower except after a broader one is dead code, and there is no linter configured here to say so.
Related: a thread started by asyncio.to_thread cannot be cancelled, so after T-74 a timeout frees the loop while the work continues to completion. Whatever ''timeout'' comes to mean should say so rather than implying the work stopped.
FIXED in v1.6.0 (e45de4f). _run_executor now catches asyncio.TimeoutError ahead of the broad handler and re-raises, so execute_task''s timeout branch is reachable for the first time.
A trap surfaced while fixing it, and it is the interesting part. While the branch was unreachable, a timeout travelled the NORMAL path — which does update scheduled_tasks. Making the branch reachable without adding that write would have swapped a wrong status (''failed'') for a stale one (last_status showing the previous run''s outcome indefinitely). _update_task_outcome now mirrors terminal outcomes onto the parent row. A fix that repairs the reported symptom while breaking something adjacent is worse than the bug.
The timeout message also records that the work may still be running: after T-74 executors are handed to asyncio.to_thread, and a thread cannot be cancelled, so wait_for frees the loop while the work continues to completion. Saying "timed out" without that would imply the work stopped.
Mutation-checked: removing the narrow except clause fails the propagation test. A third test pins that ordinary exceptions are still returned rather than raised, so the narrow clause cannot start swallowing anything else.', 'done', 'medium', NULL, NULL, NULL, '2026-08-11 09:59:05.427', '2026-08-11 10:11:39.420', NULL, '333edb040cfa995cf26f45b6fa2cd2f9', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0PNFBDB743QJQ9MF38MPJ8', 'task', NULL, 'check_history rows name the task that produced them', NULL, 'backlog', 'medium', NULL, NULL, NULL, '2026-08-11 10:25:02.811', '2026-08-11 10:25:02.811', NULL, 'b099197d1cbdea7e682a054dc17788ab', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0PNFBDB743QJQ9MF38MPJ8', 'task', NULL, 'check_history rows name the task that produced them', 'DONE in v1.7.0 (538bfe5). Verified on a real row:
12:24:27 ok task=backup_portainer_daily source=scheduler/portainer_backup_executor
WHY, and why not the other fix. A row written by a failing probe on 2026-08-11 was byte-for-byte what a nightly backup failure would have written — same source, same domain, nothing naming the task. The reflex was to delete the inconvenient row; that was refused as audit tampering and the refusal was right twice over, because the row is TRUE (a run did fail) and the defect is that it cannot say which run. Attribution keeps the record intact and makes it answer the question.
MECHANISM. A ContextVar (src/task_context.py) set by the engine around executor invocation, read by health_report. Not an argument: executors are invoked as execute(config, settings), there are ten, and several are dormant — they exist only as a string in a database row and become live the moment someone inserts a task naming them. A signature change would leave those broken in a way no import, grep or test would reveal. Not injected into `config` either: config is what a human wrote in the task definition and an executor may legitimately reject unknown keys.
TWO PROPERTIES VERIFIED IN THE DEPLOYED RUNTIME rather than assumed:
- asyncio.to_thread propagates context, so attribution survives the worker thread T-74 introduced. Had it not, every row from a real executor would have lost its task while unit tests kept passing — hence a test that goes through report_async specifically.
- Each asyncio Task gets its own copy, so five concurrent executions cannot read each other''s. The isolation test yields mid-execution to force interleaving; without that it would pass against a shared global.
- A plain await does NOT get its own copy and leaks to the caller. Both entry points use create_task, but task_scope resets by token rather than relying on it.
Omitted rather than nulled when absent — report() is callable from a script, and a null would claim a task existed with no name.
The one pre-existing critical row still carries no task, which now identifies it: it is the only backup row without the field, so it is legible as pre-attribution rather than ambiguous.', 'backlog', 'medium', NULL, NULL, NULL, '2026-08-11 10:25:02.811', '2026-08-11 10:25:02.951', NULL, '5b7bbb05cf6aedfbd0a6a49a356f6e6f', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0PNFBDB743QJQ9MF38MPJ8', 'task', NULL, 'check_history rows name the task that produced them', 'DONE in v1.7.0 (538bfe5). Verified on a real row:
12:24:27 ok task=backup_portainer_daily source=scheduler/portainer_backup_executor
WHY, and why not the other fix. A row written by a failing probe on 2026-08-11 was byte-for-byte what a nightly backup failure would have written — same source, same domain, nothing naming the task. The reflex was to delete the inconvenient row; that was refused as audit tampering and the refusal was right twice over, because the row is TRUE (a run did fail) and the defect is that it cannot say which run. Attribution keeps the record intact and makes it answer the question.
MECHANISM. A ContextVar (src/task_context.py) set by the engine around executor invocation, read by health_report. Not an argument: executors are invoked as execute(config, settings), there are ten, and several are dormant — they exist only as a string in a database row and become live the moment someone inserts a task naming them. A signature change would leave those broken in a way no import, grep or test would reveal. Not injected into `config` either: config is what a human wrote in the task definition and an executor may legitimately reject unknown keys.
TWO PROPERTIES VERIFIED IN THE DEPLOYED RUNTIME rather than assumed:
- asyncio.to_thread propagates context, so attribution survives the worker thread T-74 introduced. Had it not, every row from a real executor would have lost its task while unit tests kept passing — hence a test that goes through report_async specifically.
- Each asyncio Task gets its own copy, so five concurrent executions cannot read each other''s. The isolation test yields mid-execution to force interleaving; without that it would pass against a shared global.
- A plain await does NOT get its own copy and leaks to the caller. Both entry points use create_task, but task_scope resets by token rather than relying on it.
Omitted rather than nulled when absent — report() is callable from a script, and a null would claim a task existed with no name.
The one pre-existing critical row still carries no task, which now identifies it: it is the only backup row without the field, so it is legible as pre-attribution rather than ambiguous.', 'done', 'medium', NULL, NULL, NULL, '2026-08-11 10:25:02.811', '2026-08-11 10:25:03.084', NULL, '63f6b48e753891a75d47dd0b3d7b5a4c', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'bug', NULL, 'DELETE /tasks/{name} 500s for any task that has ever run', 'DELETE /tasks/{task_name} returns 500 whenever the task has at least one row in task_executions:
psycopg2.errors.ForeignKeyViolation: update or delete on table "scheduled_tasks"
violates foreign key constraint "task_executions_task_id_fkey" on table "task_executions"
DETAIL: Key (id)=(46) is still referenced from table "task_executions".
src/main.py:355. Since every task that has ever fired has execution history, the endpoint works only for tasks that have never run — which is close to none of them. Found on 2026-08-11 while cleaning up a temporary probe task created for T-74; it had to be removed with hand-written SQL against two tables, which is not something the API should require.
The caller gets a bare "Internal Server Error" with no indication that history is the obstacle, so it reads as the service being broken rather than the request being refusable.
Deciding what delete should MEAN is the actual work here, and it should not be guessed:
- cascade — drop the execution history with the task. Simple, and silently destroys the audit trail for a task someone deletes by mistake.
- soft delete — mark it deleted and keep the history. Keeps the audit trail, adds a state every query then has to filter on.
- refuse with 409 and a real message ("task has N executions; pass ?purge=true"). Explicit, and makes the destructive variant a deliberate act.
The third is the smallest correct change and matches how the rest of this system treats destructive operations. Whichever is chosen, a 500 on a foreseeable, well-defined condition is the part that is simply wrong.
FIXED in v1.8.0 (c34db66). 409 with a message that can be acted on; ?purge=true proceeds.
Verified against the live service with a throwaway task that had one execution row:
DELETE /tasks/t1_delete_probe2 -> HTTP 409
Task ''t1_delete_probe2'' has 1 execution record(s). Deleting it would discard
that history. Re-send with ?purge=true to delete the task and its history
together, or PUT enabled=false to stop it running while keeping the record.
task still present afterwards: 1 (the refusal deleted nothing)
DELETE /tasks/t1_delete_probe2?purge=true -> HTTP 200
{"message":"...deleted successfully","executions_purged":1}
tasks: 24, probe execution rows: 0
REFUSE RATHER THAN CASCADE, chosen for asymmetry of recovery: a task definition can be recreated from the API in one call, its execution history cannot be recreated at all. Defaulting to the destructive reading of an ambiguous request is how audit trails disappear quietly.
The message carries the three things a caller needs — how much history is at stake, the flag that proceeds, and PUT enabled=false, which is usually what was actually wanted since it stops the task running and keeps the record. A bare "conflict" would be little better than the 500 it replaced.
History and task are deleted in ONE transaction. Split across two, a failure between them leaves the audit trail gone and the task alive: the worst of both outcomes.
Mutation-checked: removing the guard fails the refusal tests. Separate tests pin that a refused delete issues no DELETE at all, and that ?purge=true against a missing task is still 404 rather than a success.
NOTE: the commit for this work is missing its Co-Authored-By trailer — I wrote the message file without it. Already pushed, and fixing it would require rewriting published history on main, so it stands as-is.', 'backlog', 'high', NULL, NULL, NULL, '2026-08-11 09:53:56.826', '2026-08-11 10:31:46.465', NULL, '9299b6bccd53cdc77e2551e2de94a10e', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FZ0FHPBBXXKBRWGR3FJZRK58', 'bug', NULL, 'DELETE /tasks/{name} 500s for any task that has ever run', 'DELETE /tasks/{task_name} returns 500 whenever the task has at least one row in task_executions:
psycopg2.errors.ForeignKeyViolation: update or delete on table "scheduled_tasks"
violates foreign key constraint "task_executions_task_id_fkey" on table "task_executions"
DETAIL: Key (id)=(46) is still referenced from table "task_executions".
src/main.py:355. Since every task that has ever fired has execution history, the endpoint works only for tasks that have never run — which is close to none of them. Found on 2026-08-11 while cleaning up a temporary probe task created for T-74; it had to be removed with hand-written SQL against two tables, which is not something the API should require.
The caller gets a bare "Internal Server Error" with no indication that history is the obstacle, so it reads as the service being broken rather than the request being refusable.
Deciding what delete should MEAN is the actual work here, and it should not be guessed:
- cascade — drop the execution history with the task. Simple, and silently destroys the audit trail for a task someone deletes by mistake.
- soft delete — mark it deleted and keep the history. Keeps the audit trail, adds a state every query then has to filter on.
- refuse with 409 and a real message ("task has N executions; pass ?purge=true"). Explicit, and makes the destructive variant a deliberate act.
The third is the smallest correct change and matches how the rest of this system treats destructive operations. Whichever is chosen, a 500 on a foreseeable, well-defined condition is the part that is simply wrong.
FIXED in v1.8.0 (c34db66). 409 with a message that can be acted on; ?purge=true proceeds.
Verified against the live service with a throwaway task that had one execution row:
DELETE /tasks/t1_delete_probe2 -> HTTP 409
Task ''t1_delete_probe2'' has 1 execution record(s). Deleting it would discard
that history. Re-send with ?purge=true to delete the task and its history
together, or PUT enabled=false to stop it running while keeping the record.
task still present afterwards: 1 (the refusal deleted nothing)
DELETE /tasks/t1_delete_probe2?purge=true -> HTTP 200
{"message":"...deleted successfully","executions_purged":1}
tasks: 24, probe execution rows: 0
REFUSE RATHER THAN CASCADE, chosen for asymmetry of recovery: a task definition can be recreated from the API in one call, its execution history cannot be recreated at all. Defaulting to the destructive reading of an ambiguous request is how audit trails disappear quietly.
The message carries the three things a caller needs — how much history is at stake, the flag that proceeds, and PUT enabled=false, which is usually what was actually wanted since it stops the task running and keeps the record. A bare "conflict" would be little better than the 500 it replaced.
History and task are deleted in ONE transaction. Split across two, a failure between them leaves the audit trail gone and the task alive: the worst of both outcomes.
Mutation-checked: removing the guard fails the refusal tests. Separate tests pin that a refused delete issues no DELETE at all, and that ?purge=true against a missing task is still 404 rather than a success.
NOTE: the commit for this work is missing its Co-Authored-By trailer — I wrote the message file without it. Already pushed, and fixing it would require rewriting published history on main, so it stands as-is.', 'done', 'high', NULL, NULL, NULL, '2026-08-11 09:53:56.826', '2026-08-11 10:31:46.597', NULL, 'e020f77c66ce6631f6d05ee63ec0c2b8', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
AGENTS.md
-72
View File
@@ -1,72 +0,0 @@
# AGENTS.md
> **Start every session by reading this file.**
> This file outlines the operational protocols, coding standards, and architectural decisions for this FastAPI project.
## 1. Agent Operational Protocols
### 🧠 Work Patterns (Plan-Act-Reflect)
* **Plan:** Before writing code, briefly outline your plan. Identify which files you will touch and what the side effects might be.
* **Act:** Execute the changes in small, atomic steps.
* **Reflect:** After coding, verify your work. Did you break existing tests? Did you add new tests?
### 🛡️ Git Discipline
* **NEVER commit to `main` or `master` directly.** Always create a feature branch: `feature/your-feature-name` or `fix/issue-description`.
* **Commit Messages:** Use the [Conventional Commits](https://www.conventionalcommits.org/) format.
* `feat: add user login endpoint`
* `fix: resolve database connection timeout`
* `refactor: split monolith dependency file`
* **Atomic Commits:** Keep commits small. One logical change = one commit.
### 📝 Changelog Maintenance
* **Update `CHANGELOG.md`** with every user-facing change.
* Format: `## [Unreleased] - YYYY-MM-DD` followed by `### Added`, `### Changed`, or `### Fixed`.
### 🚀 Release Flow
When changes are ready for deployment:
1. **Ask user if deploy cycle is desired **
2. **Update version** in `pyproject.toml`:
- Bug fixes: bump patch version (1.8.3 → 1.8.4)
- New features: bump minor version (1.8.4 → 1.9.0)
3. **Update CHANGELOG.md**:
- Move items from `[Unreleased]` to new version section
- Add release date: `## [1.8.4] - 2025-12-16`
4. **Commit and tag**:
```bash
git add -A
git commit -m "fix: description of changes"
git tag v1.8.4
git push origin main --tags
```
5. **CI/CD triggers automatically**:
- Gitea CI builds Docker image on new tag
- Watchtower pulls and deploys to production
- Verify deployment: `curl http://192.168.86.149:8000/health`
---
## 2. FastAPI Architecture & Best Practices
*Reference: [FastAPI Best Practices](https://github.com/zhanymkanov/fastapi-best-practices)*
### 📂 Project Structure (Directory-based, NOT File-type based)
Do **not** group files by type (e.g., one huge `routers` folder). Group by **domain/module** inside a `src/` directory.
**Correct Structure:**
```text
src/
├── auth/
│ ├── router.py # Endpoints
│ ├── schemas.py # Pydantic models
│ ├── service.py # Business logic (CRUD, etc.)
│ ├── dependencies.py# Module-specific dependencies
│ └── config.py # Module-specific settings
├── posts/
│ ├── router.py
│ └── ...
└── main.py # App entry point
CHANGELOG.md
+102 -3
View File
@@ -4,6 +4,107 @@ All notable changes to The Scheduler will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
## [Unreleased]
## [1.9.0] - 2026-08-11
### Changed
- `check_history` rows carry `summary` at the top level, beside `status`, matching the other
writer of that table. It was nested under `metrics`, so a reader had to know which producer
wrote a row to find its substance. Old rows keep the nested spelling.
## [1.8.0] - 2026-08-11
### Fixed
- Deleting a task that has run returns 409 instead of a bare 500. It failed on a foreign key
against its own execution history, which the error never mentioned.
### Added
- `DELETE /tasks/{name}?purge=true` removes a task together with its execution history, in one
transaction. The response reports `executions_purged`.
## [1.7.0] - 2026-08-11
### Added
- `check_history` rows name the scheduled task that produced them. `source` names the code,
which cannot distinguish two tasks sharing one executor — so a failure could not be
attributed to the job that caused it. Omitted when there is no task.
## [1.6.0] - 2026-08-11
### Fixed
- A restart mid-task no longer unschedules that task forever. An execution row left
`running` excluded its task from every future minute, silently; startup now releases them.
- Timeouts are recorded as `timeout` instead of a generic failure. The status existed but was
unreachable, so all 18,785 executions since December contain zero of them.
### Added
- `orphaned` execution status — an execution whose process died, whose outcome is unknown.
Distinct from `failed`, which asserts the work did not succeed.
## [1.5.1] - 2026-08-11
### Fixed
- Backups no longer freeze the API. The config backup ran its ~21 minutes of tarring on the
event loop, so the whole service was unreachable 03:05-03:25 nightly; it now runs in a
worker thread. Health reports moved off the loop too.
- The health report's permission diagnostic prints the database's own message instead of
asserting a cause. It claimed the user lacked INSERT on `check_history` when that grant was
present and the missing one was USAGE on the sequence behind its serial id.
### Notes
- Requires `GRANT USAGE ON SEQUENCE check_history_id_seq TO scheduler_user`, applied
2026-08-11. The table grant alone does not permit the insert.
## [1.5.0] - 2026-08-11
### Added
- Backup executors report their own outcome to the homelab health record — one row in
`check_history` per run, success or failure. Replaces a monitor that inferred backup health
from file age and could not tell a failed backup from one that had not run yet.
### Notes
- Requires `GRANT INSERT ON check_history TO scheduler_user` in the `sysmon` database, applied
2026-08-11. Without it the report is refused, logged, and skipped; the backup itself is unaffected.
## [1.4.0] - 2026-08-08
### Added
- **Portainer Backup Executor** (`portainer_backup_executor.py`) — archives Portainer's
own state through its `/api/backup` endpoint. Portainer's BoltDB lives in a Docker
volume that the daily config backup does not cover, so losing that volume would take
every stack definition with it. Uses the API rather than tarring the live volume, and
rejects a 200 whose body is not a readable archive.
## [1.3.0] - 2026-08-08
### Added
- **Postgres Retention Executor** (`postgres_retention_executor.py`) — deletes rows
past a retention window from a table on the shared Postgres server. Uses the
Scheduler's own credentials with only the database name overridden, so the target
database grants `scheduler_user` SELECT and DELETE on the table.
- **Docker Prune Executor** (`docker_prune_executor.py`) — scheduled reclaim of Docker
disk usage. Build cache and dangling images are pruned by default; unused images and
volumes are opt-in, since volume pruning also removes volumes belonging to stopped
containers.
### Fixed
- `POST /tasks` returned HTTP 500 after successfully creating the task. The response
model declared `created_at`/`updated_at` as strings while the database returns
timestamps, so every create looked like a failure and retrying hit a duplicate-key
error.
### Changed
- `TASK_REGISTRATION.md` now lists the executors that exist. It previously advertised
`shell`, `python` and `docker` executors that were never implemented.
## [1.2.0] - 2026-03-30
### Added
- **GCS Backup Executor** (`gcs_backup_executor.py`) — offsite backup to Google Cloud
Storage.
## [1.1.3] - 2026-01-08
### Changed
@@ -229,9 +330,7 @@ config_backup_executor.py 50% 📈
TOTAL 80% 🎯
```
## [Unreleased]
### Planned
## Planned
- Redis integration for distributed locking
- Webhook notifications for task completion
- Task dependencies (run task B after task A succeeds)
CLAUDE.md
+160
View File
@@ -0,0 +1,160 @@
# CLAUDE.md — scheduler
"The Scheduler" — system-wide maintenance orchestration for tower-of-joy: config backups, doc
mirroring to Gitea, cleanup and retention, and arbitrary REST calls on a cron. Python 3.12 /
FastAPI, APScheduler, PostgreSQL and Redis. Container `scheduler` on `docker-dataplane`,
port **8090**, Redis DB **3**, Postgres DB **scheduler**.
It is the homelab's cron. Recurring work belongs here rather than in a systemd timer (workspace D-5).
## Live contract
`http://localhost:8090/openapi.json` — 10 paths, `version: 1.9.0` (verified 2026-08-11). Human
docs at `/docs`. Generated from running code, so read it instead of inferring routes.
**Every route except `/health` requires `Authorization: Bearer $SCHEDULER_API_KEY`.** An
unauthenticated call returns `{"detail": "Missing API key"}` at 200-shape JSON, not a 401 body
you might pattern-match on.
Note the old AGENTS.md told you to verify deploys against `192.168.86.149:8000/health`. That is
**tatlock's** port, not this service's. It is 8090.
## The thing that will mislead you: executors are chosen by data, not code
`src/executors/*.py` are **never statically imported**. `src/tasks/executor.py:202` does:
```python
module_path = f"src.executors.{executor_name}"
module = __import__(module_path, fromlist=['execute'])
```
where `executor_name` comes from a **row in the `scheduled_tasks` table**. Consequences, and
they defeat both of the usual checks:
- **grep finds nothing.** No file imports `config_backup_executor`; the name only ever exists as
a database string.
- **`sys.modules` finds nothing either.** A cold `import src.main` loads only `src`, `src.config`,
`src.main`, `src.models`, `src.tasks`, `src.tasks.executor`. Every executor is absent until a
task actually fires. Absence there is a timing artifact, not evidence of death.
**The authoritative source is the database.** As of 2026-08-09:
| Executor | Rows | Enabled |
|---|---|---|
| `rest_api_executor` | 16 | yes |
| `doc_sync_executor` | 2 | yes |
| `config_backup_executor`, `docker_prune_executor`, `gitea_release_cleanup_executor`, `portainer_backup_executor`, `postgres_retention_executor` | 1 each | yes |
| `example_executor` | 1 | **no** |
| `gcs_backup_executor` | **0** | — |
`gcs_backup_executor` has no rows at all. That does **not** make it dead code: it becomes live
the instant someone inserts a row naming it, with no code change and no deploy. Treat unreferenced
executors as *dormant*, not removable. An executor's contract is a module-level
`execute(config, settings)` — a missing one is caught at run time and reported as
`Executor <name> missing execute() function`, not at import or startup.
Re-check with the query rather than trusting the table above:
```bash
docker exec scheduler python3 -c "
import psycopg2
from src.config import get_settings
s = get_settings()
c = psycopg2.connect(host=s.postgres_host, port=s.postgres_port, dbname=s.postgres_db,
user=s.postgres_user, password=s.postgres_password)
cur = c.cursor()
cur.execute('SELECT executor, count(*), bool_or(enabled) FROM scheduled_tasks GROUP BY executor ORDER BY 1')
[print(r) for r in cur.fetchall()]"
```
Build the connection from `get_settings()` fields as above. Do not print the assembled URL — it
carries the Postgres password.
## Database
Three tables, and the names do not match the API paths: **`scheduled_tasks`** (not `tasks` —
`SELECT … FROM tasks` fails with `UndefinedTable`), `task_executions`, `doc_sources`. Schema is
SQLAlchemy (`src/models.py`); there is no Alembic here, unlike core-api.
## Layout, and one trap in it
`src/main.py` (app + routes), `src/config.py` (pydantic-settings), `src/models.py`,
`src/tasks/executor.py` (the scheduling engine), `src/executors/` (the dynamically-loaded units).
**`src/config/` also exists and is an empty directory.** `import src.config` resolves to
`src/config.py` — verified in the container, `__file__` is `/app/src/config.py`, because a
regular module wins over a namespace package. Do not "fix" this by moving config into the
directory, and do not assume the directory is a package with contents.
## Registering tasks
Tasks are DB-driven, registered over the API — not YAML, not a file in this repo. See
`TASK_REGISTRATION.md` for the payload shape and the cron-field conventions (`hour: -1` means
every hour). There is also a workspace-level `scheduler` skill for driving it conversationally.
## Working here
Group new work by domain rather than by file type; a single large `routers/` folder is the thing
to avoid. Reference: [FastAPI best practices](https://github.com/zhanymkanov/fastapi-best-practices).
```bash
.venv/bin/python -m pytest tests/ # or: pytest tests/
```
Test dependencies are the `test` extra in `pyproject.toml` (pytest, pytest-asyncio, pytest-cov,
freezegun). `pytest.ini` is at the repo root. No linter is configured — no ruff/flake8 config and
neither in the dependencies — so do not assume `ruff check` exists here.
## CI
`.gitea/workflows/build.yml` is the only workflow and triggers **only on `v*` tag push**: build,
push image, ping Watchtower. There is **no CI test or lint gate**. Run the tests yourself before
tagging.
## Work tracking
Work lives in **pql**, not a markdown TODO. **This repo's vault is standalone** — its tickets
and its internal decisions live here in `.pql/` and `governance/`, and travel with a clone,
because `.pql/changelog/` is committed and replayed by the git hooks (workspace D-15). The databases are
gitignored and rebuildable with `pql plan rebuild`.
`pql` is **not** on the non-interactive `PATH` — invoke it as `/home/jpmschweitzer/.local/bin/pql`.
From inside this repo no `--vault` is needed: pql anchors at the nearest `.git/` ancestor, which
is this repo.
```bash
/home/jpmschweitzer/.local/bin/pql ticket list # this repo's open work
/home/jpmschweitzer/.local/bin/pql plan whatsnext # next unblocked item, with context
/home/jpmschweitzer/.local/bin/pql decisions list # this repo's own decisions
```
Stack-level decisions that constrain this service — the host, the network, deploy mechanics,
and the fact that recurring work belongs here at all (workspace D-5) — live in the **workspace** vault
and need the flag:
```bash
/home/jpmschweitzer/.local/bin/pql --vault /mnt/media/Projects decisions list --domain scheduler
```
Note `ticket new --decision D-N` resolves ids within **one** vault, so a ticket here cannot link
to a workspace decision. Cite the id in the ticket body instead.
## Git
- **History is linear — no merge commits.** Work on `main`, or a short-lived branch that is
fast-forwarded and deleted. The previous AGENTS.md mandated a feature branch per change; that
rule was retired workspace-wide on 2026-08-08.
- **Conventional Commits**: `feat:`, `fix:`, `refactor:`, `docs:`, `chore:`.
- **Stage explicitly. Never `git add -A`** — it is denied by policy, and it sweeps in whatever
else is dirty, including secrets.
- Update `CHANGELOG.md` for every user-facing change, under `[Unreleased]`.
## Releasing
Ask whether a deploy is wanted first — it is not automatic.
1. Bump `version` in `pyproject.toml` (patch for fixes, minor for features).
2. Move `[Unreleased]` entries into a dated section in `CHANGELOG.md`.
3. Stage the changed files by name, commit, tag `vX.Y.Z`, `git push origin main --tags`.
4. Gitea CI builds and pushes on the tag; Watchtower deploys it.
5. Verify: `curl http://192.168.86.149:8090/health`.
Makefile
+93
View File
@@ -0,0 +1,93 @@
# scheduler — the repo's command surface (D-27).
#
# There is no venv in this working tree today, even though CLAUDE.md documents
# `.venv/bin/python -m pytest`. `make test` says so rather than failing with a
# bare "No such file or directory", and `make setup` creates one.
#
# `python3` on this host is 3.8; PYTHON names 3.12 explicitly (D-26).
VENV := $(CURDIR)/.venv
PYTHON ?= python3.12
.DEFAULT_GOAL := help
.PHONY: help
help: ## Show this help
@grep -hE '^[a-z][a-z0-9_-]*:.*?## ' $(MAKEFILE_LIST) \
| awk 'BEGIN{FS=":.*?## "}{printf " \033[36m%-14s\033[0m %s\n", $$1, $$2}'
.PHONY: setup
setup: ## Create the venv, install the test extra, and prove it actually works
$(PYTHON) -m venv .venv
$(VENV)/bin/pip install -e ".[test]"
@# Exit 0 from `pip install` is not evidence (D-24) — pip reports success even
@# when the result is unusable. Prove the environment works instead of trusting
@# the install step: `--collect-only` imports every test module and therefore
@# every src module each one pulls in, which is exactly the failure mode this
@# target exists to catch (T-47 — this repo had no venv at all on 2026-08-09,
@# and the documented test command could not work). It runs zero tests, so it
@# stays cheap, and unlike a bare `import src.main` it exercises the tests/
@# tree too, not just the package.
$(VENV)/bin/python -m pytest tests/ --collect-only -q
.PHONY: test
test: ## Run the test suite — hermetic, no live services (D-26)
@test -x $(VENV)/bin/python || { echo "FAIL — no venv in this tree; run: make setup"; exit 69; }
# Integration tests are deselected here, not skipped by accident. Twelve
# tests in test_database_integration.py carry @pytest.mark.integration and
# need a real Postgres; they errored on every run of this target because
# nothing deselected them, and the marker had no target to select it either.
# So they neither passed nor ran — they just made `make test` exit 2 forever,
# which trains a reader to ignore the exit code (T-55).
#
# They were invisible to the netns audit that found the rest of this: they
# fail identically with and without a network, because postgres-shared is a
# Docker-internal name a host process cannot resolve in either case. A
# namespace proves a test does not reach the network; it cannot tell that
# apart from a test whose dependency is unreachable anyway.
$(VENV)/bin/python -m pytest tests/ -m "not integration"
.PHONY: test-integration
test-integration: ## Run only the tests that need live Postgres/Redis
@test -x $(VENV)/bin/python || { echo "FAIL — no venv in this tree; run: make setup"; exit 69; }
# Refuses an empty selection. A target that passes because it selected
# nothing is the defect this repo keeps meeting from the other side, so
# pytest's exit 5 (no tests collected) is a failure with its own message,
# and a collection error gets a different one — "nothing to run" must never
# read as "everything passed" (D-24).
@$(VENV)/bin/python -m pytest tests/ -m integration --collect-only -q >/dev/null 2>&1; \
rc=$$?; \
if [ $$rc -eq 5 ]; then \
echo "FAIL test-integration — selected 0 tests (marker renamed, moved, or lost — this is a defect, not a pass)"; exit 1; \
elif [ $$rc -ne 0 ]; then \
echo "FAIL test-integration — collection errored (rc=$$rc)"; exit 1; \
fi
$(VENV)/bin/python -m pytest tests/ -m integration -v
# No `lint` target, deliberately. CLAUDE.md states it outright: no linter is
# configured, no ruff or flake8 config, neither in the dependencies. Per D-27
# the name is reserved for repos that lint rather than mandated everywhere — a
# target here could only fail or report clean for something never run.
# git hands a hook a non-login shell, which never sees ~/.local/bin — where
# gitleaks lands. Without this the scan reports "not installed" on every push,
# which is a check that fails open (D-24).
export PATH := $(HOME)/.local/bin:/usr/local/bin:$(PATH)
.PHONY: secrets
secrets: ## Scan the commits about to be pushed for credentials
@ci/secrets.sh
# The call surface is identical in every repo; what it runs is not.
#
# `secrets` runs first, deliberately: it is the only failure here that cannot be
# undone by fixing it afterwards. A failed lint costs another commit; a pushed
# credential is cached and indexed whether or not it is later deleted.
#
# Some of these fail today, and are left wired anyway. The state was measured
# once and written down in T-56 rather than being worked around here — a gate
# quietly narrowed to what already passes is a gate that reports success for
# doing nothing, which is the failure this workspace keeps rediscovering.
.PHONY: pre-push
pre-push: secrets ## Everything the pre-push hook runs
@echo " -- not gated here yet: lint (no linter configured) and test (T-56)"
TASK_REGISTRATION.md
+77 -5
View File
@@ -105,11 +105,83 @@ Calls HTTP endpoints. Supports environment variable substitution in headers/body
### Other Executors
- `shell`: Execute shell commands
- `python`: Execute Python scripts
- `docker`: Docker operations
- `backup`: Backup operations
- `doc_sync`: Documentation sync
The `executor` field is the module name under `src/executors/`. These are the
modules that actually exist:
- `config_backup_executor`: tar.gz backup of mounted directories, with retention
- `gcs_backup_executor`: offsite backup to Google Cloud Storage
- `doc_sync_executor`: mirror upstream docs into Gitea
- `gitea_release_cleanup_executor`: drop old Gitea releases, keeping the newest N
- `postgres_retention_executor`: delete rows past a retention window (see below)
- `docker_prune_executor`: reclaim Docker disk usage (see below)
- `portainer_backup_executor`: archive Portainer's own state via its backup API (see below)
- `example_executor`: demo/test
There is **no `shell` or `python` executor**. Earlier revisions of this document
listed them and they were never implemented; work needing a shell belongs either
in a purpose-built executor or on a host systemd timer.
#### `postgres_retention_executor`
Connects with the Scheduler's own Postgres credentials, overriding only the
database name, so the target database must grant `scheduler_user` SELECT and
DELETE on the table. Table and column names are validated against a strict
identifier pattern because they cannot be bound as query parameters.
```json
{
"database": "sysmon",
"table": "check_history",
"timestamp_column": "ts",
"retention_days": 30,
"dry_run": false
}
```
#### `portainer_backup_executor`
Portainer keeps every stack definition, endpoint, user and access-control rule in
a BoltDB inside the `portainer_data` Docker volume, which lives under
`/var/lib/docker/volumes/` and is **not** covered by the daily config backup.
This calls Portainer's `/api/backup` rather than tarring the volume: BoltDB is a
single memory-mapped file, so copying it live can capture a torn page.
The archive contains TLS certificates and private keys and is written `0600`. A
200 response whose body is not a readable archive is treated as a failure — an
archive that will not open is worse than a missing one, because it looks like a
backup until the day it is needed.
```json
{
"url": "${PORTAINER_URL}",
"api_key": "${PORTAINER_API_KEY}",
"output_dir": "/backups/portainer",
"retention_days": 30
}
```
Portainer runs host-networked, so a container name does not resolve; use the
host address. Requires `/mnt/media/backups/portainer` mounted into the container.
#### `docker_prune_executor`
Uses the docker socket already mounted into the container. Only the two stages
that discard regenerable data are on by default.
```json
{
"build_cache": true,
"dangling_images": true,
"unused_images": false,
"volumes": false,
"build_cache_until_hours": 168,
"dry_run": false
}
```
**`volumes` removes volumes belonging to merely-stopped containers, not just
orphaned ones.** Leave it off unless you have checked what is currently
unattached; on this host it is a plausible way to lose a database.
## Complete Task Schema
ci/secrets.sh
Executable
+50
View File
@@ -0,0 +1,50 @@
#!/usr/bin/env bash
# Secret scan over the commits about to be pushed.
#
# Lives here rather than inside .githooks/pre-push so it can be read, run by
# hand (`make secrets`), and changed under review. A hook is a trigger; it is
# not a home for logic. Identical in every repo in this workspace (D-27).
set -euo pipefail
cd "$(git rev-parse --show-toplevel)"
# A non-login shell — which is what git gives a hook — skips /etc/profile.d
# and never sees ~/.local/bin, where the gitleaks release tarball lands.
# Without this the scan reports "not installed" on every push.
[ -d "$HOME/.local/bin" ] && PATH="$HOME/.local/bin:$PATH"
if ! command -v gitleaks >/dev/null 2>&1; then
echo "FAIL secrets — gitleaks not installed, so this check would be a no-op pretending to pass." >&2
echo " https://github.com/gitleaks/gitleaks/releases → ~/.local/bin/gitleaks" >&2
exit 1
fi
# Scan the outgoing range, not full history. History here carries findings
# that are settled — test fixtures and vendored third-party code — and a gate
# that fails on something unfixable gets bypassed within a week. What matters
# is what is about to leave this machine.
if upstream=$(git rev-parse --abbrev-ref --symbolic-full-name '@{u}' 2>/dev/null); then
range="$upstream..HEAD"
elif git rev-parse --verify --quiet origin/main >/dev/null; then
range="origin/main..HEAD"
else
range=""
fi
if [ -z "$range" ]; then
gitleaks dir . --redact --no-banner --exit-code 1 || {
echo "FAIL secrets — gitleaks found a credential in the working tree." >&2; exit 1; }
exit 0
fi
[ -n "$(git log --oneline "$range" 2>/dev/null)" ] || exit 0
gitleaks git . --log-opts="$range" --redact --no-banner --exit-code 1 >/dev/null 2>&1 || {
echo "FAIL secrets — gitleaks found a credential in the commits being pushed." >&2
echo " inspect (values redacted): gitleaks git . --log-opts=\"$range\" --redact" >&2
echo " then remove and rotate it, or suppress deliberately:" >&2
echo " inline '# gitleaks:allow <reason>'" >&2
echo " or add the fingerprint to .gitleaksignore WITH a reason" >&2
exit 1
}
echo " ok secrets"
governance/README.md
+54
View File
@@ -0,0 +1,54 @@
# Decisions, Questions, Rejected
This directory holds structured planning records that pql parses
into pql.db. Each record is a `### [DQR]-N: Title` heading inside
a markdown file. Files live in three per-type subdirectories:
- `decisions/<domain>.md` — confirmed design decisions
- `questions/<domain>.md` — open questions that may resolve into
decisions or rejected proposals
- `rejected/<domain>.md` — rejected proposals (kept for the audit
trail)
The parser infers domain from the filename stem and record type
from the parent subdirectory.
D-records that propose implementation work link to `initiative`-type
tickets via `decision_ref`. Run `pql decisions show <id>
--with-tickets` to inspect implementation status.
## Recommended domains
Start with this canonical set; create files as records land in
each domain:
- **architecture** — structural commitments (storage, layering,
languages, libraries)
- **process** — team workflow (commits, branches, releases, reviews)
- **design** — user-facing surface (UX, UI, public APIs)
- **coding-conventions** — team-internal code shape (style, lint,
file layout)
- **testing** — quality strategy (coverage, layers, gates)
You might also want, project-permitting:
- `accessibility` — if you ship user-facing software
- `security` — if you handle user data or network surfaces
- `licensing` — if you release open-source or commercial
- `documentation` — if user-docs are non-trivial
- `deployment` — if shipping is non-trivial
- `performance` — if you have perf budgets / SLOs
<!-- pql:records (auto-generated; do not edit manually) -->
## Decisions
- _(none)_
## Open questions
- _(none)_
## Rejected
- _(none)_
pyproject.toml
+1 -1
View File
@@ -1,6 +1,6 @@
[project]
name = "the-scheduler"
version = "1.2.0"
version = "1.9.0"
description = "System-wide maintenance orchestration - backups, doc mirroring, cleanup, task automation"
readme = "README.md"
requires-python = ">=3.12"
src/executors/config_backup_executor.py
+111 -53
View File
@@ -2,6 +2,20 @@
Config Backup Executor
Backs up Docker container configs and host-based service configs.
Replicates functionality of maintenance container's backup-configs.sh
The work runs in a worker thread. `execute` is awaited by the scheduling engine
on the same event loop that serves `/health` and the whole REST API, and this
job spends ~21 minutes inside tarfile and zlib. Called inline it starves the
loop for that entire window: on 2026-08-11 the API was unreachable 03:05-03:25
nightly and again at 07:39 when the job was triggered by hand, which is T-74.
The hourly health check fires at :35 and had never once sampled the outage.
`asyncio.to_thread` is the whole fix — zlib releases the GIL while compressing,
so the loop gets scheduled normally. One caveat worth knowing: the engine wraps
executors in `asyncio.wait_for`, and a thread cannot be cancelled. On timeout
the task is recorded as failed while the tar keeps running to completion. That
is still strictly better than blocking everything, and the configured timeout
(3600s) is well clear of the observed 1263s.
"""
import asyncio
import logging
@@ -9,16 +23,75 @@ import tarfile
import tempfile
from datetime import datetime, timedelta
from pathlib import Path
from typing import List, Dict, Any
from typing import Any, Callable, Dict, List, Optional
from src.config import Settings
from src.executors import health_report
logger = logging.getLogger(__name__)
async def execute(config: dict, settings: Settings) -> str:
def _create_tar_filter(excludes: List[str]) -> Callable[[Any], Optional[Any]]:
"""Build the tarfile filter for a set of exclude patterns.
Matching is **substring**, not glob, and that is deliberate: a pattern is
reduced to its literal core by dropping leading `*/` and trailing `/*`, and
a member is excluded when that core appears anywhere in its name.
It looks like a half-finished glob and the temptation is to "fix" it with
`fnmatch`. Doing so would break the deployed configuration badly, because
that configuration is written against these semantics:
".log" fnmatch would match only a file named exactly `.log`,
so every log file starts being archived instead.
"ollama/models/*" fnmatch anchors at the start of the name, and members
are named `docker-data/ollama/models/...`, so nothing
matches and many GB of model blobs enter the archive.
Same for `amp/Versions/*`, `qdrant/storage/*` and the rest — every one is an
unanchored mid-path fragment. The nightly archive would grow, not shrink.
`tests/test_config_backup_executor.py` pins both cases so this cannot be
changed silently.
"""
Execute config backup task.
cores = [p.replace('*/', '').replace('/*', '') for p in excludes]
def tar_filter(tarinfo):
for core in cores:
if core in tarinfo.name:
logger.debug(f"Excluding: {tarinfo.name}")
return None
return tarinfo
return tar_filter
def _cleanup_old_backups(backup_dir: Path, retention_days: int) -> None:
"""Remove backups older than the retention period."""
cutoff_date = datetime.now() - timedelta(days=retention_days)
removed_count = 0
removed_size = 0
logger.info(f"Cleaning up backups older than {retention_days} days...")
for backup_file in backup_dir.glob('docker-configs-*.tar.gz'):
file_mtime = datetime.fromtimestamp(backup_file.stat().st_mtime)
if file_mtime < cutoff_date:
file_size = backup_file.stat().st_size
logger.info(f"Removing old backup: {backup_file.name} (from {file_mtime:%Y-%m-%d})")
backup_file.unlink()
removed_count += 1
removed_size += file_size
if removed_count > 0:
removed_size_mb = removed_size / (1024 * 1024)
logger.info(f"Removed {removed_count} old backups, freed {removed_size_mb:.2f}MB")
else:
logger.info("No old backups to remove")
def _backup(config: dict) -> str:
"""The blocking body of the backup. Runs in a worker thread, never on the loop.
Config schema:
{
@@ -34,20 +107,11 @@ async def execute(config: dict, settings: Settings) -> str:
"compress": true
}
Args:
config: Backup configuration
settings: Global scheduler settings
Returns:
Summary of backup operation
Raises:
Exception: On backup failure
Returns a one-line summary. Raises on failure.
"""
sources = config.get('sources', [])
backup_dir = Path(config.get('backup_dir', '/backups/docker-configs'))
retention_days = config.get('retention_days', 30)
compress = config.get('compress', True)
if not sources:
raise ValueError("No backup sources configured")
@@ -58,15 +122,12 @@ async def execute(config: dict, settings: Settings) -> str:
logger.info(f"Starting Docker configs backup: {backup_filename}")
# Create backup directory
backup_dir.mkdir(parents=True, exist_ok=True)
# Create temporary directory for staging
with tempfile.TemporaryDirectory(prefix='backup-') as temp_dir:
temp_path = Path(temp_dir)
results = []
# Backup each source
for source in sources:
source_path = Path(source['path'])
source_name = source['name']
@@ -78,23 +139,13 @@ async def execute(config: dict, settings: Settings) -> str:
logger.info(f"Backing up {source_name} from {source_path}")
# Create tar for this source
source_tar = temp_path / f"{source_name}.tar.gz"
def tar_filter(tarinfo):
"""Filter function to exclude patterns."""
for pattern in excludes:
# Simple pattern matching (could be enhanced with fnmatch)
if pattern.replace('*/', '').replace('/*', '') in tarinfo.name:
logger.debug(f"Excluding: {tarinfo.name}")
return None
return tarinfo
with tarfile.open(source_tar, 'w:gz') as tar:
tar.add(
source_path,
arcname=source_name,
filter=tar_filter,
filter=_create_tar_filter(excludes),
recursive=True
)
@@ -102,23 +153,19 @@ async def execute(config: dict, settings: Settings) -> str:
results.append(f"{source_name}: {source_size:.2f}MB")
logger.info(f"Backed up {source_name}: {source_size:.2f}MB")
# Combine all source backups into final archive
logger.info("Creating combined backup archive...")
with tarfile.open(backup_file, 'w:gz') as final_tar:
for item in temp_path.glob('*.tar.gz'):
final_tar.add(item, arcname=item.name)
# Verify backup created
if not backup_file.exists():
raise Exception("Backup file was not created")
backup_size = backup_file.stat().st_size / (1024 * 1024) # MB
logger.info(f"Backup created successfully: {backup_size:.2f}MB")
# Clean up old backups
await cleanup_old_backups(backup_dir, retention_days)
_cleanup_old_backups(backup_dir, retention_days)
# Count remaining backups
backup_count = len(list(backup_dir.glob('docker-configs-*.tar.gz')))
total_size = sum(f.stat().st_size for f in backup_dir.glob('docker-configs-*.tar.gz'))
total_size_mb = total_size / (1024 * 1024)
@@ -133,27 +180,38 @@ async def execute(config: dict, settings: Settings) -> str:
return output
async def cleanup_old_backups(backup_dir: Path, retention_days: int):
"""Remove backups older than retention period."""
cutoff_date = datetime.now() - timedelta(days=retention_days)
removed_count = 0
removed_size = 0
async def _run(config: dict, settings: Settings) -> str:
"""Await the backup without holding the event loop. See the module docstring."""
return await asyncio.to_thread(_backup, config)
logger.info(f"Cleaning up backups older than {retention_days} days...")
for backup_file in backup_dir.glob('docker-configs-*.tar.gz'):
# Get file modification time
file_mtime = datetime.fromtimestamp(backup_file.stat().st_mtime)
async def execute(config: dict, settings: Settings) -> str:
"""Run the backup and report its own outcome to check_history (D-33, T-69).
if file_mtime < cutoff_date:
file_size = backup_file.stat().st_size
logger.info(f"Removing old backup: {backup_file.name} (from {file_mtime:%Y-%m-%d})")
backup_file.unlink()
removed_count += 1
removed_size += file_size
if removed_count > 0:
removed_size_mb = removed_size / (1024 * 1024)
logger.info(f"Removed {removed_count} old backups, freed {removed_size_mb:.2f}MB")
else:
logger.info("No old backups to remove")
The report wraps the work rather than living inside it, so the failure path
cannot be forgotten: an exception is reported as critical and then re-raised,
leaving the task's own status untouched. Reporting only success would
reproduce exactly the blind spot this replaces — a monitor that cannot tell
a failed backup from one that has not run.
"""
try:
output = await _run(config, settings)
except Exception as exc:
await health_report.report_async(
settings,
domain="backup",
status=health_report.CRITICAL,
source="scheduler/config_backup_executor",
summary=f"backup failed: {str(exc)[:300]}",
metrics={"job": "scheduler/config_backup_executor", "error": str(exc)[:400]},
)
raise
await health_report.report_async(
settings,
domain="backup",
status=health_report.OK,
source="scheduler/config_backup_executor",
summary=output[:400],
metrics={"job": "scheduler/config_backup_executor"},
)
return output
src/executors/docker_prune_executor.py
+109
View File
@@ -0,0 +1,109 @@
"""
Docker Prune Executor
Scheduled, non-interactive reclaim of Docker disk usage. The host equivalent is
system-admin-toj's scripts/disk/prune-docker.sh, which prompts per stage; a cron
task cannot prompt, so the destructive stages are opt-in instead.
Runs the docker CLI against the socket already mounted into this container.
Config schema:
{
"build_cache": true, # safe: cache is rebuilt on demand
"dangling_images": true, # safe: untagged layers nothing references
"unused_images": false, # re-pull on next deploy; costs bandwidth
"volumes": false, # DESTRUCTIVE - see below
"build_cache_until_hours": 168,
"dry_run": false
}
`volumes` is off by default and should stay off unless you have checked what is
actually unattached. `docker volume prune` removes every volume not bound to a
*running* container, which includes the data volume of anything merely stopped.
On this host that is a plausible way to lose a database.
Defaults are the two stages that only ever discard regenerable data.
"""
import asyncio
import logging
from typing import Any, Dict, List, Tuple
from src.config import Settings
logger = logging.getLogger(__name__)
COMMAND_TIMEOUT = 900
async def _run(args: List[str]) -> Tuple[int, str, str]:
proc = await asyncio.create_subprocess_exec(
*args,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
)
try:
stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=COMMAND_TIMEOUT)
except asyncio.TimeoutError:
proc.kill()
await proc.wait()
raise Exception(f"timed out after {COMMAND_TIMEOUT}s: {' '.join(args)}")
return proc.returncode, stdout.decode().strip(), stderr.decode().strip()
def _reclaimed(output: str) -> str:
"""Pull the 'Total reclaimed space: X' line out of docker's prune output."""
for line in output.splitlines():
if "reclaimed space" in line.lower():
return line.split(":", 1)[1].strip()
return "0B"
async def execute(config: Dict[str, Any], settings: Settings) -> str:
dry_run = bool(config.get("dry_run", False))
until_hours = int(config.get("build_cache_until_hours", 168))
stages: List[Tuple[str, List[str]]] = []
if config.get("build_cache", True):
stages.append(
("build cache", ["docker", "builder", "prune", "-f", "--filter", f"until={until_hours}h"])
)
if config.get("dangling_images", True):
stages.append(("dangling images", ["docker", "image", "prune", "-f"]))
if config.get("unused_images", False):
stages.append(("unused images", ["docker", "image", "prune", "-a", "-f"]))
if config.get("volumes", False):
logger.warning(
"volume pruning is enabled; this removes volumes belonging to stopped "
"containers, not just orphaned ones"
)
stages.append(("volumes", ["docker", "volume", "prune", "-f"]))
if not stages:
return "no prune stages enabled; nothing to do"
rc, out, err = await _run(["docker", "system", "df"])
if rc != 0:
raise Exception(f"docker unavailable: {err or out}")
before = out
if dry_run:
planned = ", ".join(name for name, _ in stages)
logger.info("dry run; would prune: %s", planned)
return f"dry run - would prune: {planned}\n{before}"
results = []
for name, args in stages:
rc, out, err = await _run(args)
if rc != 0:
# Report rather than abort: a later stage may still reclaim space, and
# a partial reclaim is more useful than none.
logger.error("prune stage %r failed: %s", name, err or out)
results.append(f"{name}: FAILED ({(err or out).splitlines()[0] if (err or out) else 'unknown'})")
continue
results.append(f"{name}: {_reclaimed(out)}")
logger.info("pruned %s -> %s", name, _reclaimed(out))
summary = "; ".join(results)
if any("FAILED" in r for r in results):
raise Exception(f"one or more prune stages failed: {summary}")
return f"reclaimed - {summary}"
src/executors/health_report.py
+185
View File
@@ -0,0 +1,185 @@
"""Report a task's own outcome to the homelab's central health record.
Why a process reports itself, rather than a monitor inferring it:
The sysmon `backup` domain used to poll the mtime of the newest file in the
backup directory, hourly, against a 48-hour threshold — for a job that runs once
a day. Forty-seven of every forty-eight runs could not produce a new answer, and
worse, a file-age poll cannot distinguish "the backup failed" from "the backup
has not run yet". If tonight's job dies, yesterday's archive is 24 hours old and
still reads healthy, and keeps reading healthy until hour 48. A failure stayed
invisible for two days to the check whose only job was noticing it.
This executor knows at 03:05. So it says so.
Recorded as D-33 in the workspace vault: `check_history` is the central health
record and any self-maintained service may push a row describing its own
outcome. sysmon polls only the things that cannot report themselves.
The properties that are load-bearing here — a count is not given, because
this list has grown twice and a stale number is worse than none:
- `source` names the producer, because the table now has several writers and a
row must say which one wrote it.
- `summary` sits at the top level, beside `status`, because that is where the
other writer puts it. One spelling per fact, or a reader has to know which
producer wrote a row before it can find out what the row says.
- `task` names the schedule that invoked it, which `source` cannot: two tasks
may share one executor. On 2026-08-11 two did, and their rows were identical
apart from their contents — a failure could not be attributed to either. It
comes from a ContextVar the engine sets (`src/task_context.py`), so executors
need no signature change and dormant ones stay valid.
- `domain` is a shared namespace. Two producers claiming one name would
interleave silently.
- **A reporting failure must never fail the task.** Backing up successfully and
failing to mention it is strictly better than the reverse. Everything here is
caught and logged, which means the absence of rows is the only symptom a
broken reporter produces — so check for rows, not for errors.
"""
import asyncio
import json
import logging
from datetime import datetime, timezone
from typing import Any, Dict, Optional
import psycopg2
from src.task_context import current_task_name
logger = logging.getLogger(__name__)
# The database holding check_history. Not the scheduler's own database — this is
# a cross-service write into the health record, and it is deliberate (D-33).
HEALTH_DB = "sysmon"
OK = "ok"
WARNING = "warning"
CRITICAL = "critical"
def report(
settings: Any,
domain: str,
status: str,
source: str,
summary: str,
metrics: Optional[Dict[str, Any]] = None,
) -> bool:
"""Write one row to check_history. Returns whether it landed.
Never raises. A caller that lets this failure surface would turn a
successful backup into a failed task, which inverts the point.
"""
metrics = metrics or {}
now = datetime.now(timezone.utc)
task = current_task_name()
result = {
# The envelope the table has carried since the shell era. A reader of a
# year of history should not have to know which producer wrote a row in
# order to parse it.
"timestamp": now.isoformat(),
"source": source,
"domain": domain,
"status": status,
# Top level, beside status — the same place sysmon-go writes it. It lived
# under metrics until 2026-08-11, so the two writers of this shared table
# disagreed about where the substance of a row was, and any query written
# the obvious way found one and missed the other. That is the T-36 shape
# exactly: per-domain queries returned nothing because the data was
# nested somewhere else. D-33 made this table a contract between
# producers; a contract needs one spelling.
"summary": summary,
"metrics": metrics,
}
# Which scheduled task produced this. `source` names the code; two tasks can
# share one executor, and on 2026-08-11 two did — a 425 MB probe and the 5 GB
# nightly backup wrote rows that were identical apart from their contents, so
# a failure could not be attributed to either. Omitted rather than nulled
# when absent, per the convention that a missing field means "not
# applicable": report() is also callable from a script with no task around it.
if task:
result["task"] = task
try:
conn = psycopg2.connect(
host=settings.postgres_host,
port=settings.postgres_port,
database=HEALTH_DB,
user=settings.postgres_user,
password=settings.postgres_password,
connect_timeout=10,
)
except Exception as exc: # noqa: BLE001 - reporting must not raise
logger.warning("health report for %s could not connect to %s: %s", domain, HEALTH_DB, exc)
return False
try:
with conn:
with conn.cursor() as cur:
# Unqualified table name, resolved through the search_path of the
# sysmon database. Qualifying it as sysmon.check_history looks
# more careful and is wrong — that schema does not exist.
cur.execute(
"INSERT INTO check_history (host, domain, status, ts, result) "
"VALUES (%s, %s, %s, %s, %s)",
(_host(), domain, status, now, json.dumps(result)),
)
logger.info("health report: %s=%s recorded", domain, status)
return True
except psycopg2.errors.InsufficientPrivilege as exc:
# Named separately because it is the expected first failure and the fix
# is a grant rather than a code change. The database's own message is
# printed verbatim rather than summarised: the first version of this
# asserted "lacks INSERT on check_history" and was wrong — the table
# grant was present and what was actually missing was USAGE on
# check_history_id_seq, the sequence behind its serial id. A diagnostic
# that names a cause it did not observe sends the reader to the wrong
# fix with confidence.
#
# GRANT INSERT ON check_history TO <user>;
# GRANT USAGE ON SEQUENCE check_history_id_seq TO <user>;
logger.warning(
"health report for %s refused by the database: %s. The task itself succeeded; "
"only the report was lost.",
domain, str(exc).strip().splitlines()[0],
)
return False
except Exception as exc: # noqa: BLE001 - reporting must not raise
logger.warning("health report for %s failed: %s", domain, exc)
return False
finally:
conn.close()
async def report_async(
settings: Any,
domain: str,
status: str,
source: str,
summary: str,
metrics: Optional[Dict[str, Any]] = None,
) -> bool:
"""`report` for callers on the event loop. Prefer this one inside executors.
psycopg2 is a blocking driver, so calling `report` directly from an
`async def` holds the loop for the length of the connect and insert — up to
`connect_timeout` seconds if the database is unreachable, which is exactly
when a report is most likely to be attempted. The scheduler serves its own
`/health` from that loop, so the cost of a slow report is the whole service
appearing down (T-74).
"""
return await asyncio.to_thread(report, settings, domain, status, source, summary, metrics)
def _host() -> str:
"""The host a row is attributed to.
Every Redis key and check_history row is scoped by host so a second machine
reporting into the same store stays distinguishable. The scheduler runs in a
container, whose hostname is a container id — useless as an attribution — so
the physical host is named explicitly.
"""
import os
return os.environ.get("SYSMON_HOST", "tower-of-joy")
src/executors/portainer_backup_executor.py
+173
View File
@@ -0,0 +1,173 @@
"""
Portainer Backup Executor
Archives Portainer's own state through its `/api/backup` endpoint.
Why it needs backing up separately: Portainer keeps every stack definition,
endpoint, user and access-control rule in a BoltDB inside the Docker volume
`portainer_data`, which lives under /var/lib/docker/volumes/. The daily config
backup covers ~/docker-data and code-server-config only, so that volume is not
in it. Losing it takes all 24 stack definitions with it.
Why the API rather than tarring the volume: BoltDB is a single memory-mapped
file, so copying it while Portainer is writing can capture a torn page. The API
serialises a consistent snapshot.
The archive contains TLS certificates and private keys, so it is written 0600.
Config schema:
{
"url": "http://172.17.0.1:8001", # Portainer is host-networked, so a
# container name does not resolve;
# use the bridge gateway
"api_key": "${PORTAINER_API_KEY}", # ${VAR} reads the container env
"output_dir": "/backups/portainer",
"retention_days": 30,
"password": "" # optional; encrypts the archive
}
"""
import logging
import os
import re
import tarfile
import time
from datetime import datetime, timedelta, timezone
from pathlib import Path
import httpx
from src.config import Settings
from src.executors import health_report
logger = logging.getLogger(__name__)
BACKUP_TIMEOUT = 300
FILENAME_RE = re.compile(r"^portainer-\d{8}T\d{6}Z\.tar\.gz$")
def _substitute_env(value: str) -> str:
"""Expand ${VAR} against the container environment, as rest_api does."""
if not isinstance(value, str):
return value
for var in re.findall(r"\$\{([A-Z_][A-Z0-9_]*)\}", value):
resolved = os.getenv(var, "")
if not resolved:
logger.warning("environment variable not found: %s", var)
value = value.replace(f"${{{var}}}", resolved)
return value
def _prune(output_dir: Path, retention_days: int) -> int:
"""Delete archives older than the retention window. Returns how many went."""
cutoff = datetime.now(timezone.utc) - timedelta(days=retention_days)
removed = 0
for path in output_dir.glob("portainer-*.tar.gz"):
# Match the exact name this executor writes; never delete a stray file
# someone else put here.
if not FILENAME_RE.match(path.name):
continue
if datetime.fromtimestamp(path.stat().st_mtime, timezone.utc) < cutoff:
path.unlink()
removed += 1
logger.info("pruned old portainer backup: %s", path.name)
return removed
async def _run(config: dict, settings: Settings) -> str:
url = _substitute_env(config.get("url", "")).rstrip("/")
api_key = _substitute_env(config.get("api_key", ""))
output_dir = Path(config.get("output_dir", "/backups/portainer"))
retention_days = config.get("retention_days", 30)
password = _substitute_env(config.get("password", "") or "")
if not url:
raise ValueError("Missing required config: 'url'")
if not api_key:
raise ValueError("Missing or unresolved config: 'api_key'")
if not isinstance(retention_days, int) or isinstance(retention_days, bool) or retention_days < 1:
raise ValueError(f"retention_days must be a positive integer, got {retention_days!r}")
output_dir.mkdir(parents=True, exist_ok=True)
stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ")
final = output_dir / f"portainer-{stamp}.tar.gz"
partial = final.with_suffix(".partial")
started = time.monotonic()
try:
async with httpx.AsyncClient(timeout=BACKUP_TIMEOUT) as client:
response = await client.post(
f"{url}/api/backup",
headers={"X-API-Key": api_key, "Content-Type": "application/json"},
json={"password": password} if password else {},
)
if response.status_code != 200:
raise Exception(
f"Portainer returned HTTP {response.status_code}: {response.text[:200]}"
)
partial.write_bytes(response.content)
# A 200 with a truncated body is still a failed backup. An archive that
# cannot be opened is worse than a missing one, because it looks like a
# backup until the day it is needed.
if not password:
try:
with tarfile.open(partial, "r:gz") as archive:
entries = len(archive.getnames())
except Exception as exc: # noqa: BLE001
# Deliberately broad. A truncated archive raises EOFError, which
# is neither TarError nor OSError, and any failure to open it
# means the same thing regardless of type: this is not a backup.
raise Exception(f"response is not a readable archive: {exc}") from exc
else:
entries = -1 # encrypted; contents cannot be verified here
partial.replace(final)
final.chmod(0o600) # contains TLS certs and private keys
finally:
if partial.exists():
partial.unlink()
removed = _prune(output_dir, retention_days)
kept = len([p for p in output_dir.glob("portainer-*.tar.gz") if FILENAME_RE.match(p.name)])
size_mb = final.stat().st_size / 1_048_576
elapsed = time.monotonic() - started
summary = (
f"backed up Portainer to {final.name} "
f"({size_mb:.2f} MB{'' if entries < 0 else f', {entries} entries'}, {elapsed:.1f}s); "
f"kept {kept}, pruned {removed} older than {retention_days}d"
)
logger.info(summary)
return summary
async def execute(config: dict, settings: Settings) -> str:
"""Run the backup and report its own outcome to check_history (D-33, T-69).
The report wraps the work rather than living inside it, so the failure path
cannot be forgotten: an exception is reported as critical and then re-raised,
leaving the task's own status untouched. Reporting only success would
reproduce exactly the blind spot this replaces — a monitor that cannot tell
a failed backup from one that has not run.
"""
try:
output = await _run(config, settings)
except Exception as exc:
await health_report.report_async(
settings,
domain="backup",
status=health_report.CRITICAL,
source="scheduler/portainer_backup_executor",
summary=f"backup failed: {str(exc)[:300]}",
metrics={"job": "scheduler/portainer_backup_executor", "error": str(exc)[:400]},
)
raise
await health_report.report_async(
settings,
domain="backup",
status=health_report.OK,
source="scheduler/portainer_backup_executor",
summary=output[:400],
metrics={"job": "scheduler/portainer_backup_executor"},
)
return output
src/executors/postgres_retention_executor.py
+109
View File
@@ -0,0 +1,109 @@
"""
Postgres Retention Executor
Deletes rows older than a retention window from a table on the shared Postgres
server. Written for sysmon's `check_history`, which grows with every monitoring
check and had no retention at all, but the executor is table-agnostic.
Connects with the Scheduler's own Postgres credentials and only overrides the
database name. That keeps a second set of credentials out of the stack; the
target database grants `scheduler_user` exactly SELECT and DELETE on the table,
so a bug here can drop old rows but cannot corrupt or forge history.
Config schema:
{
"database": "sysmon", # defaults to the Scheduler's own database
"table": "check_history", # required
"timestamp_column": "ts", # required
"retention_days": 30, # required, must be >= 1
"dry_run": false # count what would go, delete nothing
}
Table and column names cannot be passed as query parameters, so both are
validated against a strict identifier pattern before being interpolated.
Autovacuum reclaims the space afterwards; this deliberately does not VACUUM,
which would need table ownership the Scheduler intentionally does not have.
"""
import asyncio
import logging
import re
from typing import Any, Dict
import psycopg2
from src.config import Settings
logger = logging.getLogger(__name__)
# Deliberately strict: unquoted lowercase identifiers only. Anything needing
# quoting is out of scope and would be a hole in the interpolation below.
IDENTIFIER_RE = re.compile(r"^[a-z_][a-z0-9_]*$")
MAX_RETENTION_DAYS = 3650
def _validate_identifier(value: str, label: str) -> str:
if not isinstance(value, str) or not IDENTIFIER_RE.match(value):
raise ValueError(
f"invalid {label}: {value!r} (expected an unquoted lowercase identifier)"
)
return value
def _prune(config: Dict[str, Any], settings: Settings) -> str:
table = _validate_identifier(config.get("table", ""), "table")
column = _validate_identifier(config.get("timestamp_column", ""), "timestamp_column")
database = config.get("database") or settings.postgres_db
_validate_identifier(database, "database")
retention_days = config.get("retention_days")
if not isinstance(retention_days, int) or isinstance(retention_days, bool):
raise ValueError(f"retention_days must be an integer, got {retention_days!r}")
# A zero or negative window would delete everything, including the row the
# check just wrote. Refuse rather than quietly wipe the table.
if retention_days < 1 or retention_days > MAX_RETENTION_DAYS:
raise ValueError(
f"retention_days must be between 1 and {MAX_RETENTION_DAYS}, got {retention_days}"
)
dry_run = bool(config.get("dry_run", False))
cutoff_sql = f"{column} < now() - make_interval(days => %s)"
conn = psycopg2.connect(
host=settings.postgres_host,
port=settings.postgres_port,
database=database,
user=settings.postgres_user,
password=settings.postgres_password,
connect_timeout=10,
)
try:
with conn:
with conn.cursor() as cur:
cur.execute(f"SELECT count(*) FROM {table} WHERE {cutoff_sql}", (retention_days,))
stale = cur.fetchone()[0]
if dry_run:
logger.info("dry run: %s rows in %s.%s exceed %sd", stale, database, table, retention_days)
return f"dry run: {stale} rows older than {retention_days}d in {database}.{table}"
if stale == 0:
return f"nothing to prune in {database}.{table} (retention {retention_days}d)"
cur.execute(f"DELETE FROM {table} WHERE {cutoff_sql}", (retention_days,))
deleted = cur.rowcount
cur.execute(f"SELECT count(*) FROM {table}")
remaining = cur.fetchone()[0]
finally:
conn.close()
logger.info("pruned %s rows from %s.%s, %s remain", deleted, database, table, remaining)
return f"pruned {deleted} rows older than {retention_days}d from {database}.{table}, {remaining} remain"
async def execute(config: dict, settings: Settings) -> str:
"""Delete rows past the retention window. Returns a one-line summary."""
# psycopg2 is synchronous; keep it off the scheduler's event loop.
return await asyncio.to_thread(_prune, config, settings)
src/main.py
+59 -11
View File
@@ -7,7 +7,7 @@ Architecture: Hybrid APScheduler + DB-based priority system
- Job queries DB for tasks scheduled in that minute
- Executes up to 5 tasks concurrently based on priority
"""
from fastapi import FastAPI, HTTPException, Depends, Header
from fastapi import FastAPI, HTTPException, Depends, Header, Query
from apscheduler.schedulers.asyncio import AsyncIOScheduler
from apscheduler.jobstores.sqlalchemy import SQLAlchemyJobStore
from apscheduler.triggers.cron import CronTrigger
@@ -65,6 +65,13 @@ async def lifespan(app: FastAPI):
task_executor = TaskExecutor(settings)
logger.info("Task executor initialized (max 5 concurrent tasks)")
# Before the first minute is processed, release any task still held by an
# execution row belonging to an instance that no longer exists. A 'running'
# row excludes its task from scheduling permanently, so skipping this leaves
# tasks silently unschedulable across every restart. Blocking briefly is fine
# here — the app serves no requests until lifespan yields.
task_executor.reconcile_orphaned_executions()
# Initialize APScheduler with minimal configuration
# No jobstore needed - we only have one in-memory job
scheduler = AsyncIOScheduler(
@@ -346,26 +353,67 @@ async def update_task(
@app.delete("/tasks/{task_name}")
async def delete_task(
task_name: str,
purge: bool = Query(
False,
description="Also delete this task's execution history. Required when the "
"task has ever run, and destroys its audit trail."
),
api_key: str = Depends(verify_api_key),
executor: TaskExecutor = Depends(get_task_executor)
):
"""Delete a scheduled task."""
"""Delete a scheduled task.
A task that has ever run owns rows in task_executions, and those rows are
the audit trail — when it ran, how long it took, what it returned. Deleting
the task alone violates task_executions_task_id_fkey, which surfaced as a
bare 500 with no indication that history was the obstacle, so it read as the
service being broken rather than the request being refusable. Since every
task that has ever fired has history, the endpoint effectively worked only
for tasks that had never run.
Refusing with 409 rather than cascading by default, because the two outcomes
are not equally recoverable: a task definition can be recreated from the API
in one call, its execution history cannot be recreated at all. The caller
who wants both gone says so.
Disabling is usually what was actually wanted — it stops the task running and
keeps the record — so the refusal names that too.
"""
with executor.get_db_connection() as conn:
with conn.cursor() as cur:
cur.execute("""
DELETE FROM scheduled_tasks
WHERE task_name = %s
RETURNING task_name
""", (task_name,))
deleted = cur.fetchone()
if not deleted:
cur.execute("SELECT id FROM scheduled_tasks WHERE task_name = %s", (task_name,))
row = cur.fetchone()
if not row:
raise HTTPException(404, f"Task '{task_name}' not found")
task_id = row[0]
cur.execute("SELECT COUNT(*) FROM task_executions WHERE task_id = %s", (task_id,))
executions = cur.fetchone()[0]
if executions and not purge:
raise HTTPException(
409,
f"Task '{task_name}' has {executions} execution record(s). "
f"Deleting it would discard that history. Re-send with "
f"?purge=true to delete the task and its history together, or "
f"PUT enabled=false to stop it running while keeping the record."
)
# One transaction: a purge that removed the history and then failed to
# remove the task would leave the audit trail gone and the task alive.
if executions:
cur.execute("DELETE FROM task_executions WHERE task_id = %s", (task_id,))
cur.execute("DELETE FROM scheduled_tasks WHERE id = %s", (task_id,))
conn.commit()
if executions:
logger.warning(f"Deleted task {task_name} and purged {executions} execution record(s)")
else:
logger.info(f"Deleted task: {task_name}")
return {"message": f"Task '{task_name}' deleted successfully"}
return {
"message": f"Task '{task_name}' deleted successfully",
"executions_purged": executions,
}
@app.post("/tasks/{task_name}/trigger")
async def trigger_task(
src/models.py
+8 -2
View File
@@ -3,6 +3,7 @@ Pydantic models for The Scheduler API.
"""
from pydantic import BaseModel, Field
from typing import Optional, Dict, Any
from datetime import datetime
from enum import Enum
@@ -203,8 +204,13 @@ class TaskUpdate(BaseModel):
class TaskResponse(TaskCreate):
"""Response model for task operations."""
created_at: str
updated_at: Optional[str] = None
# These are `timestamp` columns, so psycopg2 hands back datetime objects.
# Declaring them as `str` made Pydantic reject every create response, which
# 500'd the endpoint *after* the row had already been inserted and committed.
# FastAPI serialises datetime to an ISO 8601 string, so the JSON on the wire
# is unchanged — and now matches what GET /tasks/{name} already returned.
created_at: datetime
updated_at: Optional[datetime] = None
class Config:
from_attributes = True
src/task_context.py
+59
View File
@@ -0,0 +1,59 @@
"""Which scheduled task is currently executing.
Executors are invoked as `execute(config, settings)` and are never told which
task they are. That is fine until one of them writes to a shared record: on
2026-08-11 a probe task and the nightly backup both used
config_backup_executor, and the rows they wrote into `check_history` were
indistinguishable — same `source`, same `domain`. A failure from a 425 MB test
archive was impossible to tell from a failure of the 5 GB nightly job, so the
health record could not answer "which one broke?".
A ContextVar rather than a parameter, because the alternatives are worse here:
- Threading a `task` argument through `execute(config, settings, task)` is a
signature change across every executor, including the dormant ones that
exist only as a string in a database row and would break the moment
somebody enabled them.
- Injecting the name into `config` corrupts the thing executors validate.
`config` is what a human wrote in the task definition, and an executor is
entitled to reject keys it does not recognise.
Two properties make this safe, both verified in the deployed runtime rather
than assumed:
- `asyncio.to_thread` propagates the context, so a reporter still sees the
task after T-74 moved executor bodies into worker threads.
- Each asyncio Task gets its own copy, so the five concurrent executions
allowed by MAX_CONCURRENT_TASKS cannot read each other's value.
A plain `await` of a coroutine does NOT get its own copy and would leak the
value back to the caller, so `task_scope` resets it rather than relying on the
call always arriving via create_task.
"""
from contextlib import contextmanager
from contextvars import ContextVar
from typing import Optional
_current_task_name: ContextVar[Optional[str]] = ContextVar(
"current_task_name", default=None
)
@contextmanager
def task_scope(task_name: str):
"""Name the executing task for the duration of the block."""
token = _current_task_name.set(task_name)
try:
yield
finally:
_current_task_name.reset(token)
def current_task_name() -> Optional[str]:
"""The executing task's name, or None outside an execution.
None is a real answer, not an error: a reporter may be called from a script
or a test with no task around it. Callers omit the field rather than
inventing one.
"""
return _current_task_name.get()
src/tasks/executor.py
+121 -3
View File
@@ -11,6 +11,7 @@ from psycopg2.extras import RealDictCursor
import traceback
from src.config import Settings
from src.task_context import task_scope
logger = logging.getLogger(__name__)
@@ -115,6 +116,70 @@ class TaskExecutor:
return True
def reconcile_orphaned_executions(self) -> int:
"""Close out execution rows left 'running' by a process that is gone.
get_tasks_for_minute excludes any task holding a 'running' row. That row
is written before the executor runs and updated after, so a process that
dies in between leaves it 'running' forever — and the task is then
excluded from every future minute, permanently, with no error and no log
line. It does not fail; it goes quiet, and quiet reads as healthy.
Live exposure rather than theory: Watchtower restarts this container at
4 AM daily, and the config backup starts at 03:05 and runs ~21 minutes.
A row from 2025-12-07 sat 'running' for eight months before anyone
looked.
Called at startup, where the reasoning is sound by construction: this
process has just begun, so nothing it can see is genuinely running, and
any such row belongs to an instance that no longer exists.
Marked 'orphaned', not 'failed'. When the process dies mid-task the work
may well have finished — a backup that completed and never got to update
its row is indistinguishable from one that died halfway. 'failed' would
assert an outcome nobody observed. 'orphaned' says only what is known:
we lost track of it.
Deliberately not extended to a time-based sweep of long-running rows.
While this process lives, execute_task's finally clause always closes the
row out, so a stale row implies a dead owner. A duration-based rule would
have to tell a slow task from a dead one, and getting that wrong closes
the record of a task that is still working.
"""
try:
with self.get_db_connection() as conn:
with conn.cursor() as cur:
cur.execute("""
UPDATE task_executions
SET status = 'orphaned',
completed_at = %s,
error = 'Scheduler restarted while this execution was '
'running; its outcome is unknown.'
WHERE status = 'running'
RETURNING task_name, started_at
""", (datetime.now(timezone.utc),))
orphans = cur.fetchall()
conn.commit()
except Exception as e:
# Never fatal. A scheduler that refuses to start because it could not
# tidy up is worse than one carrying a stale row.
logger.error(f"Could not reconcile orphaned executions: {e}")
return 0
for task_name, started_at in orphans:
logger.warning(
f"Orphaned execution recovered: {task_name} was left 'running' "
f"since {started_at}. That task had been excluded from scheduling "
f"until now."
)
if orphans:
logger.warning(
f"{len(orphans)} task(s) were unschedulable and are now released."
)
else:
logger.info("No orphaned executions to reconcile")
return len(orphans)
async def execute_task(self, task: Dict[str, Any]):
"""
Execute a single task with timeout and error handling.
@@ -145,7 +210,10 @@ class TaskExecutor:
execution_id = cur.fetchone()[0]
conn.commit()
# Load and execute the task
# Load and execute the task. The scope names it for anything the
# executor writes to a shared record — without it, two tasks sharing
# one executor produce rows nobody can tell apart.
with task_scope(task_name):
output, error = await self._run_executor(executor_name, task, timeout)
completed_at = datetime.now(timezone.utc)
@@ -178,8 +246,17 @@ class TaskExecutor:
except asyncio.TimeoutError:
logger.error(f"Task {task_name} timed out after {timeout}s")
self._update_execution_status(execution_id, 'timeout',
error=f"Task exceeded timeout of {timeout}s")
self._update_execution_status(
execution_id, 'timeout',
error=f"Task exceeded timeout of {timeout}s. The underlying work may "
f"still be running — executors that use asyncio.to_thread hand "
f"the work to a thread, and a thread cannot be cancelled.")
# scheduled_tasks has to be written here as well. On the normal path
# it is updated alongside the execution row, and while this branch was
# unreachable a timeout travelled that path as a 'failed' result — so
# last_status did stay current. Making the branch reachable without
# this call would swap one wrong status for a stale one.
self._update_task_outcome(task_id, 'timeout', started_at)
except Exception as e:
logger.error(f"Task {task_name} failed with exception: {e}")
@@ -214,11 +291,52 @@ class TaskExecutor:
return result, None
except asyncio.TimeoutError:
# This clause must precede `except Exception`, and that ordering is
# the entire bug it fixes. Since Python 3.11 asyncio.TimeoutError IS
# the builtin TimeoutError, which inherits OSError -> Exception, so
# the broad handler below used to catch it first and convert it into
# an ordinary (None, error) tuple. execute_task then filed it as a
# generic 'failed', and its own `except asyncio.TimeoutError` branch
# was unreachable: zero 'timeout' rows across 18,785 executions and
# eight months of history.
#
# Re-raised rather than returned, because the distinction is the
# point: "too slow for its window" and "broken" call for different
# responses and were indistinguishable in the record.
raise
except ModuleNotFoundError:
return None, f"Executor module not found: {executor_name}"
except Exception as e:
return None, f"Executor error: {str(e)}\n{traceback.format_exc()}"
def _update_task_outcome(self, task_id: int, status: str, started_at: datetime):
"""Mirror a terminal outcome onto scheduled_tasks.
The happy path writes task_executions and scheduled_tasks in one
transaction. The error branches historically wrote only the former, which
did not show while every timeout was being funnelled through the happy
path as a 'failed'. Once a branch bypasses that path it has to keep
last_run/last_status current itself, or the task list quietly reports the
previous run's outcome as though it were the latest.
"""
try:
completed_at = datetime.now(timezone.utc)
with self.get_db_connection() as conn:
with conn.cursor() as cur:
cur.execute("""
UPDATE scheduled_tasks
SET last_run = %s, last_status = %s,
last_duration_seconds = %s, updated_at = %s
WHERE id = %s
""", (completed_at, status,
int((completed_at - started_at).total_seconds()),
completed_at, task_id))
conn.commit()
except Exception as e:
logger.error(f"Failed to update task outcome for {task_id}: {e}")
def _update_execution_status(self, execution_id: int, status: str, error: str = None):
"""Update execution record with final status."""
if execution_id is None:
tests/conftest.py
+17 -1
View File
@@ -20,7 +20,7 @@ os.environ["GITEA_USER"] = "test-librarian"
os.environ["GITEA_PASSWORD"] = "test-gitea-token"
os.environ["REDIS_HOST"] = "redis-shared"
from src.main import app
from src.main import app, get_task_executor
from src.config import Settings, get_settings
@@ -44,6 +44,22 @@ def auth_headers(api_key: str) -> dict:
return {"Authorization": f"Bearer {api_key}"}
# FastAPI resolves `Depends(get_task_executor)` against the function object it
# captured when each route was decorated, at import time. `unittest.mock.patch`
# on the module attribute `src.main.get_task_executor` therefore never reaches
# an already-registered route — the route keeps calling the original function.
# `app.dependency_overrides` is FastAPI's own supported mechanism for this
# (already used correctly in test_task_delete.py); this fixture centralizes it
# so call sites just need the executor mock they want installed.
@pytest.fixture
def override_task_executor() -> Generator[MagicMock, None, None]:
"""Install a mock TaskExecutor as the live dependency for this test only."""
mock_executor = MagicMock()
app.dependency_overrides[get_task_executor] = lambda: mock_executor
yield mock_executor
app.dependency_overrides.pop(get_task_executor, None)
# Synchronous test client
@pytest.fixture
def client() -> Generator[TestClient, None, None]:
tests/test_api.py
+34 -30
View File
@@ -57,10 +57,9 @@ class TestAuthenticationEndpoints:
)
assert response.status_code == 403
def test_valid_api_key_allows_access(self, client: TestClient, auth_headers: dict):
def test_valid_api_key_allows_access(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test that valid API key allows access."""
with patch('src.main.get_task_executor') as mock_executor:
mock_executor.return_value.get_db_connection.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchall.return_value = []
override_task_executor.get_db_connection.return_value.__enter__.return_value.cursor.return_value.__enter__.return_value.fetchall.return_value = []
response = client.get("/tasks", headers=auth_headers)
# May fail with 500 due to DB, but should not be 401/403
@@ -72,8 +71,16 @@ class TestAuthenticationEndpoints:
class TestTaskEndpoints:
"""Tests for task management endpoints."""
def test_create_task_missing_fields_returns_400(self, client: TestClient, auth_headers: dict):
"""Test that creating task without required fields returns 400."""
def test_create_task_missing_fields_returns_422(self, client: TestClient, auth_headers: dict):
"""Test that creating task without required fields returns 422.
`task: TaskCreate` (src/main.py) is a plain Pydantic request body with
no custom validation — FastAPI's own dependency-resolution layer
rejects a request missing required fields before create_task's body
ever runs, and that layer always answers 422, never 400. There is no
code path in this repo that could produce 400 here; renamed rather
than asserting a status this endpoint cannot return.
"""
incomplete_task = {
"task_name": "test",
# Missing service, executor, priority
@@ -83,11 +90,10 @@ class TestTaskEndpoints:
headers=auth_headers,
json=incomplete_task
)
assert response.status_code == 400
assert response.status_code == 422
def test_create_task_with_valid_data(self, client: TestClient, auth_headers: dict, sample_task_data: dict):
def test_create_task_with_valid_data(self, client: TestClient, auth_headers: dict, sample_task_data: dict, override_task_executor: MagicMock):
"""Test creating a task with valid data."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
@@ -96,8 +102,8 @@ class TestTaskEndpoints:
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.post(
"/tasks",
@@ -111,9 +117,8 @@ class TestTaskEndpoints:
call_args = mock_cursor.execute.call_args
assert 'config' in call_args[0][1]
def test_trigger_task_endpoint(self, client: TestClient, auth_headers: dict):
def test_trigger_task_endpoint(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test manually triggering a task."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
@@ -127,8 +132,8 @@ class TestTaskEndpoints:
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
with patch('asyncio.create_task'):
response = client.post(
@@ -152,13 +157,14 @@ class TestStatsEndpoint:
response = client.get("/stats")
assert response.status_code == 401
def test_stats_endpoint_returns_metrics(self, client: TestClient, auth_headers: dict):
def test_stats_endpoint_returns_metrics(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test that stats endpoint returns system metrics."""
with patch('src.main.get_scheduler') as mock_scheduler, \
patch('src.main.get_task_executor') as mock_executor:
mock_scheduler.return_value.running = True
# Note: `/stats` also depends on get_scheduler via Depends(), which this
# test does not override (out of this fix's measured scope — see
# override_task_executor's docstring for why patch() cannot reach it).
# It is not load-bearing here: the app's real scheduler is running by
# the time TestClient's lifespan completes, so `sched.running` is True
# without an override, same as test_health_endpoint_returns_healthy.
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.side_effect = [
@@ -172,8 +178,8 @@ class TestStatsEndpoint:
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get("/stats", headers=auth_headers)
@@ -189,9 +195,8 @@ class TestStatsEndpoint:
class TestExecutionHistoryEndpoint:
"""Tests for /executions endpoint."""
def test_executions_endpoint_returns_history(self, client: TestClient, auth_headers: dict):
def test_executions_endpoint_returns_history(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test that executions endpoint returns execution history."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchall.return_value = [
@@ -205,8 +210,8 @@ class TestExecutionHistoryEndpoint:
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get("/executions", headers=auth_headers)
@@ -215,17 +220,16 @@ class TestExecutionHistoryEndpoint:
assert "executions" in data
assert "count" in data
def test_executions_filter_by_task_name(self, client: TestClient, auth_headers: dict):
def test_executions_filter_by_task_name(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test filtering executions by task name."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchall.return_value = []
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get(
"/executions?task_name=test_task&limit=10",
tests/test_api_comprehensive.py
+37 -49
View File
@@ -3,7 +3,7 @@ Comprehensive API tests to improve coverage of main.py.
"""
import pytest
from fastapi.testclient import TestClient
from unittest.mock import patch, MagicMock
from unittest.mock import MagicMock
import json
@@ -12,17 +12,16 @@ import json
class TestTaskCRUDOperations:
"""Comprehensive CRUD tests for task endpoints."""
def test_list_tasks_empty(self, client: TestClient, auth_headers: dict):
def test_list_tasks_empty(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test listing tasks when none exist."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchall.return_value = []
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get("/tasks", headers=auth_headers)
@@ -31,17 +30,16 @@ class TestTaskCRUDOperations:
assert "tasks" in data
assert data["count"] == 0
def test_list_tasks_with_filters(self, client: TestClient, auth_headers: dict):
def test_list_tasks_with_filters(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test listing tasks with enabled and service filters."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchall.return_value = []
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get(
"/tasks?enabled=true&service=scheduler",
@@ -53,25 +51,23 @@ class TestTaskCRUDOperations:
call_args = str(mock_cursor.execute.call_args)
assert "enabled" in call_args.lower() or response.status_code in [200, 500]
def test_get_task_details_not_found(self, client: TestClient, auth_headers: dict):
def test_get_task_details_not_found(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test getting details for non-existent task."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.return_value = None
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get("/tasks/nonexistent", headers=auth_headers)
assert response.status_code == 404
def test_update_task(self, client: TestClient, auth_headers: dict):
def test_update_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test updating a task."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.return_value = {
@@ -81,8 +77,8 @@ class TestTaskCRUDOperations:
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.put(
"/tasks/test",
@@ -93,17 +89,16 @@ class TestTaskCRUDOperations:
# Should have attempted update
assert mock_cursor.execute.called
def test_update_task_not_found(self, client: TestClient, auth_headers: dict):
def test_update_task_not_found(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test updating non-existent task."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.return_value = None
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.put(
"/tasks/nonexistent",
@@ -123,17 +118,16 @@ class TestTaskCRUDOperations:
assert response.status_code == 400
def test_delete_task(self, client: TestClient, auth_headers: dict):
def test_delete_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test deleting a task."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.return_value = ("test_task",)
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.delete("/tasks/test_task", headers=auth_headers)
@@ -141,17 +135,16 @@ class TestTaskCRUDOperations:
data = response.json()
assert "deleted successfully" in data["message"].lower()
def test_delete_task_not_found(self, client: TestClient, auth_headers: dict):
def test_delete_task_not_found(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test deleting non-existent task."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.return_value = None
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.delete("/tasks/nonexistent", headers=auth_headers)
@@ -163,9 +156,8 @@ class TestTaskCRUDOperations:
class TestTriggerEndpoint:
"""Tests for task trigger endpoint."""
def test_trigger_disabled_task(self, client: TestClient, auth_headers: dict):
def test_trigger_disabled_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test triggering a disabled task."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.return_value = {
@@ -175,24 +167,23 @@ class TestTriggerEndpoint:
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.post("/tasks/test/trigger", headers=auth_headers)
assert response.status_code == 400
def test_trigger_nonexistent_task(self, client: TestClient, auth_headers: dict):
def test_trigger_nonexistent_task(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test triggering a task that doesn't exist."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchone.return_value = None
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.post("/tasks/nonexistent/trigger", headers=auth_headers)
@@ -254,17 +245,16 @@ class TestLegacyEndpoints:
class TestExecutionFiltering:
"""Tests for execution history filtering."""
def test_filter_by_service(self, client: TestClient, auth_headers: dict):
def test_filter_by_service(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test filtering executions by service."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchall.return_value = []
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get(
"/executions?service=scheduler",
@@ -273,17 +263,16 @@ class TestExecutionFiltering:
assert mock_cursor.execute.called
def test_filter_by_status(self, client: TestClient, auth_headers: dict):
def test_filter_by_status(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test filtering executions by status."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchall.return_value = []
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get(
"/executions?status=success",
@@ -292,17 +281,16 @@ class TestExecutionFiltering:
assert mock_cursor.execute.called
def test_custom_limit(self, client: TestClient, auth_headers: dict):
def test_custom_limit(self, client: TestClient, auth_headers: dict, override_task_executor: MagicMock):
"""Test custom limit for executions."""
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
mock_cursor.fetchall.return_value = []
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
response = client.get("/executions?limit=50", headers=auth_headers)
tests/test_config_backup_executor.py
+199 -172
View File
@@ -1,214 +1,241 @@
"""Tests for the config backup executor.
These replace a set that arrived with the portainer-core extraction and had
never passed in this repo: they patched the `Path` class wholesale, asserted
`"backed up" in result` against a function that returns `"Backup completed: …"`,
and one wrapped its only call in `except Exception: pass` with its assertion
commented out. Seven were red from the initial commit onward, and there is no CI
test gate here to notice (see CLAUDE.md).
The replacements use real directories and real archives under `tmp_path`. A
backup executor's whole job is what ends up inside the tar, and mocking
`tarfile` means nothing is checked.
"""
Tests for the config backup executor.
"""
import pytest
import asyncio
import os
import tarfile
import time
from pathlib import Path
from unittest.mock import AsyncMock, patch, MagicMock, mock_open
from src.executors import config_backup_executor
import pytest
from src.config import Settings
from src.executors import config_backup_executor
def _make_source(root: Path) -> Path:
"""A source tree with one file per exclude pattern the deployment uses."""
src = root / "docker-data"
(src / "ollama" / "models" / "blobs").mkdir(parents=True)
(src / "svc" / "cache").mkdir(parents=True)
(src / "svc" / "logs").mkdir(parents=True)
(src / "keep.conf").write_text("keep me")
(src / "svc" / "settings.json").write_text("keep me too")
(src / "ollama" / "models" / "blobs" / "sha256-abc").write_text("many GB in reality")
(src / "svc" / "cache" / "junk.bin").write_text("disposable")
(src / "svc" / "logs" / "app.log").write_text("noisy")
return src
def _members(archive: Path) -> set:
"""Names inside the inner per-source tar of a combined backup archive."""
with tarfile.open(archive, "r:gz") as outer:
inner_name = outer.getnames()[0]
fh = outer.extractfile(inner_name)
with tarfile.open(fileobj=fh, mode="r:gz") as inner:
return set(inner.getnames())
@pytest.mark.executor
@pytest.mark.unit
class TestConfigBackupExecutor:
"""Tests for config_backup_executor module."""
class TestConfigBackup:
@pytest.mark.asyncio
async def test_executor_requires_config_fields(self, test_settings: Settings):
"""Test that executor validates required config."""
incomplete_config = {
"sources": []
# Missing backup_dir
}
async def test_no_sources_is_rejected(self, test_settings: Settings, tmp_path: Path):
with pytest.raises(ValueError, match="No backup sources"):
await config_backup_executor._run({"backup_dir": str(tmp_path)}, test_settings)
with pytest.raises((ValueError, KeyError)):
await config_backup_executor.execute(incomplete_config, test_settings)
def test_archive_contains_the_source(self, tmp_path: Path):
src = _make_source(tmp_path)
out = tmp_path / "backups"
summary = config_backup_executor._backup({
"sources": [{"path": str(src), "name": "docker-data", "excludes": []}],
"backup_dir": str(out),
})
@pytest.mark.asyncio
async def test_executor_with_minimal_config(self, test_settings: Settings, tmp_path: Path):
"""Test executor with minimal valid configuration."""
backup_dir = tmp_path / "backups"
backup_dir.mkdir()
assert "Backup completed" in summary
archives = list(out.glob("docker-configs-*.tar.gz"))
assert len(archives) == 1
assert "docker-data/keep.conf" in _members(archives[0])
source_dir = tmp_path / "source"
source_dir.mkdir()
(source_dir / "test.txt").write_text("test content")
config = {
def test_excludes_keep_matching_members_out(self, tmp_path: Path):
"""The patterns here are the ones the deployed task actually carries."""
src = _make_source(tmp_path)
out = tmp_path / "backups"
config_backup_executor._backup({
"sources": [{
"path": str(source_dir),
"name": "test-source",
"excludes": []
"path": str(src),
"name": "docker-data",
"excludes": ["*/cache/*", ".log", "ollama/models/*"],
}],
"backup_dir": str(backup_dir),
"compress": True,
"retention_days": 30
}
"backup_dir": str(out),
})
with patch('src.executors.config_backup_executor.Path') as mock_path_cls:
# Setup path mocking
mock_source = MagicMock()
mock_source.exists.return_value = True
mock_source.is_dir.return_value = True
mock_source.iterdir.return_value = [MagicMock(name="test.txt")]
names = _members(next(iter(out.glob("docker-configs-*.tar.gz"))))
assert "docker-data/keep.conf" in names
assert "docker-data/svc/settings.json" in names
assert "docker-data/svc/cache/junk.bin" not in names
assert "docker-data/svc/logs/app.log" not in names
assert "docker-data/ollama/models/blobs/sha256-abc" not in names
mock_backup = MagicMock()
mock_backup.mkdir = MagicMock()
def test_missing_source_is_skipped_not_fatal(self, tmp_path: Path):
src = _make_source(tmp_path)
out = tmp_path / "backups"
summary = config_backup_executor._backup({
"sources": [
{"path": str(tmp_path / "does-not-exist"), "name": "gone", "excludes": []},
{"path": str(src), "name": "docker-data", "excludes": []},
],
"backup_dir": str(out),
})
assert "docker-data" in summary
assert "gone" not in summary
def path_side_effect(p):
if str(p) == str(source_dir):
return mock_source
elif str(p) == str(backup_dir):
return mock_backup
return MagicMock()
def test_cleanup_removes_only_expired_backups(self, tmp_path: Path):
out = tmp_path / "backups"
out.mkdir()
old = out / "docker-configs-20200101-000000.tar.gz"
recent = out / "docker-configs-20991231-000000.tar.gz"
unrelated = out / "notes.txt"
for f in (old, recent, unrelated):
f.write_text("x")
mock_path_cls.side_effect = path_side_effect
long_ago = time.time() - (30 * 86400)
os.utime(old, (long_ago, long_ago))
with patch('tarfile.open'), \
patch('src.executors.config_backup_executor._cleanup_old_backups'):
config_backup_executor._cleanup_old_backups(out, retention_days=7)
result = await config_backup_executor.execute(config, test_settings)
assert not old.exists()
assert recent.exists()
assert unrelated.exists(), "cleanup must only touch files it wrote"
assert "backed up" in result.lower() or "success" in result.lower()
@pytest.mark.executor
@pytest.mark.unit
class TestEventLoopIsNotBlocked:
"""T-74. The scheduler serves its own API from the loop that runs executors.
This job spends ~21 minutes in tarfile and zlib, so calling it inline made
the whole service unreachable 03:05-03:25 every night. The hourly health
check runs at :35 and so never once observed it — the outage was invisible
for as long as it existed.
"""
@pytest.mark.asyncio
async def test_executor_excludes_patterns(self, test_settings: Settings, tmp_path: Path):
"""Test that executor respects exclude patterns."""
backup_dir = tmp_path / "backups"
source_dir = tmp_path / "source"
async def test_run_yields_to_the_loop_while_backing_up(
self, test_settings: Settings, monkeypatch
):
blocked_for = 0.4
monkeypatch.setattr(
config_backup_executor, "_backup",
lambda config: (time.sleep(blocked_for), "Backup completed: fake")[1],
)
config = {
"sources": [{
"path": str(source_dir),
"name": "test",
"excludes": ["*.log", "cache/*"]
}],
"backup_dir": str(backup_dir),
"compress": True
}
ticks = 0
with patch('src.executors.config_backup_executor.Path'), \
patch('tarfile.open') as mock_tar, \
patch('src.executors.config_backup_executor._cleanup_old_backups'):
# Mock tarfile
mock_tar_obj = MagicMock()
mock_tar.return_value.__enter__ = MagicMock(return_value=mock_tar_obj)
mock_tar.return_value.__exit__ = MagicMock(return_value=None)
async def heartbeat():
nonlocal ticks
while True:
await asyncio.sleep(0.01)
ticks += 1
hb = asyncio.create_task(heartbeat())
try:
await config_backup_executor.execute(config, test_settings)
except Exception:
# May fail due to mocking complexity, but that's ok
pass
result = await config_backup_executor._run({}, test_settings)
finally:
hb.cancel()
# Should have attempted to create tarfile
# assert mock_tar.called # Would check if it was actually called
@pytest.mark.asyncio
async def test_executor_handles_missing_source(self, test_settings: Settings, tmp_path: Path):
"""Test executor handles missing source directory."""
backup_dir = tmp_path / "backups"
backup_dir.mkdir()
config = {
"sources": [{
"path": "/nonexistent/path",
"name": "missing",
"excludes": []
}],
"backup_dir": str(backup_dir),
"compress": True
}
with patch('src.executors.config_backup_executor.Path') as mock_path_cls:
mock_source = MagicMock()
mock_source.exists.return_value = False
mock_path_cls.return_value = mock_source
result = await config_backup_executor.execute(config, test_settings)
# Should skip non-existent sources
assert "skipped" in result.lower() or "not found" in result.lower() or "0" in result
@pytest.mark.asyncio
async def test_cleanup_old_backups(self, tmp_path: Path):
"""Test cleanup of old backup files."""
backup_dir = tmp_path / "backups"
backup_dir.mkdir()
# Create some "old" backup files
old_backup = backup_dir / "backup-2020-01-01.tar.gz"
old_backup.write_text("old")
recent_backup = backup_dir / "backup-2025-12-01.tar.gz"
recent_backup.write_text("recent")
with patch('src.executors.config_backup_executor.Path') as mock_path_cls:
mock_backup_dir = MagicMock()
mock_old_file = MagicMock()
mock_old_file.name = "backup-2020-01-01.tar.gz"
mock_old_file.stat.return_value.st_mtime = 0 # Very old
mock_recent_file = MagicMock()
mock_recent_file.name = "backup-2025-12-01.tar.gz"
mock_recent_file.stat.return_value.st_mtime = 999999999999 # Recent
mock_backup_dir.glob.return_value = [mock_old_file, mock_recent_file]
mock_path_cls.return_value = mock_backup_dir
config_backup_executor._cleanup_old_backups(mock_backup_dir, retention_days=7)
# Old file should be removed
mock_old_file.unlink.assert_called_once()
assert result == "Backup completed: fake"
# Held inline, the loop gets no scheduling opportunity at all and this is
# 0. Off the loop it is ~40. The bar is low on purpose: the distinction
# being drawn is "the loop ran" versus "the loop was dead", and a loaded
# CI box should not turn that into a flake.
assert ticks >= 5, f"event loop starved during backup: {ticks} ticks"
@pytest.mark.executor
@pytest.mark.unit
class TestConfigBackupHelpers:
"""Tests for helper functions."""
class TestExcludeMatching:
"""Substring matching, and why it must stay that way.
def test_tar_filter_excludes_cache(self):
"""Test that tar filter excludes cache directories."""
excludes = ["*/cache/*", "*.log"]
filter_func = config_backup_executor._create_tar_filter(excludes)
`_create_tar_filter` reduces each pattern to a literal core and asks whether
it appears anywhere in the member name. That reads like an unfinished glob,
and the obvious "improvement" is `fnmatch`. These tests exist to make that
change fail loudly, because the deployed config is written against these
semantics and `fnmatch` would silently stop excluding the largest things in
the tree.
"""
# Mock tarinfo for cache file
cache_tarinfo = MagicMock()
cache_tarinfo.name = "data/cache/temp.txt"
def test_mid_path_fragment_matches_anywhere(self):
"""`ollama/models/*` must exclude a member named `docker-data/ollama/...`.
result = filter_func(cache_tarinfo)
assert result is None # Should exclude
Under fnmatch the pattern anchors at the start of the name, does not
match, and many GB of model blobs enter the nightly archive.
"""
f = config_backup_executor._create_tar_filter(["ollama/models/*"])
def test_tar_filter_includes_normal_files(self):
"""Test that tar filter includes normal files."""
excludes = ["*/cache/*"]
filter_func = config_backup_executor._create_tar_filter(excludes)
class TI:
name = "docker-data/ollama/models/blobs/sha256-abc"
# Mock tarinfo for normal file
normal_tarinfo = MagicMock()
normal_tarinfo.name = "data/config.json"
assert f(TI()) is None
result = filter_func(normal_tarinfo)
assert result == normal_tarinfo # Should include
def test_bare_suffix_matches_every_file_carrying_it(self):
"""The deployment excludes logs by the bare string `.log`, not `*.log`.
def test_tar_filter_with_wildcard_patterns(self):
"""Test tar filter with various wildcard patterns."""
excludes = ["*.log", "*.tmp", "temp/*"]
filter_func = config_backup_executor._create_tar_filter(excludes)
Under fnmatch this matches only a file named exactly `.log`, so every
real log file starts being archived.
"""
f = config_backup_executor._create_tar_filter([".log"])
# Log file
log_tarinfo = MagicMock()
log_tarinfo.name = "app.log"
assert filter_func(log_tarinfo) is None
class TI:
name = "docker-data/svc/logs/app.log"
# Temp file
tmp_tarinfo = MagicMock()
tmp_tarinfo.name = "cache.tmp"
assert filter_func(tmp_tarinfo) is None
assert f(TI()) is None
# Normal file
normal_tarinfo = MagicMock()
normal_tarinfo.name = "config.json"
assert filter_func(normal_tarinfo) == normal_tarinfo
def test_wrapped_pattern_is_reduced_to_its_core(self):
f = config_backup_executor._create_tar_filter(["*/cache/*"])
class Cache:
name = "docker-data/svc/cache/junk.bin"
class Normal:
name = "docker-data/svc/settings.json"
normal = Normal()
assert f(Cache()) is None
assert f(normal) is normal
def test_a_glob_star_is_not_interpreted(self):
"""`*.log` is a literal here — it is not a suffix match.
This is the sharp edge of substring matching and the reason the deployed
config spells the pattern `.log`. Pinned so the behaviour is documented
rather than discovered.
"""
f = config_backup_executor._create_tar_filter(["*.log"])
class TI:
name = "app.log"
ti = TI()
assert f(ti) is ti
def test_no_excludes_keeps_everything(self):
f = config_backup_executor._create_tar_filter([])
class TI:
name = "anything/at/all"
ti = TI()
assert f(ti) is ti
tests/test_doc_sync_executor.py
+50 -13
View File
@@ -60,23 +60,36 @@ class TestDocSyncExecutor:
mock_upstream_dir = MagicMock()
mock_gitea_dir = MagicMock()
# Setup directory mocking
mock_upstream_dir.iterdir.return_value = [
MagicMock(name=".git", is_dir=lambda: True),
MagicMock(name="README.md", is_dir=lambda: False),
MagicMock(name="docs", is_dir=lambda: True),
]
mock_gitea_dir.iterdir.return_value = [
MagicMock(name=".git", is_dir=lambda: True)
]
# Setup directory mocking. MagicMock(name=...) sets the mock's
# repr, not its .name attribute (classic gotcha — see
# test_executor_sync_specific_paths) — set .name explicitly so
# execute()'s `item.name != '.git'` check actually excludes it.
git_item = MagicMock(is_dir=lambda: True)
git_item.name = ".git"
readme_item = MagicMock(is_dir=lambda: False)
readme_item.name = "README.md"
docs_item = MagicMock(is_dir=lambda: True)
docs_item.name = "docs"
mock_upstream_dir.iterdir.return_value = [git_item, readme_item, docs_item]
gitea_git_item = MagicMock(is_dir=lambda: True)
gitea_git_item.name = ".git"
mock_gitea_dir.iterdir.return_value = [gitea_git_item]
mock_path.return_value = mock_work_dir
mock_work_dir.__truediv__.side_effect = [mock_upstream_dir, mock_gitea_dir]
# Mock git status to show changes
# Mock git status to show changes. _get_git_commit is patched
# separately above and never calls the real _run_command, so it
# does not consume a slot in this side_effect list — the actual
# call order for this (clone-succeeds, entire-repo) path is:
# clone upstream, clone gitea, add, status, commit, tag, push
# branch, push tag. The list previously reserved a slot for
# "git rev-parse HEAD" that _run_command is never asked for,
# which shifted "M README.md\n" one call late and made
# `git status --porcelain` see "" (no changes) instead.
mock_run.side_effect = [
"", # git clone upstream
"", # git rev-parse HEAD
"", # git clone gitea
"", # git add
"M README.md\n", # git status --porcelain (has changes)
@@ -108,7 +121,15 @@ class TestDocSyncExecutor:
mock_get_commit.return_value = "abc123"
# Mock path operations
# Mock path operations. work_dir = Path(...) resolves to
# mock_path.return_value — mock_upstream_dir/mock_gitea_dir have
# to be reachable from there via __truediv__, the same way
# test_executor_successful_sync_entire_repo wires it, or
# `upstream_dir / doc_path` never reaches these mocks at all and
# falls through to an unconfigured auto-generated MagicMock
# instead (observed failure: TypeError joining a MagicMock into
# ', '.join(copied_paths)).
mock_work_dir = MagicMock()
mock_upstream_dir = MagicMock()
mock_gitea_dir = MagicMock()
mock_docs = MagicMock(name="docs")
@@ -121,6 +142,8 @@ class TestDocSyncExecutor:
mock_examples.is_dir.return_value = True
mock_examples.name = "examples"
mock_path.return_value = mock_work_dir
mock_work_dir.__truediv__.side_effect = [mock_upstream_dir, mock_gitea_dir]
mock_upstream_dir.__truediv__.side_effect = [mock_docs, mock_examples]
mock_gitea_dir.iterdir.return_value = []
@@ -159,13 +182,27 @@ class TestDocSyncExecutor:
mock_run.side_effect = run_command_side_effect
# Setup minimal mocking
# Setup minimal mocking. sample_doc_sync_config's docs_paths is
# ["/docs"] (tests/conftest.py), so execute() takes the
# specific-paths branch and needs `upstream_dir / "docs"` wired
# to something with a real string .name — see
# test_executor_sync_specific_paths for the same wiring gap and
# the TypeError it produces unwired.
mock_work_dir = MagicMock()
mock_upstream_dir = MagicMock()
mock_gitea_dir = MagicMock()
mock_gitea_dir.iterdir.return_value = []
mock_upstream_dir.iterdir.return_value = []
mock_docs = MagicMock()
mock_docs.exists.return_value = True
mock_docs.is_dir.return_value = True
mock_docs.name = "docs"
mock_path.return_value = mock_work_dir
mock_work_dir.__truediv__.side_effect = [mock_upstream_dir, mock_gitea_dir]
mock_upstream_dir.__truediv__.side_effect = [mock_docs]
result = await doc_sync_executor.execute(sample_doc_sync_config, test_settings)
assert "already up to date" in result.lower() or "no changes" in result.lower()
tests/test_docker_prune_executor.py
+135
View File
@@ -0,0 +1,135 @@
"""
Tests for the docker prune executor.
The important property is which stages run. `volumes` removes volumes belonging
to merely-stopped containers, so it must never be enabled by accident, and the
safe stages must stay on by default.
"""
from unittest.mock import AsyncMock, patch
import pytest
from src.config import Settings
from src.executors import docker_prune_executor as prune
def _runner(reclaimed="Total reclaimed space: 1.5GB", rc=0):
"""Fake _run returning docker-shaped output for every invocation."""
async def run(args):
if args[:3] == ["docker", "system", "df"]:
return 0, "TYPE TOTAL ACTIVE SIZE RECLAIMABLE", ""
return rc, reclaimed, "" if rc == 0 else "boom"
return run
@pytest.mark.executor
@pytest.mark.unit
class TestStageSelection:
@pytest.mark.asyncio
async def test_defaults_run_only_the_safe_stages(self, test_settings: Settings):
calls = []
async def run(args):
calls.append(args)
if args[:3] == ["docker", "system", "df"]:
return 0, "df output", ""
return 0, "Total reclaimed space: 0B", ""
with patch.object(prune, "_run", run):
await prune.execute({}, test_settings)
joined = [" ".join(c) for c in calls]
assert any("builder prune" in c for c in joined)
assert any("image prune -f" in c for c in joined)
# The destructive ones must not appear without being asked for.
assert not any("volume prune" in c for c in joined)
assert not any("image prune -a" in c for c in joined)
@pytest.mark.asyncio
async def test_volumes_only_when_explicitly_enabled(self, test_settings: Settings):
calls = []
async def run(args):
calls.append(args)
if args[:3] == ["docker", "system", "df"]:
return 0, "df output", ""
return 0, "Total reclaimed space: 2GB", ""
with patch.object(prune, "_run", run):
await prune.execute({"volumes": True}, test_settings)
assert any("volume prune" in " ".join(c) for c in calls)
@pytest.mark.asyncio
async def test_all_stages_disabled_is_a_no_op(self, test_settings: Settings):
with patch.object(prune, "_run", AsyncMock()) as run:
result = await prune.execute(
{"build_cache": False, "dangling_images": False}, test_settings
)
assert "nothing to do" in result
run.assert_not_called()
@pytest.mark.asyncio
async def test_dry_run_executes_no_prune(self, test_settings: Settings):
calls = []
async def run(args):
calls.append(args)
return 0, "df output", ""
with patch.object(prune, "_run", run):
result = await prune.execute({"dry_run": True}, test_settings)
assert "dry run" in result
assert all("prune" not in " ".join(c) for c in calls)
@pytest.mark.executor
@pytest.mark.unit
class TestFailureHandling:
@pytest.mark.asyncio
async def test_docker_unavailable_raises(self, test_settings: Settings):
async def run(args):
return 1, "", "Cannot connect to the Docker daemon"
with patch.object(prune, "_run", run):
with pytest.raises(Exception, match="docker unavailable"):
await prune.execute({}, test_settings)
@pytest.mark.asyncio
async def test_failed_stage_surfaces_but_others_still_run(self, test_settings: Settings):
attempted = []
async def run(args):
if args[:3] == ["docker", "system", "df"]:
return 0, "df output", ""
attempted.append(" ".join(args))
if "builder" in args:
return 1, "", "builder exploded"
return 0, "Total reclaimed space: 3MB", ""
with patch.object(prune, "_run", run):
with pytest.raises(Exception, match="one or more prune stages failed"):
await prune.execute({}, test_settings)
# The image stage must still have been attempted after builder failed.
assert any("image prune" in a for a in attempted)
@pytest.mark.executor
@pytest.mark.unit
class TestOutputParsing:
@pytest.mark.parametrize(
"output,expected",
[
("Total reclaimed space: 1.5GB", "1.5GB"),
("deleted: sha256:abc\nTotal reclaimed space: 0B", "0B"),
("no such line", "0B"),
("", "0B"),
],
)
def test_reclaimed_parsing(self, output, expected):
assert prune._reclaimed(output) == expected
tests/test_integration.py
+21 -40
View File
@@ -27,28 +27,34 @@ class TestDatabaseIntegration:
class TestEndToEndTaskFlow:
"""End-to-end tests for task lifecycle (mocked database)."""
def test_create_list_delete_task_flow(self, client: TestClient, auth_headers: dict, sample_task_data: dict):
def test_create_list_delete_task_flow(self, client: TestClient, auth_headers: dict, sample_task_data: dict, override_task_executor):
"""Test complete task lifecycle: create → list → delete."""
from unittest.mock import MagicMock
# This simulates the full flow with mocked database
with patch('src.main.get_task_executor') as mock_executor:
mock_conn = MagicMock()
mock_cursor = MagicMock()
# Mock create
# Mock create. list_tasks (GET /tasks) uses fetchall, not fetchone, so
# it does not consume a slot here — the previous list of 3 values was
# sized for a delete that has since grown a second lookup (T-97's
# 409-on-history-loss check, c34db66): delete_task now does
# `row = cur.fetchone()` for the task id, then a separate
# `cur.fetchone()[0]` for its execution count, so a real
# create->list->delete flow needs 1 (create) + 2 (delete) = 3
# fetchone() calls in that order, not create+list+delete.
created_task = {**sample_task_data, "id": 99}
mock_cursor.fetchone.side_effect = [
created_task, # Create task
created_task, # List tasks (as dict)
("test_task",) # Delete task
created_task, # create_task: INSERT ... RETURNING (dict row)
(99,), # delete_task: SELECT id FROM scheduled_tasks
(0,), # delete_task: SELECT COUNT(*) FROM task_executions — none, so it proceeds
]
mock_cursor.fetchall.return_value = [created_task]
mock_conn.cursor.return_value.__enter__ = MagicMock(return_value=mock_cursor)
mock_conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
mock_executor.return_value.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
mock_executor.return_value.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
override_task_executor.get_db_connection.return_value.__enter__ = MagicMock(return_value=mock_conn)
override_task_executor.get_db_connection.return_value.__exit__ = MagicMock(return_value=None)
# Create
create_response = client.post(
@@ -91,36 +97,6 @@ class TestTaskExecutorIntegration:
assert "Integration test" in result
assert "took" in result.lower()
@pytest.mark.asyncio
async def test_task_scheduling_logic(self, test_settings):
"""Test task scheduling logic."""
from src.tasks.executor import TaskExecutor
from datetime import datetime
executor = TaskExecutor(test_settings)
# Test various scheduling scenarios
task_every_minute = {
'minute': -1, 'hour': -1, 'day_of_month': -1,
'month': -1, 'day_of_week': -1
}
task_specific_time = {
'minute': 30, 'hour': 14, 'day_of_month': -1,
'month': -1, 'day_of_week': -1
}
now = datetime(2025, 12, 7, 14, 30, 0)
# Every minute task should always run
assert executor._should_run_now(task_every_minute, now) is True
# Specific time task should run at 14:30
assert executor._should_run_now(task_specific_time, now) is True
# But not at 14:31
now_plus_one = datetime(2025, 12, 7, 14, 31, 0)
assert executor._should_run_now(task_specific_time, now_plus_one) is False
@pytest.mark.integration
@@ -164,8 +140,13 @@ class TestConfigValidation:
settings = get_settings()
# Should have loaded test environment variables
assert settings.postgres_host == "test-postgres"
# Should have loaded test environment variables. tests/conftest.py
# sets POSTGRES_HOST="postgres-shared" (module-level, before `from
# src.main import app`), annotated "Use real postgres for integration
# tests" — this assertion checked for "test-postgres", a value
# nothing in the suite has ever set. Matched to the fixture actually
# in effect rather than to an unset value.
assert settings.postgres_host == "postgres-shared"
assert settings.postgres_db == "test_scheduler"
assert settings.scheduler_api_key == "test-api-key-12345"
tests/test_portainer_backup_executor.py
+175
View File
@@ -0,0 +1,175 @@
"""
Tests for the Portainer backup executor.
The point of this executor is producing an archive that will still open on the
day it is needed, so most of these cover the failure paths: a truncated body
behind a 200, a partial file left on disk, and retention deleting the wrong
thing.
"""
import gzip
import io
import tarfile
from datetime import datetime, timedelta, timezone
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from src.config import Settings
from src.executors import portainer_backup_executor as pbe
def _tar_gz_bytes(names=("compose/1/docker-compose.yml", "certs/cert.pem")) -> bytes:
buf = io.BytesIO()
with tarfile.open(fileobj=buf, mode="w:gz") as tar:
for n in names:
data = b"x"
info = tarfile.TarInfo(name=n)
info.size = len(data)
tar.addfile(info, io.BytesIO(data))
return buf.getvalue()
def _mock_post(status=200, content=None):
response = MagicMock()
response.status_code = status
response.content = content if content is not None else _tar_gz_bytes()
response.text = "error body"
client = MagicMock()
client.post = AsyncMock(return_value=response)
ctx = MagicMock()
ctx.__aenter__ = AsyncMock(return_value=client)
ctx.__aexit__ = AsyncMock(return_value=False)
return ctx, client
@pytest.mark.executor
@pytest.mark.unit
class TestConfigValidation:
@pytest.mark.asyncio
async def test_missing_url_rejected(self, test_settings: Settings, tmp_path):
with pytest.raises(ValueError, match="url"):
await pbe.execute({"api_key": "k", "output_dir": str(tmp_path)}, test_settings)
@pytest.mark.asyncio
async def test_missing_api_key_rejected(self, test_settings: Settings, tmp_path):
with pytest.raises(ValueError, match="api_key"):
await pbe.execute({"url": "http://x", "output_dir": str(tmp_path)}, test_settings)
@pytest.mark.asyncio
async def test_unresolved_env_var_rejected(self, test_settings: Settings, tmp_path, monkeypatch):
"""${VAR} that expands to nothing must fail, not send an empty key."""
monkeypatch.delenv("NOPE_MISSING", raising=False)
with pytest.raises(ValueError, match="api_key"):
await pbe.execute(
{"url": "http://x", "api_key": "${NOPE_MISSING}", "output_dir": str(tmp_path)},
test_settings,
)
@pytest.mark.asyncio
@pytest.mark.parametrize("bad", [0, -1, "30", None, True])
async def test_bad_retention_rejected(self, bad, test_settings: Settings, tmp_path):
with pytest.raises(ValueError, match="retention_days"):
await pbe.execute(
{"url": "http://x", "api_key": "k", "output_dir": str(tmp_path),
"retention_days": bad},
test_settings,
)
@pytest.mark.executor
@pytest.mark.unit
class TestBackupBehaviour:
def _config(self, tmp_path, **over):
cfg = {"url": "http://portainer:9000", "api_key": "k",
"output_dir": str(tmp_path), "retention_days": 30}
cfg.update(over)
return cfg
@pytest.mark.asyncio
async def test_writes_verified_archive(self, test_settings: Settings, tmp_path):
ctx, _ = _mock_post()
with patch("httpx.AsyncClient", return_value=ctx):
result = await pbe.execute(self._config(tmp_path), test_settings)
files = list(tmp_path.glob("portainer-*.tar.gz"))
assert len(files) == 1
assert "2 entries" in result
with tarfile.open(files[0], "r:gz") as tar: # opens = usable backup
assert "certs/cert.pem" in tar.getnames()
@pytest.mark.asyncio
async def test_archive_is_not_world_readable(self, test_settings: Settings, tmp_path):
"""It contains TLS private keys."""
ctx, _ = _mock_post()
with patch("httpx.AsyncClient", return_value=ctx):
await pbe.execute(self._config(tmp_path), test_settings)
f = next(tmp_path.glob("portainer-*.tar.gz"))
assert oct(f.stat().st_mode)[-3:] == "600"
@pytest.mark.asyncio
async def test_http_error_raises_and_leaves_nothing(self, test_settings: Settings, tmp_path):
ctx, _ = _mock_post(status=401)
with patch("httpx.AsyncClient", return_value=ctx):
with pytest.raises(Exception, match="HTTP 401"):
await pbe.execute(self._config(tmp_path), test_settings)
assert list(tmp_path.iterdir()) == []
@pytest.mark.asyncio
async def test_truncated_body_behind_200_is_rejected(self, test_settings: Settings, tmp_path):
"""The dangerous case: a 200 whose body is not a usable archive."""
broken = _tar_gz_bytes()[:40]
ctx, _ = _mock_post(content=broken)
with patch("httpx.AsyncClient", return_value=ctx):
with pytest.raises(Exception, match="not a readable archive"):
await pbe.execute(self._config(tmp_path), test_settings)
# no .partial and no final file left behind
assert list(tmp_path.iterdir()) == []
@pytest.mark.asyncio
async def test_gzip_that_is_not_a_tar_is_rejected(self, test_settings: Settings, tmp_path):
ctx, _ = _mock_post(content=gzip.compress(b"not a tar"))
with patch("httpx.AsyncClient", return_value=ctx):
with pytest.raises(Exception, match="not a readable archive"):
await pbe.execute(self._config(tmp_path), test_settings)
assert list(tmp_path.iterdir()) == []
@pytest.mark.asyncio
async def test_api_key_resolved_from_env(self, test_settings: Settings, tmp_path, monkeypatch):
monkeypatch.setenv("PT_KEY", "secret-value")
ctx, client = _mock_post()
with patch("httpx.AsyncClient", return_value=ctx):
await pbe.execute(self._config(tmp_path, api_key="${PT_KEY}"), test_settings)
assert client.post.call_args.kwargs["headers"]["X-API-Key"] == "secret-value"
@pytest.mark.executor
@pytest.mark.unit
class TestRetention:
def _age(self, path, days):
import os
old = (datetime.now(timezone.utc) - timedelta(days=days)).timestamp()
os.utime(path, (old, old))
def test_prunes_only_past_the_window(self, tmp_path):
fresh = tmp_path / "portainer-20260808T120000Z.tar.gz"
stale = tmp_path / "portainer-20260101T120000Z.tar.gz"
for f in (fresh, stale):
f.write_bytes(b"x")
self._age(stale, 45)
assert pbe._prune(tmp_path, 30) == 1
assert fresh.exists() and not stale.exists()
def test_leaves_unrelated_files_alone(self, tmp_path):
"""Retention must not touch anything it did not write."""
other = tmp_path / "important-database-dump.tar.gz"
named_alike = tmp_path / "portainer-backup-manual.tar.gz"
for f in (other, named_alike):
f.write_bytes(b"x")
self._age(f, 400)
assert pbe._prune(tmp_path, 30) == 0
assert other.exists() and named_alike.exists()
tests/test_postgres_retention_executor.py
+151
View File
@@ -0,0 +1,151 @@
"""
Tests for the postgres retention executor.
Focus is on the guards. The executor interpolates a table and column name
straight into SQL (they cannot be bound as parameters), and it issues DELETEs
against a live table, so the validation in front of both is what keeps a
malformed config from becoming data loss.
"""
from unittest.mock import MagicMock, patch
import pytest
from src.config import Settings
from src.executors import postgres_retention_executor as retention
@pytest.mark.executor
@pytest.mark.unit
class TestIdentifierValidation:
"""Table/column/database names are interpolated, so they must be rejected early."""
@pytest.mark.parametrize(
"bad",
[
"check_history; DROP TABLE users",
'check_history"',
"check history",
"Check_History", # uppercase would need quoting to resolve
"1_history",
"",
"--comment",
],
)
def test_rejects_unsafe_identifiers(self, bad):
with pytest.raises(ValueError):
retention._validate_identifier(bad, "table")
@pytest.mark.parametrize("good", ["check_history", "ts", "_private", "a1"])
def test_accepts_plain_identifiers(self, good):
assert retention._validate_identifier(good, "table") == good
@pytest.mark.executor
@pytest.mark.unit
class TestRetentionGuards:
"""A bad retention window must never reach the database."""
def _config(self, **overrides):
config = {
"database": "sysmon",
"table": "check_history",
"timestamp_column": "ts",
"retention_days": 30,
}
config.update(overrides)
return config
@pytest.mark.parametrize("days", [0, -1, -30, 3651])
def test_rejects_out_of_range_retention(self, days, test_settings: Settings):
# 0 or negative would delete every row including the one just written.
with patch("psycopg2.connect") as connect:
with pytest.raises(ValueError):
retention._prune(self._config(retention_days=days), test_settings)
connect.assert_not_called()
@pytest.mark.parametrize("days", ["30", None, 1.5, True])
def test_rejects_non_integer_retention(self, days, test_settings: Settings):
with patch("psycopg2.connect") as connect:
with pytest.raises(ValueError):
retention._prune(self._config(retention_days=days), test_settings)
connect.assert_not_called()
def test_rejects_injection_in_table_before_connecting(self, test_settings: Settings):
with patch("psycopg2.connect") as connect:
with pytest.raises(ValueError):
retention._prune(
self._config(table="check_history; DELETE FROM check_history --"),
test_settings,
)
connect.assert_not_called()
@pytest.mark.executor
@pytest.mark.unit
class TestRetentionBehaviour:
"""Behaviour against a mocked cursor."""
def _mock_conn(self, counts):
cursor = MagicMock()
cursor.fetchone.side_effect = [(c,) for c in counts]
cursor.rowcount = counts[0] if counts else 0
conn = MagicMock()
conn.cursor.return_value.__enter__.return_value = cursor
conn.__enter__.return_value = conn
return conn, cursor
def _config(self, **overrides):
config = {
"database": "sysmon",
"table": "check_history",
"timestamp_column": "ts",
"retention_days": 30,
}
config.update(overrides)
return config
def test_dry_run_does_not_delete(self, test_settings: Settings):
conn, cursor = self._mock_conn([7])
with patch("psycopg2.connect", return_value=conn):
result = retention._prune(self._config(dry_run=True), test_settings)
assert "dry run" in result
assert "7" in result
executed = " ".join(str(c) for c in cursor.execute.call_args_list)
assert "DELETE" not in executed.upper()
def test_no_stale_rows_skips_delete(self, test_settings: Settings):
conn, cursor = self._mock_conn([0])
with patch("psycopg2.connect", return_value=conn):
result = retention._prune(self._config(), test_settings)
assert "nothing to prune" in result
executed = " ".join(str(c) for c in cursor.execute.call_args_list)
assert "DELETE" not in executed.upper()
def test_deletes_and_reports(self, test_settings: Settings):
# count(stale) -> 5, then count(remaining) -> 42
conn, cursor = self._mock_conn([5, 42])
cursor.rowcount = 5
with patch("psycopg2.connect", return_value=conn):
result = retention._prune(self._config(), test_settings)
assert "pruned 5 rows" in result
assert "42 remain" in result
executed = " ".join(str(c) for c in cursor.execute.call_args_list)
assert "DELETE" in executed.upper()
def test_defaults_to_scheduler_database(self, test_settings: Settings):
conn, _ = self._mock_conn([0])
config = self._config()
del config["database"]
with patch("psycopg2.connect", return_value=conn) as connect:
retention._prune(config, test_settings)
assert connect.call_args.kwargs["database"] == test_settings.postgres_db
@pytest.mark.asyncio
async def test_execute_wraps_prune(self, test_settings: Settings):
conn, _ = self._mock_conn([0])
with patch("psycopg2.connect", return_value=conn):
result = await retention.execute(self._config(), test_settings)
assert "nothing to prune" in result
tests/test_rest_api_executor.py
+22 -2
View File
@@ -498,7 +498,24 @@ class TestRestApiExecutor:
assert redacted["normal_field"] == "visible"
def test_redact_sensitive_nested(self):
"""Test redaction in nested structures."""
"""A dict under a sensitive key is redacted whole, not recursed into.
This asserted fine-grained recursion — that `auth.token` was replaced
while `auth`'s other keys stayed readable — and had never passed. The
source redacts the entire value the moment the KEY matches, so
`redacted["config"]["auth"]` is the string, and indexing `["token"]`
into it raises TypeError.
Settled in favour of the source. Fine-grained redaction has to know
which sub-keys carry the secret, which is a guess about the shape of
data nobody has inspected; redacting on the key cannot be wrong that
way. Real configs here look like
{"auth": {"type": "bearer", "token": "${SOME_API_KEY}"}}, and the cost
of guessing wrong is a credential in a log, which no later fix undoes.
The price is readability: a reader learns that auth was present, not
that it was bearer. That is the trade being made deliberately.
"""
data = {
"config": {
"database": "mydb",
@@ -513,7 +530,10 @@ class TestRestApiExecutor:
assert redacted["config"]["database"] == "mydb"
assert redacted["config"]["password"] == "***REDACTED***"
assert redacted["config"]["auth"]["token"] == "***REDACTED***"
# The whole sub-dict, not a recursed copy of it.
assert redacted["config"]["auth"] == "***REDACTED***"
# And the secret is nowhere in the output, by any path.
assert "bearer123" not in str(redacted)
# Helper Function Tests
tests/test_task_attribution.py
+189
View File
@@ -0,0 +1,189 @@
"""A check_history row must say which task produced it.
On 2026-08-11 a 425 MB probe task and the 5 GB nightly backup both ran through
config_backup_executor. The probe failed, and the row it wrote was
indistinguishable from a nightly-backup failure — same `source`, same `domain`,
nothing naming the task. The health record could not answer which job broke,
which is most of what a health record is for.
Executors are called as `execute(config, settings)` and are never told which
task they are, so the name travels in a ContextVar. These tests pin what makes
that safe: it reaches the reporter, it survives the worker thread T-74
introduced, and concurrent executions cannot read each other's. They also pin
where the summary lives, since two producers write this table.
"""
import asyncio
import json
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
from src.config import Settings
from src.executors import health_report
from src.task_context import current_task_name, task_scope
from src.tasks.executor import TaskExecutor
@pytest.fixture
def captured_row(monkeypatch):
"""Capture the JSON payload report() would insert, without a database."""
box = {}
conn, cur = MagicMock(), MagicMock()
def execute(sql, params):
box['result'] = json.loads(params[4])
cur.execute.side_effect = execute
conn.cursor.return_value.__enter__ = MagicMock(return_value=cur)
conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
conn.__enter__ = MagicMock(return_value=conn)
conn.__exit__ = MagicMock(return_value=None)
monkeypatch.setattr(health_report.psycopg2, 'connect', lambda **kw: conn)
return box
def _report(settings, **kw):
health_report.report(
settings, domain="backup", status=health_report.OK,
source="scheduler/config_backup_executor",
summary="backed up 3 sources", metrics={}, **kw
)
@pytest.mark.unit
class TestAttribution:
def test_the_row_names_the_task(self, test_settings: Settings, captured_row):
with task_scope("backup_docker_configs_daily"):
_report(test_settings)
assert captured_row['result']['task'] == "backup_docker_configs_daily"
def test_source_still_names_the_code(self, test_settings: Settings, captured_row):
"""Task and source answer different questions; both are needed.
`source` says which code wrote the row, `task` says which schedule
invoked it. Replacing one with the other loses a distinction.
"""
with task_scope("t74_loop_probe"):
_report(test_settings)
r = captured_row['result']
assert r['source'] == "scheduler/config_backup_executor"
assert r['task'] == "t74_loop_probe"
def test_the_field_is_omitted_rather_than_nulled(self, test_settings: Settings, captured_row):
"""report() is callable from a script with no task around it.
A null would claim there was a task and it had no name. Absence says the
question does not apply.
"""
_report(test_settings)
assert 'task' not in captured_row['result']
@pytest.mark.asyncio
async def test_attribution_survives_the_worker_thread(
self, test_settings: Settings, captured_row
):
"""T-74 moved reporting into asyncio.to_thread. Attribution has to follow.
If the context did not propagate, every row written by a real executor
would silently lose its task while the tests above still passed.
"""
with task_scope("backup_portainer_daily"):
await health_report.report_async(
test_settings, domain="backup", status=health_report.OK,
source="scheduler/portainer_backup_executor",
summary="backed up Portainer", metrics={},
)
assert captured_row['result']['task'] == "backup_portainer_daily"
@pytest.mark.unit
class TestAttributionIsolation:
"""MAX_CONCURRENT_TASKS is 5, so mixing values between them is a live risk."""
def _executor_with_db(self, test_settings):
executor = TaskExecutor(test_settings)
conn, cur = MagicMock(), MagicMock()
cur.fetchone.return_value = [1]
conn.cursor.return_value.__enter__ = MagicMock(return_value=cur)
conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
conn.__enter__ = MagicMock(return_value=conn)
conn.__exit__ = MagicMock(return_value=None)
return executor, patch.object(executor, 'get_db_connection', return_value=conn)
def _task(self, name):
return {'id': hash(name) % 1000, 'task_name': name, 'executor': 'x',
'service': 'scheduler', 'priority': 5, 'timeout_seconds': 60}
@pytest.mark.asyncio
async def test_the_engine_names_the_task_and_clears_it_after(
self, test_settings: Settings
):
executor, db = self._executor_with_db(test_settings)
seen = {}
async def fake_run(name, task, timeout):
seen['during'] = current_task_name()
return "done", None
with db, patch.object(executor, '_run_executor', new=fake_run):
await executor.execute_task(self._task("nightly_backup"))
assert seen['during'] == "nightly_backup", "executor ran unattributed"
assert current_task_name() is None, "attribution leaked past the execution"
@pytest.mark.asyncio
async def test_concurrent_executions_do_not_see_each_other(
self, test_settings: Settings
):
executor, db = self._executor_with_db(test_settings)
seen = {}
async def fake_run(name, task, timeout):
who = task['task_name']
# Yield mid-flight so the executions genuinely interleave; without
# this they would run to completion one at a time and the test would
# pass even with a shared global.
await asyncio.sleep(0.01 if who == "slow_one" else 0)
seen[who] = current_task_name()
return "done", None
with db, patch.object(executor, '_run_executor', new=fake_run):
await asyncio.gather(
executor.execute_task(self._task("slow_one")),
executor.execute_task(self._task("fast_one")),
)
assert seen == {"slow_one": "slow_one", "fast_one": "fast_one"}
@pytest.mark.unit
class TestSummaryPlacement:
"""The two writers of check_history must agree where the substance lives.
sysmon-go writes `summary` at the top level, beside `status`. This module
wrote it under `metrics` until 2026-08-11, so a reader had to know which
producer wrote a row before it could find out what the row said — and a
query written the obvious way silently found half the data. That is the T-36
failure exactly, where per-domain queries returned nothing because the value
was nested somewhere else.
"""
def test_summary_is_top_level(self, test_settings: Settings, captured_row):
_report(test_settings)
r = captured_row['result']
assert r['summary'] == "backed up 3 sources"
assert 'summary' not in r['metrics'], "summary must not also live under metrics"
def test_summary_is_required(self, test_settings: Settings, captured_row):
"""Omitting it is an error at the call, not a silently empty column.
sysmon-go enforces this through Domain.Run's signature; a parameter with
no default is the equivalent here. A row whose substance is missing looks
exactly like a row whose check found nothing to say.
"""
with pytest.raises(TypeError):
health_report.report(
test_settings, domain="backup", status=health_report.OK,
source="scheduler/x", metrics={},
)
tests/test_task_delete.py
+112
View File
@@ -0,0 +1,112 @@
"""Deleting a task must not destroy its history by accident, or 500 by surprise.
DELETE /tasks/{name} used to issue a bare DELETE against scheduled_tasks. Any
task that had ever run owned rows in task_executions, so the foreign key
rejected it and the caller got "Internal Server Error" with nothing pointing at
history as the obstacle — it read as the service being broken rather than the
request being refusable. Since every task that has ever fired has history, the
endpoint effectively worked only for tasks that never ran.
It now refuses with 409 and takes ?purge=true to mean it. The asymmetry is the
argument: a task definition can be recreated from the API in one call, its
execution history cannot be recreated at all.
"""
import pytest
from fastapi.testclient import TestClient
from unittest.mock import MagicMock
from src.main import app, get_task_executor
def _fake_executor(task_row, execution_count=0):
"""A stand-in whose cursor answers the endpoint's two lookups in order."""
ex = MagicMock()
conn, cur = MagicMock(), MagicMock()
cur.fetchone.side_effect = (
[task_row, (execution_count,)] if task_row is not None else [None]
)
conn.cursor.return_value.__enter__ = MagicMock(return_value=cur)
conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
conn.__enter__ = MagicMock(return_value=conn)
conn.__exit__ = MagicMock(return_value=None)
ex.get_db_connection.return_value = conn
return ex, cur
def _statements(cur):
return [c[0][0] for c in cur.execute.call_args_list]
@pytest.fixture
def override():
made = {}
def _install(task_row, execution_count=0):
ex, cur = _fake_executor(task_row, execution_count)
app.dependency_overrides[get_task_executor] = lambda: ex
made['cur'] = cur
return cur
yield _install
app.dependency_overrides.pop(get_task_executor, None)
@pytest.mark.unit
class TestDeleteTask:
def test_history_blocks_deletion_with_409(self, client: TestClient, auth_headers, override):
cur = override((46,), execution_count=12)
r = client.delete("/tasks/some_task", headers=auth_headers)
assert r.status_code == 409
detail = r.json()["detail"]
# The message has to carry the facts the caller needs to act: how much
# history is at stake, the flag that proceeds, and the option they
# probably actually wanted. A bare "conflict" would be no better than
# the 500 it replaces.
assert "12 execution record" in detail
assert "purge=true" in detail
assert "enabled=false" in detail
def test_a_refused_delete_deletes_nothing(self, client: TestClient, auth_headers, override):
cur = override((46,), execution_count=12)
client.delete("/tasks/some_task", headers=auth_headers)
assert not any("DELETE" in s.upper() for s in _statements(cur))
def test_purge_removes_history_then_the_task(self, client: TestClient, auth_headers, override):
cur = override((46,), execution_count=12)
r = client.delete("/tasks/some_task?purge=true", headers=auth_headers)
assert r.status_code == 200
assert r.json()["executions_purged"] == 12
deletes = [s for s in _statements(cur) if "DELETE" in s.upper()]
assert len(deletes) == 2
# History first: the foreign key points that way, and the reverse order
# is the failure this endpoint started with.
assert "task_executions" in deletes[0]
assert "scheduled_tasks" in deletes[1]
def test_a_task_that_never_ran_deletes_without_the_flag(
self, client: TestClient, auth_headers, override
):
cur = override((46,), execution_count=0)
r = client.delete("/tasks/fresh_task", headers=auth_headers)
assert r.status_code == 200
assert r.json()["executions_purged"] == 0
deletes = [s for s in _statements(cur) if "DELETE" in s.upper()]
assert len(deletes) == 1, "nothing to purge, so history must not be touched"
assert "scheduled_tasks" in deletes[0]
def test_unknown_task_is_404_not_409(self, client: TestClient, auth_headers, override):
override(None)
r = client.delete("/tasks/nope", headers=auth_headers)
assert r.status_code == 404
def test_purge_on_an_unknown_task_is_still_404(
self, client: TestClient, auth_headers, override
):
"""The flag must not turn a missing task into a success."""
override(None)
r = client.delete("/tasks/nope?purge=true", headers=auth_headers)
assert r.status_code == 404
tests/test_task_executor.py
+236 -114
View File
@@ -4,7 +4,7 @@ Tests for the task executor module.
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
from datetime import datetime
from src.tasks.executor import TaskExecutor
from src.tasks.executor import TaskExecutor, MAX_CONCURRENT_TASKS
from src.config import Settings
@@ -17,7 +17,13 @@ class TestTaskExecutor:
executor = TaskExecutor(test_settings)
assert executor.settings == test_settings
assert executor.max_concurrent == 5
# There is no `max_concurrent` instance attribute — concurrency is
# capped by MAX_CONCURRENT_TASKS (module constant) via
# asyncio.Semaphore(MAX_CONCURRENT_TASKS) in __init__. Verify the
# semaphore was built with that bound instead of asserting an
# attribute name the class has never had.
assert MAX_CONCURRENT_TASKS == 5
assert executor.semaphore._value == 5
@patch('psycopg2.connect')
def test_get_db_connection(self, mock_connect, test_settings: Settings):
@@ -87,13 +93,25 @@ class TestTaskExecutor:
task = {**sample_task_data, 'id': 1}
with patch.object(executor, 'get_db_connection') as mock_get_conn, \
patch('src.tasks.executor.importlib.import_module') as mock_import:
# _run_executor (src/tasks/executor.py) loads the executor module with
# the __import__ builtin directly — `module = __import__(module_path,
# fromlist=['execute'])` — not importlib.import_module. This is the
# documented dynamic-loading trap in this repo's own CLAUDE.md
# ("executors are chosen by data, not code"). `importlib` is never
# imported in that module, so patching 'src.tasks.executor.importlib'
# fails at patch setup, before the test body runs at all.
real_import = __import__
def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
if name == 'src.executors.example_executor':
return mock_executor_module
return real_import(name, globals, locals, fromlist, level)
# Mock executor module
mock_executor_module = MagicMock()
mock_executor_module.execute = AsyncMock(return_value="Task completed successfully")
mock_import.return_value = mock_executor_module
with patch.object(executor, 'get_db_connection') as mock_get_conn, \
patch('builtins.__import__', side_effect=fake_import) as mock_import:
# Mock database
mock_conn = MagicMock()
@@ -106,7 +124,7 @@ class TestTaskExecutor:
await executor.execute_task(task)
# Should have imported executor module
mock_import.assert_called_with('src.executors.example_executor')
mock_import.assert_any_call('src.executors.example_executor', fromlist=['execute'])
# Should have updated task status
assert mock_cursor.execute.call_count >= 2 # Insert execution record + update task
@@ -118,13 +136,20 @@ class TestTaskExecutor:
task = {**sample_task_data, 'id': 1}
with patch.object(executor, 'get_db_connection') as mock_get_conn, \
patch('src.tasks.executor.importlib.import_module') as mock_import:
# See test_execute_task_success: _run_executor uses the __import__
# builtin directly, not importlib.import_module.
real_import = __import__
def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
if name == 'src.executors.example_executor':
return mock_executor_module
return real_import(name, globals, locals, fromlist, level)
# Mock executor that raises error
mock_executor_module = MagicMock()
mock_executor_module.execute = AsyncMock(side_effect=Exception("Task failed"))
mock_import.return_value = mock_executor_module
with patch.object(executor, 'get_db_connection') as mock_get_conn, \
patch('builtins.__import__', side_effect=fake_import):
# Mock database
mock_conn = MagicMock()
@@ -149,11 +174,16 @@ class TestTaskExecutor:
task = {**sample_task_data, 'id': 1, 'timeout_seconds': 1}
with patch.object(executor, 'get_db_connection') as mock_get_conn, \
patch('src.tasks.executor.importlib.import_module') as mock_import:
# Mock executor that takes too long
# See test_execute_task_success: _run_executor uses the __import__
# builtin directly, not importlib.import_module.
import asyncio
real_import = __import__
def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
if name == 'src.executors.example_executor':
return mock_executor_module
return real_import(name, globals, locals, fromlist, level)
mock_executor_module = MagicMock()
async def slow_execute(*args, **kwargs):
@@ -161,7 +191,9 @@ class TestTaskExecutor:
return "Done"
mock_executor_module.execute = slow_execute
mock_import.return_value = mock_executor_module
with patch.object(executor, 'get_db_connection') as mock_get_conn, \
patch('builtins.__import__', side_effect=fake_import):
# Mock database
mock_conn = MagicMock()
@@ -177,106 +209,10 @@ class TestTaskExecutor:
calls = [str(call) for call in mock_cursor.execute.call_args_list]
assert any('timeout' in str(call).lower() for call in calls)
def test_should_run_task_wildcard(self, test_settings: Settings):
"""Test task scheduling with wildcards."""
executor = TaskExecutor(test_settings)
# All wildcards should always match
task = {
'minute': -1,
'hour': -1,
'day_of_month': -1,
'month': -1,
'day_of_week': -1
}
now = datetime(2025, 12, 7, 14, 30, 0) # Saturday
assert executor._should_run_now(task, now) is True
def test_should_run_task_specific_time(self, test_settings: Settings):
"""Test task scheduling with specific time."""
executor = TaskExecutor(test_settings)
# Specific time: every day at 14:30
task = {
'minute': 30,
'hour': 14,
'day_of_month': -1,
'month': -1,
'day_of_week': -1
}
# Matching time
now = datetime(2025, 12, 7, 14, 30, 0)
assert executor._should_run_now(task, now) is True
# Non-matching time
now = datetime(2025, 12, 7, 14, 31, 0)
assert executor._should_run_now(task, now) is False
def test_should_run_task_specific_day_of_month(self, test_settings: Settings):
"""Test task scheduling with specific day of month."""
executor = TaskExecutor(test_settings)
# Run on 11th of every month at 04:00
task = {
'minute': 0,
'hour': 4,
'day_of_month': 11,
'month': -1,
'day_of_week': -1
}
# Matching date
now = datetime(2025, 12, 11, 4, 0, 0)
assert executor._should_run_now(task, now) is True
# Wrong day
now = datetime(2025, 12, 12, 4, 0, 0)
assert executor._should_run_now(task, now) is False
def test_should_run_task_specific_month(self, test_settings: Settings):
"""Test task scheduling with specific month."""
executor = TaskExecutor(test_settings)
# Run on January 1st at midnight
task = {
'minute': 0,
'hour': 0,
'day_of_month': 1,
'month': 1,
'day_of_week': -1
}
# Matching date
now = datetime(2025, 1, 1, 0, 0, 0)
assert executor._should_run_now(task, now) is True
# Wrong month
now = datetime(2025, 2, 1, 0, 0, 0)
assert executor._should_run_now(task, now) is False
def test_should_run_task_day_of_week(self, test_settings: Settings):
"""Test task scheduling with day of week."""
executor = TaskExecutor(test_settings)
# Run every Monday at 09:00
task = {
'minute': 0,
'hour': 9,
'day_of_month': -1,
'month': -1,
'day_of_week': 0 # Monday
}
# Monday
now = datetime(2025, 12, 8, 9, 0, 0) # Monday
assert executor._should_run_now(task, now) is True
# Tuesday
now = datetime(2025, 12, 9, 9, 0, 0) # Tuesday
assert executor._should_run_now(task, now) is False
@pytest.mark.asyncio
async def test_concurrent_task_limit(self, test_settings: Settings, sample_task_data: dict):
@@ -303,5 +239,191 @@ class TestTaskExecutor:
await executor.process_minute()
# Should only execute max_concurrent (5) tasks
assert mock_execute.call_count <= executor.max_concurrent
# process_minute awaits every task in this batch (asyncio.gather)
# before returning, so by the time we're back here all 10 have
# run to completion — the semaphore bounds how many can be
# in flight *concurrently*, not the eventual call_count, which
# this assertion conflated. Kept as a correctness check on the
# total (all scheduled tasks still get executed) since a
# concurrency-in-flight assertion needs a task that can be
# observed mid-execution, which mock_execute (an AsyncMock with
# no delay) does not provide.
assert mock_execute.call_count == len(tasks)
@pytest.mark.unit
class TestOrphanReconciliation:
"""T-2. A 'running' row excludes its task from scheduling forever.
get_tasks_for_minute filters out any task holding one, and nothing ever
closed those rows, so a process that died between writing the row and
updating it left its task permanently unschedulable — silently. A row from
2025-12-07 sat that way for eight months. Watchtower restarts this container
nightly, so the exposure was daily.
"""
def _mock_conn(self, executor, rows):
conn, cur = MagicMock(), MagicMock()
cur.fetchall.return_value = rows
conn.cursor.return_value.__enter__ = MagicMock(return_value=cur)
conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
conn.__enter__ = MagicMock(return_value=conn)
conn.__exit__ = MagicMock(return_value=None)
patcher = patch.object(executor, 'get_db_connection', return_value=conn)
patcher.start()
return cur, patcher
def test_running_rows_are_released(self, test_settings: Settings):
executor = TaskExecutor(test_settings)
cur, p = self._mock_conn(executor, [("backup_docker_configs_daily", datetime(2026, 8, 11))])
try:
assert executor.reconcile_orphaned_executions() == 1
sql = cur.execute.call_args[0][0]
assert "UPDATE task_executions" in sql
# It must target exactly the rows get_tasks_for_minute excludes, and
# move them to a status it does not exclude. If these two ever drift
# apart the bug returns in silence.
assert "WHERE status = 'running'" in sql
assert "status = 'orphaned'" in sql
finally:
p.stop()
def test_clean_startup_reports_nothing(self, test_settings: Settings):
executor = TaskExecutor(test_settings)
cur, p = self._mock_conn(executor, [])
try:
assert executor.reconcile_orphaned_executions() == 0
finally:
p.stop()
def test_a_database_failure_does_not_stop_startup(self, test_settings: Settings):
"""Refusing to boot because cleanup failed is worse than a stale row."""
executor = TaskExecutor(test_settings)
with patch.object(executor, 'get_db_connection', side_effect=Exception("db down")):
assert executor.reconcile_orphaned_executions() == 0 # no raise
def test_orphaned_is_not_failed(self, test_settings: Settings):
"""The outcome is unknown, not known-bad.
A backup that finished and never got to update its row looks identical to
one that died halfway. Recording 'failed' asserts something nobody
observed.
"""
executor = TaskExecutor(test_settings)
cur, p = self._mock_conn(executor, [("t", datetime(2026, 1, 1))])
try:
executor.reconcile_orphaned_executions()
sql = cur.execute.call_args[0][0]
assert "'failed'" not in sql
finally:
p.stop()
@pytest.mark.asyncio
async def test_startup_reconciles_before_the_scheduler_starts(
self, test_settings: Settings, monkeypatch
):
"""Order matters: reconcile must finish before the first minute is processed.
Run the other way round and the first tick still sees the stale rows.
"""
from src import main
order = []
monkeypatch.setattr(main, 'get_settings', lambda: test_settings)
monkeypatch.setattr(
TaskExecutor, 'reconcile_orphaned_executions',
lambda self: (order.append('reconcile'), 0)[1],
)
class FakeScheduler:
def add_job(self, **kw): order.append('add_job')
def start(self): order.append('start')
def shutdown(self, wait=True): order.append('shutdown')
monkeypatch.setattr(main, 'AsyncIOScheduler', lambda **kw: FakeScheduler())
async with main.lifespan(None):
pass
assert 'reconcile' in order, "startup never reconciled orphaned executions"
assert order.index('reconcile') < order.index('start')
@pytest.mark.unit
class TestTimeoutIsDistinguishable:
"""T-3. The 'timeout' status existed in the code and had never been written.
_run_executor's `except Exception` sat above execute_task's
`except asyncio.TimeoutError`, and since 3.11 asyncio.TimeoutError IS the
builtin TimeoutError (OSError -> Exception), so the broad handler always won.
Eight months, 18,785 executions, zero timeout rows — every one filed as a
generic failure, erasing the difference between "too slow for its window" and
"broken".
"""
@pytest.mark.asyncio
async def test_timeout_propagates_instead_of_becoming_an_error_tuple(
self, test_settings: Settings, monkeypatch
):
import sys, types, asyncio as aio
mod = types.ModuleType("src.executors.slow_probe")
async def execute(config, settings):
await aio.sleep(5)
mod.execute = execute
monkeypatch.setitem(sys.modules, "src.executors.slow_probe", mod)
executor = TaskExecutor(test_settings)
with pytest.raises(aio.TimeoutError):
await executor._run_executor("slow_probe", {"config": {}}, timeout=0.05)
@pytest.mark.asyncio
async def test_a_timed_out_task_is_recorded_as_timeout(self, test_settings: Settings):
import asyncio as aio
executor = TaskExecutor(test_settings)
task = {'id': 7, 'task_name': 'slow', 'executor': 'slow_probe',
'service': 'scheduler', 'priority': 5, 'timeout_seconds': 1}
conn, cur = MagicMock(), MagicMock()
cur.fetchone.return_value = [123]
conn.cursor.return_value.__enter__ = MagicMock(return_value=cur)
conn.cursor.return_value.__exit__ = MagicMock(return_value=None)
conn.__enter__ = MagicMock(return_value=conn)
conn.__exit__ = MagicMock(return_value=None)
with patch.object(executor, 'get_db_connection', return_value=conn), \
patch.object(executor, '_run_executor',
new=AsyncMock(side_effect=aio.TimeoutError())), \
patch.object(executor, '_update_execution_status') as upd_exec, \
patch.object(executor, '_update_task_outcome') as upd_task:
await executor.execute_task(task)
assert upd_exec.call_args[0][1] == 'timeout', "execution row must say timeout"
# The trap: while the timeout branch was unreachable a timeout travelled
# the normal path, which DOES update scheduled_tasks. Making the branch
# reachable without this call would swap a wrong status for a stale one.
assert upd_task.called, "scheduled_tasks left stale after a timeout"
assert upd_task.call_args[0][1] == 'timeout'
@pytest.mark.asyncio
async def test_ordinary_errors_are_still_returned_not_raised(
self, test_settings: Settings, monkeypatch
):
"""The narrow clause must not swallow anything else on its way past."""
import sys, types
mod = types.ModuleType("src.executors.boom_probe")
async def execute(config, settings):
raise ValueError("kaboom")
mod.execute = execute
monkeypatch.setitem(sys.modules, "src.executors.boom_probe", mod)
executor = TaskExecutor(test_settings)
output, error = await executor._run_executor("boom_probe", {"config": {}}, timeout=5)
assert output is None
assert "kaboom" in error