sync: port module changes from constructive-db - #129
Conversation
database-jobs: run_scheduled_job no longer treats an exhausted keyed job as active coverage, so a schedule's next tick re-runs it instead of raising ALREADY_SCHEDULED. function-resolution: route bindings carry an anonymous flag; resolve_capabilities echoes required_capabilities. metaschema-modules: refusal_log_module table + one-platform-scope constraint; agent_module settle_run_cost_function_name; inference_log_module price table; principal_auth_module private schema + sweep function; user_auth_module changes. metaschema-schema: index table and is_valid_step_up updates. utils: default_self_reference. Tests ported from constructive-db (pgsql-test imports) and bundles regenerated with pgpm package.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. 🟠 1 high 💬 Inline comments (1)
🧹 Nitpicks (1) — 🟢 1 low
The PR spans four packages.
Reviewed commit: 0fb9407 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds metaschema module tables and a step-up guard validator, reworks scheduled-job concurrency, and updates function-resolution route binding and capability resolution, along with regenerated SQL bundles and tests.
Key findings
- 🟠 Non-keyed schedule duplicates in-flight last-attempt job — run_scheduled_job.sql:42
| -- covers nothing: fail_job cleared its locked_at, which otherwise reads | ||
| -- as "never been run" and wedges the schedule on a permanently failed | ||
| -- job. The keyed upsert below replaces it with a fresh attempt instead. | ||
| AND js.attempts < js.max_attempts |
There was a problem hiding this comment.
🟠 bug · high
Non-keyed schedule duplicates in-flight last-attempt job
The new probe condition AND js.attempts < js.max_attempts (packages/database-jobs/deploy/schemas/app_jobs/procedures/run_scheduled_job.sql:42) also excludes a job that is currently locked and running on its final attempt (attempts == max_attempts, locked_at set), even though such a job still covers the tick. For a schedule with sched.key IS NULL the keyed in-flight guard at lines 60-71 is skipped, so the function inserts a brand-new job while the previous tick is still executing, producing two concurrent executions of the same schedule. A non-keyed schedule with max_attempts = 1 whose worker is still processing when the next tick fires now enqueues a duplicate instead of raising ALREADY_SCHEDULED.
📋 Prompt for AI Agents
In packages/database-jobs/deploy/schemas/app_jobs/procedures/run_scheduled_job.sql around line 42, change the already-scheduled probe so that a locked (in-flight) job always raises ALREADY_SCHEDULED regardless of attempt budget. Replace AND js.attempts < js.max_attempts with AND (js.attempts < js.max_attempts OR js.locked_at IS NOT NULL). Rationale: the fix intended to let a permanently-dead keyed job (locked_at NULL, attempts >= max_attempts) be refreshed, but it also excludes a job currently running on its final attempt (locked_at NOT NULL, attempts == max_attempts), which still covers the tick. For non-keyed schedules the keyed in-flight guard at lines 60-71 is skipped, so this regression enqueues a duplicate concurrent job instead of raising ALREADY_SCHEDULED.
…t it is a NOT NULL FK
1 participant
Summary
Brings every module that exists here back in line with its vendored copy in
constructive-db/pgpm-modules/, which had drifted ahead. Only modules already present upstream are synced (db-utils/infra-utilsexist only in constructive-db and are left out). No version bumps;sql/bundles regenerated withpgpm package.SQL source changes:
run_scheduled_job: the coverage lookup now addsAND js.attempts < js.max_attempts, so an exhausted keyed job (e.g.max_attempts = 1after a missing-function failure) no longer counts as "still scheduled" and the next tick resets it instead of raisingALREADY_SCHEDULEDforever (constructive-db#3723 / constructive-planning#2013).install_route_bindingshonours ananonymousflag on binding entries;resolve_capabilitiesechoesrequired_capabilities(NULL = undeclared,[]= declared none).refusal_log_moduletable +one_platform_scopeconstraint (2 plan entries);agent_module.settle_run_cost_function_name;inference_log_module.inference_price_table_*;principal_auth_module.private_schema_id/sweep_expired_principals_function;user_auth_moduleupdates.indextable,is_valid_step_up.default_self_reference(1 plan entry).Tests were ported with
constructive-test→pgsql-testimports. Two places where the vendored tests were stale relative to upstream were kept at upstream's version: thecatalog_moduleseed inserts (must includeapp_store_identities_table_id) and theenqueue()actor claims inresolution.test.ts. Themetaschema-modulesFK snapshot was regenerated (+1 constraint from the price-table FK).Verified locally:
pgpm test-packages --full-cycle— all modules pass; package tests for database-jobs, jobs, function-resolution, metaschema-modules, metaschema-schema, utils, object-store pass;pnpm run policy:checkclean.Link to Devin session: https://app.devin.ai/sessions/09f9e1a7ad7c4e6480b0e489ab2bee95
Open in Devin Desktop: https://app.devin.ai/desktop/session/09f9e1a7ad7c4e6480b0e489ab2bee95?variant=devin
Requested by: @pyramation