4 Commits

Author SHA1 Message Date
MechaCat02
251d7fe3dd test(e2e): give each dispatcher test its own database
`dispatcher_e2e` failed about one run in three, on a different test each time,
and never under `--test-threads=1`. Root cause, not timing:

Each test calls `build_app`, which spawns a REAL dispatcher, and they all shared
one database — so they shared one `outbox`. `OutboxRepo::claim_due` is
deliberately not app-scoped (in production one dispatcher serves the whole
instance, so claiming any due row is correct). Test A's dispatcher would
therefore claim test B's row, test A would finish, its server would be dropped
mid-dispatch, and the claim was stranded. Test B polled for its handler's side
effect until it timed out.

The harness was modelling something production never does: N independent
instances sharing one database. So the fix belongs in the harness. Weakening the
dispatcher (app-scoping `claim_due`) would be wrong, and shortening
`PICLOUD_OUTBOX_CLAIM_TIMEOUT_SEC` to fit a 10s test window would make production
abandon the live claims of scripts that may legitimately run 300s.

`tests/common` now hands each test its own database. Replaying 73 migrations per
test is too slow, so the first caller builds a migrated TEMPLATE database once
and every test clones it (`CREATE DATABASE ... TEMPLATE` is a file copy), guarded
by a cluster-wide advisory lock because Postgres will not clone a template that
has an open connection. Names are derived from (suite, test), so a test reclaims
its own database on entry: a crashed run leaves nothing to clean up.

Only the five local `pool_or_skip` wrappers change — all 33 call sites are
untouched, and the skip-when-`DATABASE_URL`-is-unset contract is preserved.
(`#[sqlx::test]` already does this and is what `api.rs`/`authz.rs` use, but it
requires `DATABASE_URL` and would force `#[ignore]`, silently dropping these
tests from the default `cargo test --workspace` gate.)

This also stops the e2e tests leaving stranded claims behind in the shared dev
database, which is the likely source of the occasional lone journey failure.

Before: dispatcher_e2e red ~1 run in 3. After: workspace green twice over,
journeys 157/157.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-14 21:40:27 +02:00
MechaCat02
c3baa87415 chore(v1.1.9): clippy clean — workspace -D warnings
Surfaced + fixed during the F3 attestation:

- executor-core/sdk/queue.rs: drop redundant .map(|()| ()) call sites;
  enqueue_blocking discards QueueMessageId via .map(|_id| ()) (intentional)
- executor-core/sdk/retry.rs: hoist use std::collections::hash_map::DefaultHasher
  + use std::hash::Hasher to the top of the file (clippy::items_after_statements);
  replace `as i64` casts with i64::try_from + clear comment about jitter
  bound (clippy::cast_possible_wrap)
- executor-core/sdk/invoke.rs: move _LIMITS_IS_COPY const before #[cfg(test)]
  mod tests (clippy::items_after_test_module)
- manager-core/dispatcher.rs: dispatch_one_queue gains #[allow(too_many_lines)]
  (it's the queue tick's whole logic — split makes it less readable than
  the lint); .map(...).unwrap_or(default) → .map_or(default, ...)
- picloud/lib.rs: Limits { trigger_depth_max, ..Limits::default() } via
  struct-update instead of let-mut-assign (clippy::field_reassign_with_default)
- tests: r#"..."# → r"..." where there are no `"` inside (clippy::needless_raw_string_hashes);
  combine InvokeTarget::Name | InvokeTarget::Path arms with identical bodies
  in sdk_invoke.rs (clippy::match_same_arms)

cargo fmt --all -- --check 2>&1 | tail -3: no output (exit 0)
cargo clippy --workspace --all-targets --all-features -- -D warnings:
  Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.41s
Workspace lib smoke: 432 passing (306 manager-core + 74 executor-core + 34 shared + 18 orchestrator-core).
DB-gated E2E (against docker compose postgres on localhost:15432):
  queue_e2e: 4 ok / invoke_e2e: 4 ok / retry_e2e: 3 ok / migration_queue_messages: 4 ok / schema_snapshot: 1 ok

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-07 11:02:22 +02:00
MechaCat02
c38c46b8bc test(v1.1.9): fix invoke_e2e + retry_e2e — admin bypass + rhai shape
Running the E2E suites against real Postgres surfaced three shape bugs
in the test scripts that caused false failures:

invoke_e2e:
- invoke_cross_app_rejects used two TestServer instances (one per app),
  but the second server's Owner admin isn't a member of the first
  server's app. Replaced with a single server that creates both apps
  via the same Owner admin (which has implicit access to every app).
- invoke_depth_limit_exceeds_cleanly: the recurser script had its own
  try/catch, so when the depth limit fired inside the deepest call the
  caught error became the BODY of a 200 response (which invoke()
  returns to the caller). The outer caller's try/catch never saw a
  throw → assertion failed. Rewrote so the recurser propagates throws
  (no inner try-catch); the outer caller's try-catch surfaces the
  depth error all the way up.

retry_e2e:
- All three tests used HTTP routes which need a domain claim the test
  apps don't have (`no app claims host ""` 404s). Switched to the
  admin bypass POST /api/v1/execute/{id} — same pattern dispatcher_e2e
  uses. Sidesteps the per-app domain matcher entirely.
- retry_run_surfaces_last_error_after_max_attempts: try-catch is a
  statement in Rhai, not an expression, so the block didn't evaluate
  to the catch arm's map. Refactored to bind to `let out` inside the
  catch arm, then return `#{ statusCode: 200, body: out }` as the
  final expression.

All 11 v1.1.9 E2E tests now pass against Postgres:
  queue_e2e: 4 passed (33s — exercises retry + dead-letter)
  invoke_e2e: 4 passed (2s)
  retry_e2e: 3 passed (2.5s)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-07 10:54:52 +02:00
MechaCat02
328d7d55e6 test(v1.1.9): queue_e2e + invoke_e2e + retry_e2e + migration test
Four new test binaries, DB-gated via the established pool_or_skip()
pattern (returns Some(pool) when DATABASE_URL is set; prints a skip
notice and returns None otherwise — so plain `cargo test` stays green
locally while CI runs them).

crates/picloud/tests/queue_e2e.rs (4 tests):
  - queue_receive_acks_on_success: enqueue → consumer fires → KV
    marker observable → queue row deleted (ack worked)
  - queue_receive_dead_letters_after_max_attempts: throwing handler
    retries 3× via the dispatcher's compute_backoff path → dead_letters
    row written, queue row deleted
  - queue_one_consumer_per_queue_rejected: second trigger create for the
    same (app_id, queue_name) returns 4xx with the documented "already
    has a consumer trigger" message (advisory-lock guard works)
  - queue_visibility_timeout_reclaim: insert message with stale claim
    (claimed_at = NOW() - 60s, visibility_timeout = 5s); either the
    dispatcher re-claims after reclaim runs, or the claim clears

crates/picloud/tests/invoke_e2e.rs (4 tests):
  - invoke_by_name_same_app_returns_value: callee returns body.x+1;
    caller invokes by name and writes the result to KV (42 round-trips)
  - invoke_cross_app_rejects: callee in app_B, caller in app_A; error
    string contains "different app" or "CrossApp"
  - invoke_depth_limit_exceeds_cleanly: recursive script throws with
    "depth" in the message at the bound (Limits::trigger_depth_max=8)
  - invoke_async_enqueues_outbox_row: invoke_async returns execution_id,
    dispatcher fires the OutboxSourceKind::Invoke arm, callee marker
    appears

crates/picloud/tests/retry_e2e.rs (3 tests):
  - retry_run_eventually_succeeds_inside_http_handler: counter mutates
    across 3 retries through a real HTTP route, returns 3
  - retry_run_surfaces_last_error_after_max_attempts: max_attempts=2,
    always throws — caller's try/catch surfaces "boom"
  - retry_on_codes_filters_unmatched_errors: counter == 1 after a
    throw NOT in the codes list (immediate surface, no retry)

crates/manager-core/tests/migration_queue_messages.rs (4 tests):
  - queue_messages_table_exists_with_expected_columns: shape +
    nullability of every column
  - queue_trigger_details_table_exists: shape
  - queue_widens_trigger_kind_and_outbox_source_kind: triggers.kind
    CHECK admits 'queue'; outbox.source_kind admits 'invoke'
  - queue_messages_dispatch_index_is_partial: idx_queue_messages_dispatch
    has WHERE claim_token IS NULL (guards against future migrations
    accidentally dropping the partial)

All 15 tests pass clean skip when DATABASE_URL is unset. Workspace unit
suite remains at 432 passing (306 manager-core + 74 executor-core + 34
shared + 18 orchestrator-core).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-06-07 10:25:22 +02:00