Aggregates the existing execution_logs table (no hot-path instrumentation):
ExecutionLogRepository::summarize_for_app returns counts, error rate, latency
avg/p50/p95 (percentile_cont), by-status/by-source breakdowns, and an hourly
series over a trailing window (clamped 1h..90d). New metrics_api router at
GET /api/v1/admin/apps/{id}/metrics?window= (AppLogRead), and a dashboard
Metrics tab (summary cards + an inline-SVG per-hour bar chart, no JS dep).
Pinned by a manager-core test asserting the exact rollup (percentiles included)
and the empty-app zero case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exports a group as a [group] manifest: its scripts, [[routes]]/[[triggers.*]]
TEMPLATES (with the group-only sealed/shared flags), a shared collections
entry, [vars]/[secrets], extension_points, and [suppress]. Groups own no
workflows.
The group trigger/route reports were display-only (no ops, dispatch, retry,
host_kind, cron tz, queue timeout) so a pulled manifest could not re-plan
clean. Enriched TriggerTemplateInfo with the full internally-tagged
TriggerDetails JSON + dispatch_mode + retry_max_attempts, and RouteTemplateInfo
with raw manifest-shaped fields (method/host_kind/host/host_param_name/
path_kind/dispatch_mode); pic routes ls --group now applies the ANY/host munge
client-side, consistent with the app pic routes ls. pull.rs factors the
script-writing + trigger-decoding it shares with the app path.
Pinned by a pull journey: a group with a sealed+shared kv trigger, a sealed
route, a shared collection, and a var round-trips and the exported manifest
re-applies clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fills the one per-app admin-read gap kv/files already covered. New
docs_api.rs mirrors kv_api (GET /apps/{id}/docs[/{collection}/{doc_id}],
AppDocsRead capability, DocsRepo::list/get) with the group_blobs_api docs
response shape so the CLI shares one deserialize. DocsCmd gains optional
--app/--group (mutually exclusive, like KvCmd) dispatched via
require_one_owner; new client docs_list/docs_get. Read-only by design,
matching kv/files. Pinned by a collections journey: a script writes the
app's own docs collection, the operator lists + fetches it with no script.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- roles: member invoke asserts the script's actual stdout, not just exit 0;
- api: app-slug script filter asserts the returned script's name;
- workflow_orchestrator: unknown-workflow asserts WorkflowError::NotFound,
not just is_err().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An outbox row whose trigger_depth exceeds max_trigger_depth is dropped
without executing the handler. Mirrors the disabled-http drop harness;
mutation-verified (weakening the depth gate fails it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirrors group_kv_caps_resolve_by_role_up_the_chain for the two other
group write capabilities: a Viewer is denied GroupPubsubPublish and
GroupQueueEnqueue, an Editor is allowed, and an outsider is denied.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extends the atomic_write suite with three write-path gaps:
- an app cannot exceed PICLOUD_APP_DOCS_MAX_ROWS (create-only, update
net-zero, delete frees a slot) via a real PostgresDocsWriter;
- concurrent writers cannot push a group past its docs total-bytes quota
(16 racers against a 450-byte ceiling under pg_advisory_xact_lock);
- a files create with a broken outbox leaves no row AND no blob on disk,
while a files delete with a broken outbox keeps both the row and the
readable blob (the disk-ordering invariant).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three related fixes from the test audit.
**The CLI journey fixture gets its own database.** It spawned a real picloud —
whose dispatcher/orchestrator claim loops are global by design (one instance owns
one database) — against the shared dev DB, so it could claim the manager-core
suites' outbox/workflow rows (the same class of bug already fixed for the e2e
suites, one binary over). It now clones one dedicated database per journey run
from the migrated template. test-support gains `named_test_db_url` (explicit
stable name) + a blocking wrapper for the sync `LazyLock` fixture. The one journey
that talks to Postgres directly (dead-letter injection) now uses the fixture's DB
URL, not the base DATABASE_URL, so it hits the database the server reads.
**workflow_orchestrator moves to per-test databases.** Its `claim_ready_step` is
global, so the old harness serialized every test behind a process-wide CLAIM_LOCK
AND ran `DELETE FROM workflow_runs` (unscoped — it wiped every app's runs) before
each one. A private database per test makes the global claim see only that test's
rows, so both the lock and the unscoped DELETE are deleted.
**DB-backed suites fail loud instead of skipping green.** ~15 manager-core suites
`return None` when DATABASE_URL is unset and report PASS — so in any environment
that lost its database the entire integration surface reports green while running
nothing (why the CI gap went unnoticed for so long). New
`picloud_test_support::abort_if_db_required` panics when `PICLOUD_REQUIRE_DB` is
set (CI now sets it) but DATABASE_URL is not, injected into each suite's skip
path. Local runs without the var still skip cleanly.
Mutation-verified: with PICLOUD_REQUIRE_DB=1 and DATABASE_URL unset, a suite
panics; without the var it skips.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`PICLOUD_SESSION_ABSOLUTE_TTL_HOURS` (audit C1, migration 0070) is the whole
mitigation for a stolen-but-warm admin token, and nothing tested it — grepping
`absolute_expires_at` across all tests returned zero hits.
New DB-backed suite pins the authoritative enforcement: the lookup filter
`absolute_expires_at > NOW()`. A session with a future sliding `expires_at` but a
past absolute cap must NOT resolve (the stolen-but-warm case), and `touch` cannot
resurrect it even when it pushes `expires_at` a year out — the filter is the
backstop regardless of the middleware clamp.
Mutation-verified: dropping the `absolute_expires_at > NOW()` predicate makes both
tests fail. Runs on a private database via picloud-test-support.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CLAUDE.md calls the KV/docs/pubsub/queue value-size caps an anti-DoS rail:
oversized payloads are rejected "before authz so anonymous public scripts can't
DoS Postgres." Only queue had a test, and it used an allow-all authz + anon cx —
which catches a DROPPED cap but not a REORDERED one, because an anon cx passes
script_gate regardless.
Each service now has an ordering-proof test: a DENYING authz repo + an
AUTHENTICATED member cx, so a size-check-first service returns *TooLarge while an
authz-first one would return Forbidden. Each pairs it with an under-cap control
through the same denied cx (returns Forbidden) to prove the cx really is denied,
so the *TooLarge case genuinely bypassed authz. Queue's pre-existing test is
upgraded to the same shape (+ a shared member_cx helper).
Mutation-verified: moving the KV size check after authz flips its result to
Forbidden and the test fails.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`GroupQueueServiceImpl` shipped with ZERO tests; `GroupPubsubServiceImpl` had four
that exercised neither boundary — its resolver always resolved and its principal
was always an instance Owner (who bypasses the capability check). So on the two
newest shared services, both the isolation boundary and the anonymous fail-closed
gate were unpinned — and they matter most here: a shared-queue consumer is
materialized into EVERY descendant app, so an unauthorized enqueue is
attacker-controlled input fanned out as script execution across a whole subtree.
Both now get the pair their three older siblings have:
`unrelated_app_gets_collection_not_shared` (with a positive control, so a
reject-everything impl can't pass it) and an anon/non-editor fail-closed test that
also confirms reads (depth) stay open.
Mutation-verified: swapping `script_gate_require_principal` for `script_gate` —
the one-token edit that would let an unauthenticated public route enqueue into any
ancestor group's shared queue — makes the anonymous enqueue succeed, and the test
catches it exactly there.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`resolve_owning_group`'s ancestor-chain CTE is the isolation boundary for EVERY
group shared collection — kv, docs, files, topics, queues. Its own doc says so.
Yet no test exercised the actual SQL: the tests that claimed to
(`unrelated_app_gets_collection_not_shared` in the three older group services,
`realtime_authority`'s SSE 404) inject a `FakeResolver` HashMap the test itself
populates — the foreign app resolves to `None` only because the test never
inserted it. They prove the service propagates a `None`; they cannot see the
query that produces it. Dropping `JOIN chain c ON gc.group_id = c.group_owner`
would let ANY app resolve ANY group's shared collection — a cross-tenant breach —
and the whole unit suite would stay green.
This drives the real query over a real tree: a descendant resolves, a
sibling-subtree app gets nothing, the kind filter is part of the boundary (a kv
and a docs collection of one name are distinct stores), nearest-declaration
shadows (security-relevant — a deep app must reach the nearer group's store), an
app-owned marker shares with nobody, and lookup is case-insensitive.
Mutation-verified: with the chain join dropped, the boundary assertion fires — and
under that breach `app_mid` resolves a FOREIGN group's `catalog` left in the DB by
another suite, i.e. the leak reproduces live.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Turning CI on surfaced `atomic_write` as intermittently failing under load —
self-inflicted. Its fault injection is `CREATE TRIGGER ... ON outbox`, which takes
an ACCESS EXCLUSIVE lock on a table every other suite is concurrently inserting
into. The header even claimed it "cannot affect any test running in parallel";
that was wrong — scoping the trigger by app_id bounds which ROWS it rejects, not
the table lock installing it takes. Shipping that alongside "run the DB tests in
CI" would have poisoned the signal.
The fix is the one already used for the e2e suites: a private database per test.
That logic (build a migrated template once, clone it per test via
`CREATE DATABASE ... TEMPLATE`) was duplicated in `picloud/tests/common`, and this
would have been a third copy — so it moves into a shared `picloud-test-support`
dev-dependency crate, and `picloud/tests/common` now re-exports it. The invariant
it encodes: manager-core's claim loops are global by design, so a test must not
share a database with anything that runs them OR with anything that does DDL.
Moved onto it:
- `atomic_write` — the DDL fault injection above; its per-test cleanup is now
deleted (the database is thrown away).
- `outbox_reclaim` — asserts `reclaim_stale_claims` returns exactly 0, a count
over the WHOLE outbox table. On the shared DB any stale row from any other suite
(or a killed picloud that died holding a claim) would fail it permanently, until
someone cleared the table by hand.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`with_events()` and `with_atomic_writes()` both install a writer, last call wins.
Appending them in the wrong order in the host would swap the transactional writer
back out for the best-effort one — silently reverting the write-path invariant AND
dropping the per-group quota enforcement that lives in it, with nothing in the type
system to catch it.
The services now carry an `atomic` flag: once `with_atomic_writes` has run,
`with_events` warns and no-ops instead of clobbering it. Also enforce the group
byte ceiling in `BestEffortGroupFilesWriter::update`, so the non-transactional
writer can't be used as an unmetered bypass and a unit test can catch its removal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stale-claim reclaimer added earlier in this branch un-claimed every timed-out
row, including SYNCHRONOUS ones. `reply_to.is_some()` is the schema's "never
retry" signal (0009_outbox.sql) and the dispatcher honours it: those rows are an
HTTP request someone is blocked on, and re-running one means re-running its side
effects — a charge, an email, an external POST — for a caller that is long gone.
So a dispatcher dying mid-sync-dispatch would have its row resurrected 10 minutes
later and executed a second time, for nobody. Before the reclaimer those rows were
merely stranded; the reclaimer turned inert into harmful.
The reclaim now gates on `reply_to IS NULL`. Stale sync rows are DELETED instead:
they can never be dispatched again and nobody is waiting on them, so leaving them
would just accumulate garbage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-review of this branch caught a data-loss bug I introduced with the
per-app / group files ceilings: the quota check ran AFTER the blob write.
`write_atomic_at` renames the new bytes over the FINAL path, so by the time
the ceiling refused an update the previous bytes were already gone. The
per-app path then unlinked the blob (row survives, file destroyed — every
read 404s); the group path left the new bytes in place under the old row's
checksum (every read fails `Corrupted`). Either way a user permanently lost
a file merely by exceeding a quota — a strictly worse outcome than the
unchecked-update bypass the ceiling was added to close.
The check now precedes the write on create AND update, per-app and group. As
a bonus this stops an over-quota caller driving unbounded write+unlink disk
churn: the ceiling now bounds I/O, not just stored bytes. The blob still goes
down inside the transaction, under the advisory lock, so a rollback unlinks
it and nothing can reference it in between.
The original test passed against the bug: it asserted the refused update did
not change the stored byte TOTAL — true, while the blob was already
destroyed. The regression test now reads the file back through the
checksum-verifying `FsFilesRepo::get`, and was confirmed to fail with
`Corrupted` against the old ordering.
Also in the KV writer (same file): drop the redundant pre-read on the hottest
write path. `set`/`set_if` did a SELECT purely to learn whether the write
would add a row, when the upsert already returns the previous value. Check
after the insert instead (`>` not `>=`) and let the transaction roll back on
refusal — identical outcome, one round-trip fewer, and an update pays nothing
at all. `kv_repo::get_on` and `FsFilesRepo::final_path` are now unused and
deleted; `FilesRepo::delete` delegates to the `delete_meta_on` + `unlink_blob`
helpers rather than re-implementing them.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`users_service` still write-then-emits at nine call sites. That is currently
INERT — `triggers.kind` has no `users` value, so the outbox emitter has no arm
for the source and drops the event — but it is the exact shape audit #6
removed from every other stateful service, and it would be reintroduced by
whoever adds a users trigger kind and finds the plumbing already there.
Note that this one does not even log the failure (`let _ = ...`).
Documented at the emit site, pointing at `atomic_write` and the write-path
invariant in CLAUDE.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chasing the e2e flakiness turned up a production durability bug, not a test
bug.
The dispatcher claims an outbox row, executes it, then either deletes it
(success) or reschedules it (failure) — both of which clear the claim. If
the PROCESS DIES in between, neither runs. And `claim_due` only ever selects
`claimed_at IS NULL`. Nothing else in the codebase clears `outbox.claimed_at`
— grep it: there are exactly three writers, and those are two of them.
So a crash or restart mid-dispatch stranded every in-flight row PERMANENTLY.
Its trigger never fired and no retry could notice. The outbox is the
universal trigger path, so the loss covered kv / docs / files / cron /
pubsub / email / invoke_async / dead-letter alike. This is the same
durability class as the audit's #6 (lost trigger event), which was just
fixed at the WRITE end — this is the same hole at the READ end.
Every other claim-based store already had the safety net: `queue_messages`
and `group_queue_messages` have `reclaim_visibility_timeouts`,
`workflow_steps` has its own reclaim. The outbox was the one that didn't.
`OutboxRepo::reclaim_stale_claims(timeout)` returns rows whose claim is older
than `PICLOUD_OUTBOX_CLAIM_TIMEOUT_SEC` (default 600s), run from the
dispatcher's existing reclaim ticker. The default is deliberately generous —
a script may run for up to 300s (the `scripts.timeout_seconds` CHECK), so a
claim held past twice that is abandoned rather than slow; reclaiming a row a
LIVE dispatcher is still working on would double-execute it (survivable —
dispatch is at-least-once — but not worth courting).
A reclaim does NOT bump `attempt_count`: the handler never ran, so it must
not consume the row's retry budget, or repeated restarts would dead-letter an
event that executed zero times. (Same reasoning as the transient queue
`release` fixed earlier in this branch.)
This is also the root cause of the flaky e2e suites: each test drops its
dispatcher, and `claim_due` is not scoped per app or per dispatcher, so a
test's dispatcher could claim another test's row and strand it on teardown.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An app's OWN store had no ceiling of any kind — only the group-SHARED
collections did, which is the inversion of what you'd expect. And
`authz::script_gate` returns `Ok(())` when there is no principal, so a
PUBLIC UNAUTHENTICATED route could reach `files::collection(..).create(..)`
and write blobs in a loop until the disk filled. `PICLOUD_FILES_MAX_FILE_SIZE_BYTES`
caps ONE file at 100 MB; nothing capped the count. Same for KV keys and docs
against Postgres.
Ceilings: `PICLOUD_APP_KV_MAX_ROWS` / `PICLOUD_APP_DOCS_MAX_ROWS` (100k) and
`PICLOUD_APP_FILES_MAX_TOTAL_BYTES` (10 GiB).
They are enforced DIFFERENTLY from the group ones, on purpose — porting the
group design as-is would have been a serious regression:
* KV/docs check a ROW count, on INSERT only, with NO lock. `kv::set` is the
hottest write path in the system; taking a per-app advisory lock on every
set would serialize an app's entire data plane, and a `SUM(...)` scan would
make write cost grow with the app's stored size. What the lock buys is
small — unlocked overshoot is bounded by write concurrency (32) against a
ceiling of 100k, ~0.03%. These are anti-DoS rails, not billing. An update
adds no row and is bounded by the per-value cap, so it pays nothing at all.
* No per-app BYTE ceiling for KV/docs: every value is already capped at
`PICLOUD_KV_MAX_VALUE_BYTES`, so `max_rows x max_value_bytes` is ALREADY a
finite bound. A second scan would buy nothing.
* FILES are the exception and DO lock + sum: one blob may be 100 MB, so 32
racing uploads could overshoot by gigabytes of real disk, and a file write
is heavy enough that the lock and the SUM are lost in the noise. Checked on
create AND update — an update that skipped the ceiling would be a free
bypass (the same bug just fixed for group files: grow a 1-byte file to
100 MB, repeat).
`group_quota` is renamed `quota`: it now serves both owners, and the module
doc is where the two enforcement strategies are contrasted.
Pinned by tests/atomic_write.rs: the key ceiling refuses a new key while
still allowing an update at the ceiling (rejecting updates would brick an app
the moment it filled up — worse than the DoS the rail exists to stop), and 10
concurrent uploads cannot exceed the disk ceiling, nor can an update grow past it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit #6 and #8 for the last of the three stores, plus a bypass found on
the way.
**#6.** create/update/delete wrote the metadata row, then emitted
best-effort, so an outbox failure left a committed file whose trigger never
fired. `atomic_write::FilesWriter` commits the metadata row and the fan-out
together.
Files are the one store where the ordering is subtle, because the BYTES live
on disk and cannot join a transaction:
* create/update — blob first, then commit metadata + fan-out. A rollback
unlinks the blob. (A crash at that exact point still orphans it; that
hazard predates this change — the repo already wrote the blob and then
inserted the row in a separate, failable statement — and the orphan is
inert, referenced by nothing.)
* delete — commit the metadata removal + fan-out FIRST, then unlink. The
reverse order would destroy the bytes of a row that a rollback keeps,
leaving a file that can never be read.
**#8.** `GroupFilesService::create` read `total_bytes` on one connection and
wrote on another, so concurrent uploads each saw the same pre-write total and
together overshot the ceiling. This is the worst instance of the race in the
codebase: the ceiling is DISK (10 GiB by default) and one file may be 100 MB,
so a racing fleet overshoots by gigabytes. `PostgresGroupFilesWriter` takes
the per-group advisory lock (on its own `files` key) across the check and the
write.
**The bypass.** `GroupFilesService::update` checked NO quota at all — so a
1-byte file could be updated to a 100 MB one without the ceiling ever being
consulted, repeatedly, for unbounded disk. It now checks the projected total
(the replaced file's bytes subtracted in SQL, so a same-size-or-smaller
update near the cap still goes through).
Also drive-by: `queue_e2e` asserted the ack the instant the marker appeared,
but the marker is written DURING the handler and the ack happens after it
returns — a zero-tolerance race. It polls now. (This does not fix the
suite's flakiness, which reproduces on the pre-pass commit too.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit #6 and #8, applied to the docs store — the same two bugs KV had, in
the same two places.
Per-app docs (#6): create/update/delete wrote the row, then emitted
best-effort. An outbox failure left a committed doc whose trigger never
fired. They now go through `atomic_write::DocsWriter`, whose Postgres impl
writes and fans out on one connection in one transaction.
Group docs (#8): the service read `count_rows`/`projected_total_bytes` on
pooled connections and wrote on another, so concurrent creators each saw the
same pre-write count and together overshot the ceiling.
`PostgresGroupDocsWriter` takes a per-group advisory lock — on its OWN
`docs`-namespaced key, so it serializes against other docs writers to that
group but not against KV writers, which draw on a separate ceiling.
The bespoke `check_total_bytes` (with its own copy of the upper-bound fast
path) is gone; group docs now shares `group_quota::check_group_write` with
group KV, so the row ceiling, the projected-bytes ceiling, and the fast path
that skips the O(n) SUM scan have exactly one implementation between them.
tests/atomic_write.rs gains the docs row-quota race (16 concurrent creators
against a ceiling of 4).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit #8. `GroupKvServiceImpl` read the group's usage (`count_rows` /
`projected_total_bytes`) on one pooled connection and wrote on another. N
concurrent writers therefore each observed the same pre-write total, each
concluded it had room, and together sailed past the ceiling. The code
called this out as "best-effort ... quotas are safety rails, not exact
accounting" — but the overshoot is not small. With a ceiling of 5 and 20
concurrent writers, 19 rows land.
Route group-KV mutations through `atomic_write::GroupKvWriter`, whose
Postgres impl runs the quota reads, the row write, and the shared-trigger
fan-out on ONE connection in ONE transaction — and, crucially, takes a
per-group `pg_advisory_xact_lock` first.
The lock is the fix, not the transaction. Putting the check inside the
writing tx is necessary but NOT sufficient: under READ COMMITTED each
transaction's `COUNT(*)`/`SUM(...)` still sees a snapshot without the other
writers' uncommitted rows, so they all still pass. Serializing the
check-then-write per group is what makes a writer see its predecessor's row.
The lock key is namespaced per kind (kv/docs/…) so writes drawing on
different ceilings don't contend. A delete takes no lock — it only frees
space.
The quota POLICY (row ceiling, projected-bytes ceiling, and the
upper-bound fast path that skips the O(n) SUM scan for a group nowhere near
its cap) moves to one place, `group_quota::check_group_write`, over a small
`GroupUsage` trait. Both backends — the transactional one reading through
`&mut *tx`, and the in-memory one the unit tests use — share it, so there is
exactly one copy of the decision.
`set_if` gains a correctness nicety on the way: the row ceiling now keys on
whether the write ADDS a row, so a compare-and-swap against an absent key
with a `Some` precondition (which cannot swap, and so consumes nothing) is
no longer charged for one.
tests/atomic_write.rs pins both ceilings against 20/16 concurrent writers.
Both tests fail without the lock (19 rows stored against a ceiling of 5).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audit #6. `KvServiceImpl` wrote the row, waited for it to commit, then
asked the emitter to resolve matching triggers and insert outbox rows —
a second transaction on a second connection. If that second step failed,
the row was permanently in the store with its trigger having never
fired: invisible to the caller (the write "succeeded"), unrecoverable by
any retry, and only ever logged. The code said as much:
// Audit finding (Medium): this is non-transactional with the data
// write — the row above has already committed by the time emit()
// runs, so an emit failure means triggers silently don't fire.
Introduce `atomic_write::KvWriter` — the mutating half of the service (the
write AND the fan-out it produces), so the two can share a transaction:
* `PostgresKvWriter` opens a tx, writes via `kv_repo::*_on(&mut *tx, …)`,
runs the fan-out on the SAME connection via `emit_on(&mut *tx, …)`, and
commits. An emit failure now rolls the write back and surfaces to the
script, which is the honest outcome — the caller learns the write did
not happen instead of silently getting a store that disagrees with its
triggers.
* `BestEffortKvWriter` keeps the old write-then-log-on-emit-failure
semantics for the in-memory unit tests (no Postgres).
Both sit behind one trait, so the service body has a single code path and
the choice is a constructor detail (`with_atomic_writes(pool)` in the host).
Pool-deadlock rule, documented on the module: everything inside the tx runs
on the tx's connection. A writer that held a tx and then reached for a
second pooled connection could starve — the pool is sized to the execution
concurrency cap, so N executions each wanting 2 connections deadlock. The
fan-out takes `&mut *tx` and never touches a repo.
`tests/atomic_write.rs` pins it by injecting an outbox failure (a Postgres
BEFORE-INSERT trigger scoped to the test's own app_id, so parallel tests are
unaffected): the set errors and the key is NOT in the store; likewise a
delete rolls back rather than dropping a key nothing downstream hears about.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`OutboxEventEmitter` resolved matching triggers and inserted outbox rows
through `Arc<dyn TriggerRepo>` / `Arc<dyn OutboxRepo>`, i.e. against the
pool — each query on whatever connection it happened to get. That makes a
transactional outbox impossible: the data write and the outbox rows can
never share a transaction, so a crash (or an outbox error) between them
silently loses the trigger event.
Take the emitter down to a connection instead of a pool:
* `outbox_repo::insert_on(exec, row)` and `trigger_repo::list_matching_on`
/ `list_matching_shared_on(exec, ...)` are generic over `PgExecutor`, so
the same SQL serves a pooled connection or a `&mut *tx`. The repo trait
methods delegate to them — the SQL keeps exactly one home.
* `emit_on` / `emit_shared_on` take a `&mut PgConnection` and run the whole
fan-out on it. The `ServiceEventEmitter` impl acquires one pooled
connection and calls them, so behaviour is unchanged today; a caller
holding a transaction can now pass `&mut *tx` and have the outbox rows
commit with the write.
* `OutboxEventEmitter::new` takes the `PgPool` directly (it was only ever
constructed once, in the host wiring).
Also collapses the three copy-pasted per-app match queries (kv/docs/files
differ only in the `kind` discriminator and detail table) and the three
`emit_*` bodies into one `plan()` + one match fn, so the suppression
anti-join, the chain walk, and the empty-ops-means-any-op semantic each
exist once rather than three times.
No behaviour change — pure refactor. It is the seam the transactional
write lands on next.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The §11.6 M4 byte quota mixed two incompatible byte measures: `used` came from
Postgres as `octet_length(value::text)` (jsonb CANONICAL text — whitespace after
`:` and `,`, keys normalized), while `old_len`/`new_len` were computed in Rust
with `serde_json::to_vec` (COMPACT, no spaces). The projection
`used - old_len + new_len` therefore subtracted and added a *smaller* measure
than what is actually stored, so a group could be admitted over its ceiling —
the cap drifted permissive. The two forms are not reconcilable in Rust (jsonb
also reorders/dedups keys), so the projection has to be measured by Postgres.
Add `projected_total_bytes` to `GroupKvRepo` (keyed by collection+key) and
`GroupDocsRepo` (keyed by the replaced doc id, `None` on create). Each computes
`SUM(canonical) - existing_row(canonical) + new_value(canonical)` in ONE query,
binding the new value as compact TEXT and re-parsing it through `::jsonb::text`
so PG measures it in exactly the form it stores. All three terms now share one
metric. The services call it instead of the mixed Rust math; the near-cap
fast-path (`rows_after * max_value_bytes <= ceiling` → skip the SUM) is kept, so
the common case still does no extra work. The docs `update` path no longer needs
its old-doc fetch (the subtraction happens in SQL), removing a read.
The in-memory test repos implement the same projection with their own (compact)
metric, so the quota unit tests keep their exact byte arithmetic. No schema
change.
Note: the check-then-write TOCTOU (concurrent writers can each pass and
collectively overshoot) is NOT addressed here — it needs a transaction spanning
the check and the write, the same missing infrastructure as the transactional
outbox. Tracked separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The one-consumer-per-(app_id, queue_name) invariant has no DB unique index (the
parent's app_id and the detail's queue_name live in different tables), so it is
enforced by a pg_advisory_xact_lock on hashtext(app_id||queue_name) around a
SELECT-then-INSERT. But `materialize` guarded its own check-then-insert only
with the GLOBAL rematerialize lock, not that per-(app,queue) key — so a
tree-apply / app-create rematerialize could interleave with an interactive
`create_queue_trigger` for the same (app, queue): each SELECT sees no consumer
(the other's INSERT uncommitted), both INSERT, and the app ends up with TWO
enabled consumers draining the same queue, splitting messages unpredictably.
Fix: `materialize_one` now resolves the template's queue_name and takes the SAME
`advisory_lock_key(app_id, queue_name)` (exposed `pub(crate)`) before its slot
check + insert, so the two paths mutually exclude. No deadlock: the interactive
path never acquires the global rematerialize lock, so there is no lock cycle,
and the rematerialize lock already serializes reconcilers against each other.
No schema change.
stateful_templates DB + journey coverage stays green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A group's template suppression is "coarse by reference" (path for routes,
handler-name for triggers), and both resolution points dropped ANY inherited
row matching that reference across the whole subtree — including one a NEARER
descendant group deliberately re-declared. So an ancestor group G that declines
a far-ancestor's `/x` (or `audit` handler) would also silently kill a child
group H's OWN `/x` / `audit`-bound trigger at the same reference, violating the
documented "an owner can only decline what it inherits, never a descendant's
own rows" invariant.
Fix: gate each decline on the chain DEPTH of the suppressor vs the target's
owner — a suppressor at depth `d_s` may only decline a row whose owner is
strictly ABOVE it (`target_depth > d_s`):
- Routes: `list_route_suppressions` now returns `(app, path, suppressor_depth)`;
the rebuild folds it to the min depth per `(app, path)` and
`compile_effective_routes` skips an inherited route only when
`route.depth > suppressor_depth`. (This also subsumes the old `depth > 0`
inherited-only gate.)
- Triggers: the dispatch anti-join gains `AND sc.depth < c.depth` (the
suppressor's chain depth below the trigger owner's), correlating on the outer
`chain c` that every kv/docs/files/pubsub match query already binds.
An app's own suppression is depth 0 → still declines anything it inherits; a
group's suppression declines only what that group itself inherits. `sealed`
still overrides. No schema change.
Pinned by a new `compile_effective_routes` unit test (a depth-1 descendant
route survives a depth-2 suppression that declines a depth-3 template) and a
new `group_suppression` DB test (same for triggers); all existing suppression /
sealed / template journeys stay green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`claim` pre-increments `attempt` before the handler runs, so a real execution
failure legitimately counts toward `max_attempts`. But the two TRANSIENT
release paths — gate saturation (server at capacity) and script-disabled-at-
fire-time — re-queue the message WITHOUT executing it, and previously used the
same `nack` that leaves the pre-increment in place. Under sustained overload a
message could therefore be dead-lettered after N gate-rejections having run
zero times (each rejection burning a retry).
Add a non-counting `release` to both `QueueRepo` and `GroupQueueRepo`
(re-queue + `attempt = GREATEST(attempt - 1, 0)`, mirroring `nack` otherwise)
and route the two transient paths through a new `Dispatcher::q_release`. A real
handler failure still uses `nack` and counts. No schema change.
Pinned by a new `queue_release` DB test (release undoes the claim increment;
nack keeps it) and the updated `disabled_queue_consumer_...` dispatcher test
(now asserts a release, not a nack).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An end-to-end audit of the (unmerged) interceptor slice surfaced seven ways a
KV allow/deny guard could be silently defeated or misbehave. Close all of them:
1. Shared-collection bypass — `kv::shared_collection(...).set/delete` skipped
the hook entirely, so any `(kv, set/delete)` guard was circumvented by
choosing the shared handle. `GroupKvHandle` now runs the same before-op hook.
2. Seal to the declaring owner — the marker resolved nearest-owner-wins but its
script name was then re-resolved on the CALLER's chain, so a descendant app
could shadow a group's mandatory guard with a same-named local script.
`resolve_before` now returns the script sealed to the owner that declared the
marker (one LEFT JOIN in manager-core), so the executor never re-resolves by
name. A descendant can only override with its OWN explicit marker.
3. Re-entrancy — an "allow + audit-log" guard that itself wrote KV re-triggered
itself to the depth cap and then DENIED the original write (plus ~8x
resolve/compile/execute amplification). A thread-local guard makes a write
performed by an interceptor bypass interception.
4. Fail-closed verdict — only `#{ allowed: false }` denied; a bare bool, a
typo'd key, a non-bool, or a bare unit all ALLOWED. Now allow ONLY on an
explicit `#{ allowed: true }`; every other shape denies.
5/6. Fail-closed edges — a dangling (missing/disabled) interceptor script and a
missing engine back-reference now DENY instead of allowing.
7. AppInvoke coupling — resolution no longer routes through `invoke.resolve`, so
installing a guard no longer denies writes to principals who hold KV-write
but not AppInvoke.
The seam (`InterceptorService::resolve_before`) now returns an
`InterceptorResolution { None | Dangling | Run(ResolvedScript) }`; the executor
runs the sealed script straight through the shared `run_resolved_blocking` core
and drops its `InvokeService` dependency. executor-core stays Postgres-free.
No migration change (0073 unchanged).
Pinned by three new journeys in tests/interceptors.rs: the seal (a same-named
app script does NOT shadow the group's guard), shared-collection coverage, and
the fail-closed verdict. Full journey suite 157/157.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Smallest honest vertical slice of §9.4: a `[[interceptors]]` block (app OR
group) binds a script to run BEFORE `kv::set`/`delete`; it reads the operation
context (`ctx.request.body`: service, action, collection, key, value, caller
ids) and returns `#{ allowed, reason }` — `allowed == false` denies the op (the
write never runs, the caller gets a runtime error).
Reuses two existing mechanisms rather than inventing new ones:
- Registration mirrors extension points (§5.5): a marker table
`0073_interceptors.sql` (owner-polymorphic app_id/group_id XOR, keyed
(service, op) → script), `interceptor_repo` (insert/delete/list + the
nearest-owner-wins `resolve_before` chain walk), reconciled through the
declarative apply exactly like `vars` (create/update/delete, prunable).
- Execution reuses the `invoke()` re-entry path: the new `InterceptorService`
(shared trait + Postgres-backed impl) only RESOLVES the script name (keeping
executor-core Postgres-free); the executor's `sdk::interceptor::run_before`
resolves that name and runs it via `run_resolved_blocking` (extracted from
`invoke_blocking` — shared depth bound + AST cache). An un-hooked write pays
one indexed `Ok(None)` resolve; no interceptor ⇒ zero overhead.
Nearest-owner-wins so an app overrides a group's interceptor, and a group
interceptor is inherited by every descendant app — the chain walk is the
isolation boundary (a sibling subtree never matches). `validate_bundle_for`
restricts the MVP to `service = "kv"`, `op ∈ {set, delete}`, one marker per
(service, op).
Deferred (documented in §9.4): the `data` transform return, services other
than kv, `after_*` hooks, chaining + circular-dependency guard, the timeout
policy, and a `pic interceptors ls` read surface (needs a server route).
Pinned by `tests/interceptors.rs` (deny blocks the write; allow passes;
group→app inheritance), schema snapshot re-blessed. 154/154 journeys pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`users_service::require` open-coded the `AuthzDenied → {Forbidden, Backend}`
match that `authz::script_gate` centralizes for every other stateful service —
the one straggler. Extract the mapping into `authz::require_mapped` (used now by
both `script_gate` variants and `users_service::require`), so the verdict→error
mapping lives in exactly one place. Also refresh the stale `to_app_user` comment
(it referenced an unshipped "commit 8" stub; roles are resolved via
`fetch_roles` and passed in). No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two clean, local dedups in the group shared-collection services:
- Extract `GroupKvServiceImpl::check_value_size` — `set` and `set_if` inlined
the identical encode-and-cap block; now one method (mirrors the sibling
`group_docs_service::check_data_size`).
- Add `group_collection_repo::best_effort_emit_shared` — the KV/docs/files
services each copied the same emit-and-log-on-error tail for shared-collection
triggers. Centralize the tail (logging the event's own `source`/`op`); each
service still builds its own `ServiceEvent` (payload shapes differ).
Pure refactor, pinned by the existing group-service unit tests. (The
`owning_group` resolver was evaluated for dedup too but left as-is: the five
error enums diverge — `GroupFilesError::InvalidCollection` carries a `String`,
`GroupPubsubError` lacks the variant, and files/pubsub skip the empty-check kv/
docs/queue do — so a shared conversion would be a behavior change, not a
refactor.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial review of the v1.2 Workflows track surfaced two HIGH footguns and
several correctness/hardening gaps. Fixes:
HIGH
- pull round-trip: `pic pull` dropped `[[workflows]]`, so a later
`apply --prune` silently deleted them. The list endpoint now returns the full
`definition`; pull rebuilds the manifest block (inverse of workflow_to_wire).
- case-colliding names: two workflows differing only by case collided in the
reconcile diff (keyed by lower(name)), silently dropping one. Rejected up
front in validate_bundle_for.
MEDIUM
- reclaim retry budget: a crashed attempt (no outcome) consumed the retry
budget. reclaim_stale_steps now decrements `attempt` (floored) and clears
`next_attempt_at`, so a crash no longer counts as a failed try.
- on_error/backoff: typed the manifest fields against the shared enums so a
bad value fails at TOML parse with a clear message, not an opaque 500.
- dedupe depends_on: a repeated dependency inflated the Kahn in-degree past the
single decrement, reporting a spurious cycle for a valid DAG. Count distinct.
- canceled child: a canceled sub-workflow resolved the parent step with a
message naming the cause instead of a generic "failed"; documented that a
run-level cancel op is not yet supported.
LOW
- list_run_steps is now app-scoped at the query (JOIN workflow_runs) rather
than relying on caller pre-verification.
- partial failure is surfaced: a run that succeeds with on_error=continue
failures records them in the run's `error` field.
- admin-started runs log the root_execution_id against the principal.
- documented the deliberate when(missing→false) vs template(missing→fail)
asymmetry; corrected the claim's atomicity comment.
Tests: unit (dedupe-deps), DB-gated reclaim-budget assertion, and two new CLI
journeys (duplicate-name rejection, pull→plan clean round-trip). fmt + clippy
-D warnings clean; 450 lib + 14 orchestrator DB + 5 workflow journeys pass;
schema snapshot unchanged (no migration).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A per-app "Workflows" tab: list definitions, browse run history, and inspect a
run's per-step progress with a static layered DAG.
- API: the run-detail endpoint now returns each step's `depends_on` (loaded
from the definition via a new `get_workflow_by_id` reader) so the dashboard
can draw the graph edges — the run's step rows don't carry them.
- dashboard `$lib/api`: a `workflows` namespace (list / runs / start / run) +
the matching types.
- `apps/[slug]/workflows/+page.svelte`: workflow table → drill into runs →
drill into a run. The run view renders a step table plus an SVG DAG (nodes =
steps colored by status, laid out by longest-path level; edges = depends_on,
arrowed) and a "Start run" button. Nested/parked steps show their child run.
- AppTabBar + the per-app layout gain the Workflows tab (admin-only).
Tests: `npm run check` clean (0 errors); two Playwright smoke tests
(navigation tabs — workflows loads cleanly + renders its empty state) pass
against the full stack.
With M6 the v1.2 Workflows track (M1 schema/validation → M2 durable
orchestrator → M3 when/templating → M4 nested → M5 SDK/API/CLI → M6 dashboard)
is COMPLETE.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The user-facing surface for the durable DAG engine. A workflow run can now be
started three ways, all resolving the workflow by name in the caller's app
(`cx.app_id` / the resolved app — never a passed-in arg, the isolation
boundary), seeding a run the orchestrator advances.
SDK seam (shared):
- `WorkflowService` trait + `NoopWorkflowService` + `WorkflowError` in
`shared::workflow` (mirrors `InvokeService`); added to `Services` as a
noop-defaulted `with_workflow` builder (all `Services::new` call sites
unchanged).
- `WorkflowServiceImpl` (manager-core): resolves + seeds a run; authenticated
callers gated on `AppInvoke` (anonymous skips, script-as-gate), the same gate
`invoke()` uses.
- `executor-core::sdk::workflow` — the Rhai bridge `workflow::start(name, input)`
/ `workflow::start(name)`, registered in `register_all`. Returns the run id.
Admin API (manager-core `workflows_api`, mounted in the binary):
- `GET /apps/{app}/workflows` — definitions (AppRead)
- `POST /apps/{app}/workflows/{name}/runs` — start a run (AppInvoke)
- `GET /apps/{app}/workflows/{name}/runs` — run history (AppRead)
- `GET /apps/{app}/workflow-runs/{run_id}` — one run + its steps (AppRead)
`app_id` scopes every query; a foreign run 404s.
CLI (`pic workflows`): `ls` · `run <name> [--input JSON]` · `runs <name>` ·
`run-status <run_id>` — all `--app`-scoped; run definitions stay declarative
(`[[workflows]]` → `pic apply`).
Also: `list_runs_for_workflow` repo reader.
Tests: `workflow_service_start_seeds_a_run` (DB-gated — the SDK/API-shared
entry) + `workflow_run_executes_end_to_end` CLI journey (apply → `pic workflows
run` → poll `run-status` → succeeded, through the real binary + orchestrator).
fmt + clippy -D warnings clean, 449 manager-core lib tests, 14 orchestrator DB
tests, 18 executor-core lib tests, 3 workflow CLI journeys green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A `workflow`-kind step now starts a nested sub-workflow run instead of a
function. The orchestrator's step processing is restructured into prepare →
dispatch (`Prep`): after the shared context/`when`/input resolution, a
function step runs through the executor as before, while a workflow step:
- checks the nesting depth ceiling (`PICLOUD_WORKFLOW_MAX_DEPTH`; the child
would run at parent depth + 1) — over-deep → the step fails, not infinite;
- resolves the child workflow by name in the run's app scope;
- `start_child_and_park`: in one token-gated tx, seeds a child run (depth + 1,
`parent_run_id`/`parent_step_id` linkage, correlated under the same
`root_execution_id`) and PARKS the parent step (`running`, claim_token
cleared, `child_run_id` set). A parked step is never re-claimed (only `ready`
steps are) nor reclaimed (only *leased* running steps). The token-gated park
runs before the child insert, so a stale claim writes nothing (no orphan).
Each tick first runs `resume_finished_children`: a parent step parked on a
now-terminal child is resolved (child output → parent step output, or child
error → parent step failed) and the parent run advanced — idempotent via a
`status='running'` gate. The child's own steps are claimed by the same global
scan, so nesting is just more runs. A failed sub-workflow honors the parent
step's `on_error`.
Shared plumbing extracted for reuse: `advance_run_tx` (out of
`complete_step_and_advance`) and `seed_run_tx` (out of `start_run`).
Apply-time soft-warn (`workflow_nesting_warnings`, wired into `plan_warnings`):
a workflow that nests into itself — directly or via a mutual cycle within the
bundle — is flagged at plan (bounded by the depth ceiling, but almost always a
bug). Not an error.
Tests: nested output-flows-to-parent + depth-ceiling-fails-the-run (DB-gated,
end-to-end), self/mutual-cycle warning (pure unit). fmt + clippy -D warnings
clean, 449 manager-core lib tests, 13 orchestrator DB tests, 26 CLI journeys
(workflows/apply/plan/prune) green, binary boots.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire the two pure M1 modules into the orchestrator's execute path:
- Before running a step, build a `RunContext` from the run input + every prior
succeeded step's output (`load_run_step_outputs`, read fresh at execution
time so a step sees all upstream results).
- `workflow_expr::eval` the step's `when` condition (if any): false → the step
is `skipped` — never executed — and counts as satisfied-but-empty for its
dependents (a new `StepOutcome::Skipped`, written in `complete_step_and_advance`
then advanced; `compute_advance` already treats skipped as satisfying deps).
- `workflow_template::resolve` the step's `input` against the context: an
exact single `{{ ref }}` preserves the referenced JSON type, an embedded ref
interpolates as text; a missing reference is a hard step failure (a definition
bug surfaced, not hidden), never a silent null.
`when` and templates were already parse-validated at apply time (M1); this is
the runtime half.
Tests (DB-gated, end-to-end via a scripted fake executor):
- `when_false_skips_step` — b skipped, never executed, omitted from run output
- `step_output_flows_into_downstream_input` — a's `{n:7}` feeds b's input,
type-preserved for a bare ref and interpolated in a larger string
- `missing_input_ref_fails_the_step` — an unresolved ref fails b → run fails
Verified: cargo fmt, clippy -D warnings clean, 448 manager-core lib tests,
11 workflow_orchestrator DB tests (3 new).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The v1.2 Workflows durable DAG engine gains its runtime. A dedicated
background worker (`workflow_orchestrator.rs`, mirroring `cron_scheduler`,
NOT folded into the dispatcher) advances every in-flight run step-by-step,
durably, surviving restarts.
Per tick, two phases:
A. Claim + execute — up to a small batch of `ready`, due steps are claimed
with the same `FOR UPDATE SKIP LOCKED` competing-consumer lease the queue
uses (`claim_ready_step`), one execution-gate permit per step acquired
BEFORE the claim so the shared gate bounds real parallelism. Each step
resolves its function by name in the run's app scope (never a
script-passed arg — the isolation boundary), builds an `ExecRequest`, and
runs through the injected `ExecutorClient`. Claimed steps run concurrently.
B. Advance — the outcome is written and the DAG advanced in one token-gated
transaction (`complete_step_and_advance`): pending steps whose deps are
satisfied flip to `ready`, and the run's terminal status is recomputed.
Fan-in falls out (a join waits until its last dep flips it); a stale worker
matches zero rows and writes nothing.
The graph-advance decision is a pure, DB-free function (`compute_advance`) —
promotions + terminal run status, folding `on_error` fail-vs-continue and
skipped/failed dependency satisfaction — so it is unit-tested in isolation.
Retry uses the step's own policy via `compute_backoff`; a second, slower
cadence reclaims steps leased by a crashed worker (`reclaim_stale_steps`) — the
durability safety net. Steps run with no principal (like invoke_async), and log
under the new `ExecutionSource::Workflow` so `pic logs` surfaces them.
M2 executes function steps only; `when` + input templating land in M3, nested
sub-workflows in M4. Seams present (`StepTarget`, `run_input`, `workflow_depth`).
- migration 0072: widen the `execution_logs.source` CHECK with `workflow`
- shared: `ExecutionSource::Workflow`, `StepStatus::is_terminal`
- workflow_repo: run/step state — `start_run`, `claim_ready_step`,
`complete_step_and_advance`, `reclaim_stale_steps`, `get_run`,
`list_run_steps`, `compute_advance` (+ 7 pure unit tests)
- workflow_orchestrator: the worker + config (`PICLOUD_WORKFLOW_*` knobs),
spawned in `picloud/src/lib.rs` beside the dispatcher/cron scheduler
- tests: 8 DB-gated integration tests (linear, parallel fan-out/fan-in, retry,
on_error fail/continue, double-complete idempotency, stale-lease reclaim,
end-to-end tick with a fake executor)
Verified: cargo fmt, clippy -D warnings clean, 448 manager-core lib tests,
8 workflow_orchestrator DB tests, schema snapshot reblessed, M1 workflow CLI
journeys still pass (binary boots with the orchestrator wired in).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First milestone of the v1.2 Workflows track (blueprint §9.1/§9.2): the durable
DAG engine's foundation — the definition model, its validation, and the
declarative `apply` reconcile path. No execution yet (the orchestrator is M2).
- Migration 0071_workflows: `workflows` (owner-polymorphic like scripts, app-
owned in M1), `workflow_runs`, and `workflow_run_steps` (the per-step
competing-consumer lease table the M2 orchestrator will claim). Schema golden
reblessed.
- `shared::workflow`: the `WorkflowDefinition` / `WorkflowStepDef` DTOs (shared
by the CLI, apply, and the future orchestrator) + run/step status enums.
- Pure, DB-free `workflow_template` (input `{{ input.x }}` / `{{ steps.a.output.y }}`
resolution, type-preserving) and `workflow_expr` (a small safe JSON-predicate
evaluator for `when` — keeps manager-core free of a scripting engine).
- `workflow_repo`: read trait + reconcile tx free-fns (insert/update/delete).
- apply_service: `BundleWorkflow` + `Plan.workflows` + `CurrentState.workflows`
+ `ApplyReport.workflows_*`; server-side `validate_workflow_definition`
(unique/acyclic steps via topological sort, deps exist, function XOR workflow,
`when`/template parse) rejecting on a group node; `diff_workflows` by
lower(name) (Update on a definition change) + in-tx reconcile.
- CLI: `[[workflows]]` + `[[workflows.steps]]` manifest structs → wire bundle;
plan/apply render + report counts.
- Tests: 16 manager-core lib tests (template, expr, definition validation) +
the `workflows` CLI journey (apply → NoOp → prune; cyclic DAG rejected).
Deferred to later milestones: the orchestrator worker (M2), conditional/template
runtime wiring (M3), nested sub-workflows (M4), the SDK/API/CLI run surface
(M5), the dashboard (M6). The `group_id` column + run/step tables ship now so
those slot in without a migration churn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A five-agent roadmap-verification pass found the v1.2 Hierarchies code fully and
correctly implemented, but the three source-of-truth docs carried stale
"deferred/remaining/v1.3" notes — and one self-contradiction — for features that
have since shipped (§3 M3 server-side approval gate, §6 group create/reparent, the
Track A closeout, and the 2026-07-11 audit remediation). Reconcile all three to
reflect the true code; no behavior change.
CLAUDE.md: retract the inline "groups pre-exist" / "per-env approval deferred" /
"shared-topic SSE deferred" / "group dead-letter store deferred" / "email v0/no-AAD"
notes → mark §6, §3 M3, and Track A M1–M6 shipped; add per-group KV/docs byte
quotas + set_if; note migration 0063; refresh the "Out of MVP" section.
design doc: fix the header ("Phases 4–6 remain" → all shipped); resolve the §11.6
self-contradiction (the "Deferred" list still named shared-topic SSE / byte quotas /
set_if / operator admin API, all shipped — line 1317 already said SSE shipped); mark
the D3 dead-letter store shipped; retract the stale "groups pre-exist / §6 deferred"
forward references.
blueprint (comprehensive sweep): dashboard Alpine.js → SvelteKit + CodeMirror
(diagram, §3.3, tech table); Docker-per-execution → embedded in-process Rhai
(diagram + data flow); §12 Phase 4 "current focus" → shipped through v1.1.9; Phase 5
"in active development" + "Remaining" block → Hierarchies complete; per-app RBAC
"v1.3+" → shipped Phase 3.5; SDK reference → handle-pattern notation note, S3 tag
v1.1 → v1.3+; MVP schema + docker-compose flagged non-authoritative; §9 header noted
Hierarchies-shipped / Workflows-future; top status line refreshed.
Also fix two behavior-neutral stale in-code comments (sdk/kv.rs `kv::shared` →
`kv::shared_collection`; dispatcher.rs `q_terminal` "no group dead-letter store yet"
→ dead-letters to group_dead_letters, Track A M2).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remediate the MEDIUM backend correctness/perf findings from the 2026-07-11 audit.
B1 — a group cron template toggled disabled→enabled no longer re-fires every
descendant's cron. Materialization now KEEPS a disabled cron template's copies
(syncing `enabled` in place) instead of delete+recreate, preserving
`last_fired_at`; the scheduler already skips disabled copies, so keeping them is
inert. Queue/email arms still delete-on-disable (freeing the one-consumer slot).
B2 — `rematerialize_stateful_templates` warnings are now logged at the
app-create/delete and group-reparent chokepoints (were silently dropped); only
the `Err` arm was handled before.
B3 — the group KV/docs byte-quota check skips the O(rows) `SUM(octet_length(..))`
scan when a cheap upper bound (`rows_after * max_value_bytes`) is already under
the cap, so the full aggregate only runs near-cap. Every row is validated
≤ max_value_bytes, so the bound is sound (never lets an over-quota write through).
B6 — the in-process SSE broadcaster caps live channels per map
(`PICLOUD_REALTIME_MAX_CHANNELS`, default 100k); a subscribe that would open a
NEW channel past the cap is refused with 503 + `Retry-After: 1` rather than
growing the (app,topic)/(group,topic) maps unboundedly. Check+insert is atomic
under the map mutex (no TOCTOU); an existing-channel subscribe is exempt.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remediate the HIGH and security-relevant findings from the 2026-07-11 audit.
H1 — the per-env approval gate is now server-authoritative. The governing
project is resolved from the target node's nearest-claimed ancestor
(`governing_env_policy`/`_tree` + `governing_project_id` +
`ProjectRepository::get_environments_by_id`), independent of the client-supplied
`[project]`. Omitting or spoofing the project block can no longer skip a gate the
owning project established; a to-create group resolves its declared parent's
chain so a fresh subtree node inherits the gate. Fails closed on any read error.
H2 — the API-key prefix slice (`&rest[..8]`) is now the boundary-safe
`rest.get(..8)`, so an attacker-supplied multibyte bearer can't panic the request
task (unauthenticated per-request DoS). Regression test added.
C1 — admin sessions gain an absolute lifetime cap (migration 0070,
`PICLOUD_SESSION_ABSOLUTE_TTL_HOURS`, default 30d): `lookup` filters it, `touch`
clamps the sliding bump to it, so a continuously-used or stolen-but-warm token
self-expires. Mirrors the data-plane app-user cap.
C2 — `Cache-Control: no-store` on the login and API-key-mint responses (the two
that return a raw credential), so a proxy/CDN/browser cache can't retain it.
B8 — file downloads are header-safe: `sanitize_stored_filename` guarantees a
valid `HeaderValue` (no panic on a control-char name) and BOTH the per-app and
group download paths now set attachment + `X-Content-Type-Options: nosniff` +
a restrictive CSP, closing a group-path stored-XSS gap.
Also folds in the server-side plan-warning plumbing (`plan_warnings`,
`PlanResult::warnings`) and the `app_only_reject` message helper that the CLI
plan-preview change builds on, plus operator security notes (reads-open shared-
topic SSE; the `--env` label is advisory, not a boundary).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Shared TOPICS fanned out only to in-cluster trigger handlers; external clients
could not subscribe (per-app topics already can). Add SSE for shared topics.
- RealtimeBroadcaster gains a parallel (group_id, topic) channel map:
subscribe_group / publish_group / drop_group_topic (default no-ops so
NoopRealtimeBroadcaster + test doubles are untouched). InProcessBroadcaster
implements them with a second map; GC + channel_count span both.
- Route GET /realtime/shared/topics/{topic}: Host->app dispatch (as the per-app
route), then RealtimeAuthority::authorize_subscribe_shared resolves the OWNING
GROUP from the app's chain (kind=topic, root segment). Reads-open model — the
resolution IS the authorization, consistent with in-script shared reads; a
foreign-subtree app never resolves (404, the isolation boundary). No principal
machinery needed.
- GroupPubsubServiceImpl::with_realtime bridges a shared-topic publish to the
owning-group channel (best-effort) after the durable trigger fan-out.
- Host wires the broadcaster into the group pubsub service + the collection
resolver into the authority.
Auth-model note: chose reads-open (subtree app's Host is the grant) over
"authenticated principal + GroupKvRead" — it's both simpler and faithful to how
shared-collection reads already work. Pinned by realtime broadcaster group-map
tests, realtime_api shared-route tests (404 + stream), and
group_pubsub_service::publish_bridges_to_the_group_broadcaster. No migration.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No atomic CAS existed, so concurrent writers to the same KV key raced. Add
set_if(key, expected, new): writes only when the current value equals expected,
or (expected absent) only when the key is missing — the primitive for lock-free
counters / optimistic concurrency.
- KvService + GroupKvService traits gain set_if with a defaulted "unsupported"
impl (Noop + other impls untouched); the real services override it.
- kv_repo/group_kv_repo: atomic set_if — a single UPDATE ... WHERE value =
$expected (JSONB semantic equality) or INSERT ... ON CONFLICT DO NOTHING.
Atomic without a transaction; returns whether the write happened.
- Services enforce the same value-size cap + (group) writes-authed + row/byte
quotas as set; the event fires only on a successful swap.
- Executor Rhai SDK: kv::collection(c).set_if(key, expected, new) and the
shared_collection variant; Rhai () for expected means "only if absent".
Pinned by kv_service/group_kv_service set_if unit tests (CAS + fail-closed) +
sdk_kv::kv_set_if_compare_and_swap (through a real script). No migration.
docs/sdk-shape.md documents the primitive.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
M3 quotas capped a group's shared KV/docs by ROW count only; a group could still
blow past storage with few-but-huge values. Add the symmetric total-bytes ceiling
(files already had one).
- group_quota: PICLOUD_GROUP_{KV,DOCS}_MAX_TOTAL_BYTES env vars (default 256 MiB)
+ accessors.
- GroupKvRepo/GroupDocsRepo: total_bytes(group) = SUM(octet_length(value::text))
(default Ok(0) so non-Postgres impls skip the check).
- GroupKv/DocsServiceImpl: check the PROJECTED total (old value's bytes
subtracted, new added) on every set/create/update, so a same-or-smaller update
near the cap is still allowed (unlike a naive used+new check). New
TotalBytesQuotaExceeded errors; +with_max_total_bytes for tests.
- KV set switched has->get to obtain the old value's size for the delta.
Pinned by group_kv_service + group_docs_service per_group_byte_quota_uses_the_
projected_total unit tests (reject over-cap, allow same/smaller update near cap).
No migration. CLAUDE.md env-var table updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
email_trigger_details.inbound_secret_encrypted was sealed v0 (no AAD, bound only
to the master key) because the table had no version column — a ciphertext could
in principle be relocated between rows (audit 2026-06-11 H-D1, Medium). Bring the
AAD versioning the `secrets` table gained in 0042 to email secrets.
- Migration 0069: `email_trigger_details.inbound_secret_version SMALLINT DEFAULT 0`.
- secrets_service: `seal_email`/`open_email` seal v1 with AAD bound to the
SEALING OWNER (`email:{app}` / `email:group:{group}`) — deliberately NOT the
per-row trigger_id, so a group template's sealed bytes stay valid when the
materializer copies them verbatim to each descendant (all share the group AAD).
v0 rows keep their exact legacy no-AAD read path.
- Both email-trigger create paths (declarative apply resolve_and_seal +
triggers_api create_email_trigger) seal v1 under the trigger's owner; the
version threads through CreateEmailTrigger + insert_email_trigger_tx.
- materialize copies inbound_secret_version verbatim with the bytes.
- email_inbound_target recovers the sealing owner (materialized_from ->
template.group_id, else the app) so receive_inbound_email opens a v1 secret
under the right AAD.
Cross-app/tenant relocation now fails the GCM tag; a same-owner swap is not
distinguished (accepted low-severity residual). Pinned by
email_secret_aad::materialized_email_copy_decrypts_under_the_group_aad (the
novel materialized-copy path) + the extended secret_round_trips_through_seal_open
lib test. Schema golden reblessed; materialization + email e2e regressions green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An exhausted SHARED durable-queue message was `drop_exhausted()`-ed with a
warning — silent data loss. Per-app queues persist to `dead_letters` (0010);
add the symmetric group store so an exhausted shared-queue message is preserved
and operator-visible.
- Migration 0068: `group_dead_letters`, keyed by (group_id, collection),
CASCADE on the group (an app delete leaves the data — it belongs to the
group, not the consuming app).
- `GroupQueueRepo::dead_letter` (replaces `drop_exhausted`): one tx that INSERTs
the dead-letter + DELETEs the live message, filtered by claim_token so a lost
lease can't dead-letter a re-claimed message (mirrors queue_repo::dead_letter).
- Dispatcher `q_terminal` shared arm now dead-letters instead of dropping. It
returns None (not the dl id) so the per-app `fan_out_dead_letter` is SKIPPED —
firing the consuming app's per-app handlers on a shared message (competing
consumers → nondeterministic app) would be wrong. Fan-out to a *shared*
dead_letter trigger is deferred (needs a new trigger kind).
- Read side: `GroupDeadLetterRepo::list_for_group` backs a new read-only
operator endpoint GET /api/v1/admin/groups/{id}/dead-letters (GroupKvRead,
mirrors the M4 group-blobs surface). `pic dead-letters ls --group` deferred
(optional; the HTTP endpoint is the operator surface).
Pinned by group_queue::dead_letter_moves_an_exhausted_message_to_the_group_store
(store move + claim-token-mismatch guard + operator read). Schema golden
reblessed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-env approval policy was applier-supplied — a hand-crafted request
that omitted `project.environments` was ungated, and flipping a gate to
`confirm = false` in the same request un-gated it. Persist the policy
server-side and enforce against `persisted ∪ declared`.
- Migration 0067: `project_environments (project_id, env_name, confirm)`,
CASCADE on the project. Written declaratively (delete-then-insert) inside
`upsert_project_tx`, same tx as the project row + node claim.
- `ProjectRepository::get_environments_by_slug` (read side) +
`ApplyService::effective_env_policy` union the persisted policy with the
request's declared one (confirm if EITHER says so — monotonic).
- `env_gate_check` now evaluates the effective policy; the three handlers load
it before the gate. `plan.approvals_required` is the effective gated set, so
CI sees a persisted gate even when the manifest omits it.
- The union rule closes both bypasses: an omitted policy still trips a
persisted gate, and an ungating apply must itself pass the gate (the
declarative replace then takes effect next time) — TOCTOU-safe.
Pinned by env_approval::{persisted_policy_gates_a_request_that_omits_it,
flipping_a_gate_to_false_in_the_same_request_still_gates} +
projects_repo::get_environments_by_slug_returns_persisted_policy. Schema golden
reblessed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-env approval gate (`[project.environments]`, `confirm = true`) was
client-side only — the policy was `#[serde(skip_serializing)]`, so a patched or
older CLI applying to a confirm-required env bypassed it entirely. This makes
the gate server-enforced, admin-gated, and audited (the deferred §4.2 piece).
- Wire: `ProjectDecl` gains `environments` (the CLI stops skip_serializing it and
sends the selected `env` + the `--approve` set on the apply request). The
server RE-DERIVES the gate from the request (`env_gate_check`): a
confirm-required env not in `approved_envs` → 409 `ApprovalRequired`.
- Admin gate: an approved override additionally requires `AppAdmin`/`GroupAdmin`
on EVERY declared node — a step up from the editor write caps an ordinary
apply needs (reusing the admin caps per §4.2) — and emits a `tracing::info!`
audit line. Enforced on all three handlers (single app, single group, tree);
the check runs before any tx, so a denial (403) precedes any mutation.
- Token: the env policy folds into the TREE bound-plan token (a policy edit
between plan and apply trips StateMoved). The single-node token is left
unchanged (a bare live-state hash), so plan/apply still match without threading
a project into it.
- CLI: single-node `apply`/`plan` now resolve the GOVERNING project (own block
else nearest ancestor's) so a leaf apply carries the root's policy; `plan`
surfaces `approvals_required`. Client-side `require_env_approval` fast-fail
kept as UX.
Adapts the earlier feat/ownership-and-approval M5 (654e387) to main's DTOs; main
addresses tree group nodes by slug only, so the admin loop mirrors authz_tree
(app: resolve_app_id fail-closed; group: get_by_slug, skip to-create).
Threat-model note (honest scope, in the doc + code comments): this closes the
patched/older-CLI bypass and admin-gates + audits the override, but is NOT a
hermetic boundary against a hand-crafted request that OMITS the policy — the
policy is applier-supplied, not persisted server state. Full closure (persist a
`project_environments` source of truth + a server-determined env) is deferred.
Review (adversarial pass): fixed a MEDIUM (single-node `plan` didn't resolve the
governing project, so a leaf plan under-reported the gate `apply` enforces) and a
LOW (duplicate clippy attr); corrected overstated "can't bypass" wording.
Tests: ProjectDecl gate unit test; env_approval journeys
`server_enforces_the_gate_against_a_non_stock_client` (raw request → 409 without
approval, admin-approved → 200) and `approving_a_gated_apply_requires_admin`
(editor + --approve → 403). 417 lib tests + 33 CLI journeys + workspace clippy
-D warnings + fmt clean. No migration.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`group_blast_radius`'s `app_chain` up-walk is depth-capped (`ac.depth < 64`,
added by 76926de to stop a should-be-impossible `groups.parent_id` cycle from
spinning the recursive CTE forever), but the sibling `subtree` down-walk in the
same query was left unbounded. A `parent_id` cycle would loop `subtree`
indefinitely and hang the plan — the exact exposure 76926de set out to close,
left half-done on the descendant side.
Add a `depth` column + `WHERE s.depth < 64` to `subtree`, mirroring `app_chain`.
Defense-in-depth: the reparent cycle-guard already prevents `parent_id` cycles,
so this hardens against an unreachable state — but that is precisely the bar
76926de applied. Surfaced by a post-adoption audit of the §7/§6 ownership code.
Verified: 416 manager-core lib tests; 12 ownership/create/divergence/approval
journeys (incl. plan_previews_ownership_and_blast_radius, which drives this
query); clippy -D warnings + fmt clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
