From 982ff6294942197e83fcd65b1a4918a2f327a8e1 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:06:50 -0500 Subject: [PATCH] test(shared): consolidate ZIP conversion observations (#16118) Co-Authored-By: Paperclip --- ...apter-utils-upload-duplicate-test-audit.md | 45 +++ ...-adapter-validation-route-fixture-audit.md | 21 ++ ...agent-instructions-module-reuse-recheck.md | 25 ++ ...routes-duplicate-canonicalization-audit.md | 18 ++ ...6-10-13-codex-reconciliation-test-audit.md | 123 ++++++++ ...-13-db-client-test-migration-pass-audit.md | 29 ++ ...execution-wrapper-test-audit-controls.json | 112 +++++++ ...2026-10-13-execution-wrapper-test-audit.md | 48 +++ ...teraction-audience-duplicate-rows-audit.md | 38 +++ ...n-route-duplicate-boundaries-test-audit.md | 20 ++ ...6-10-13-issue-comment-wakeup-test-audit.md | 56 ++++ ...sue-work-product-attribution-test-audit.md | 33 +++ ...-13-issues-service-fixture-replay-audit.md | 31 ++ ...3-onboarding-route-duplicate-test-audit.md | 36 +++ ...6-10-13-pipeline-bulk-review-test-audit.md | 52 ++++ ...10-13-shared-access-null-dsn-test-audit.md | 50 ++++ doc/plans/2026-10-13-shared-zip-test-audit.md | 85 ++++++ ...13-telemetry-client-contract-test-audit.md | 54 ++++ ...untracked-worktree-duplicate-test-audit.md | 108 +++++++ ...026-10-13-worktree-config-fixture-audit.md | 55 ++++ .../src/command-managed-runtime.test.ts | 31 +- packages/db/src/client.test.ts | 15 - .../standalone-thin-paperclip-adapter.md | 9 +- .../scripts/generate-protocol-coverage.mjs | 2 +- .../spec/capability/protocol-coverage.json | 2 +- packages/shared/src/portability-zip.test.ts | 25 +- packages/shared/src/telemetry/README.md | 4 +- .../src/telemetry/client-contract.test.ts | 205 ------------- packages/shared/src/telemetry/client.test.ts | 15 +- packages/shared/src/validators/access.test.ts | 11 +- .../agent-adapter-validation-routes.test.ts | 16 +- .../agent-instructions-routes.test.ts | 66 +---- .../src/__tests__/agent-skills-routes.test.ts | 48 --- .../heartbeat-native-runner-selection.test.ts | 2 - .../heartbeat-retry-scheduling.test.ts | 26 -- ...esolution-cross-issue-cap-postgres.test.ts | 7 +- ...ue-agent-mutation-ownership-routes.test.ts | 16 - .../issue-thread-interaction-routes.test.ts | 121 +------- ...issue-update-comment-wakeup-routes.test.ts | 57 ---- server/src/__tests__/issues-service.test.ts | 279 ++++-------------- .../__tests__/native-run-finalizer.test.ts | 4 - .../native-runner-phase6.integration.test.ts | 3 - server/src/__tests__/pipelines-routes.test.ts | 6 +- .../src/__tests__/workspace-runtime.test.ts | 21 -- server/src/__tests__/worktree-config.test.ts | 159 ++-------- ...ssue-thread-interaction-resolution.test.ts | 8 +- ui/src/lib/onboarding-route.test.ts | 33 --- 47 files changed, 1183 insertions(+), 1047 deletions(-) create mode 100644 doc/plans/2026-10-13-adapter-utils-upload-duplicate-test-audit.md create mode 100644 doc/plans/2026-10-13-agent-adapter-validation-route-fixture-audit.md create mode 100644 doc/plans/2026-10-13-agent-instructions-module-reuse-recheck.md create mode 100644 doc/plans/2026-10-13-agent-skills-routes-duplicate-canonicalization-audit.md create mode 100644 doc/plans/2026-10-13-codex-reconciliation-test-audit.md create mode 100644 doc/plans/2026-10-13-db-client-test-migration-pass-audit.md create mode 100644 doc/plans/2026-10-13-execution-wrapper-test-audit-controls.json create mode 100644 doc/plans/2026-10-13-execution-wrapper-test-audit.md create mode 100644 doc/plans/2026-10-13-interaction-audience-duplicate-rows-audit.md create mode 100644 doc/plans/2026-10-13-interaction-route-duplicate-boundaries-test-audit.md create mode 100644 doc/plans/2026-10-13-issue-comment-wakeup-test-audit.md create mode 100644 doc/plans/2026-10-13-issue-work-product-attribution-test-audit.md create mode 100644 doc/plans/2026-10-13-issues-service-fixture-replay-audit.md create mode 100644 doc/plans/2026-10-13-onboarding-route-duplicate-test-audit.md create mode 100644 doc/plans/2026-10-13-pipeline-bulk-review-test-audit.md create mode 100644 doc/plans/2026-10-13-shared-access-null-dsn-test-audit.md create mode 100644 doc/plans/2026-10-13-shared-zip-test-audit.md create mode 100644 doc/plans/2026-10-13-telemetry-client-contract-test-audit.md create mode 100644 doc/plans/2026-10-13-untracked-worktree-duplicate-test-audit.md create mode 100644 doc/plans/2026-10-13-worktree-config-fixture-audit.md delete mode 100644 packages/shared/src/telemetry/client-contract.test.ts delete mode 100644 server/src/__tests__/heartbeat-native-runner-selection.test.ts delete mode 100644 server/src/__tests__/native-run-finalizer.test.ts delete mode 100644 server/src/__tests__/native-runner-phase6.integration.test.ts diff --git a/doc/plans/2026-10-13-adapter-utils-upload-duplicate-test-audit.md b/doc/plans/2026-10-13-adapter-utils-upload-duplicate-test-audit.md new file mode 100644 index 00000000000..219425111a7 --- /dev/null +++ b/doc/plans/2026-10-13-adapter-utils-upload-duplicate-test-audit.md @@ -0,0 +1,45 @@ +# Command-managed-runtime upload duplicate retirement — 2026-10-13 + +## Scope and baseline + +- Base: `418eb4623ae767b5c9ce28c1c3e9fcb98f63702e`, the head of [PR #16173](https://github.com/paperclipai/paperclip/pull/16173). +- Branch: `codex/test-audit-r19-adapter-utils-upload-case`, stacked on `codex/test-audit-r18-worktree-config`. +- Production owner: `createCommandManagedRuntimeClient().writeFile` in `packages/adapter-utils/src/command-managed-runtime.ts`. `prepareCommandManagedRuntime` constructs this client for command-backed adapter staging and fallback sync operations. +- Owner file: `packages/adapter-utils/src/command-managed-runtime.test.ts`. +- Method: [OpenClaw test-audit](https://github.com/openclaw/openclaw/blob/main/.agents/skills/test-audit/SKILL.md), its audit workflow, owning-boundary guide, and proof policy. +- Required repository guidance, contributor rules, roadmap, and PR template were consulted. GitHub searches for the exact case and upload-test consolidation found no duplicate retirement. Related adapter audit: [PR #16115](https://github.com/paperclipai/paperclip/pull/16115). + +## Retirement and retained proof + +Retire only `test_single_stream_writeFile_collapses_roundtrips_under_96MiB`, originally at line 921. It was added in `030dd9d15c` ([PR #10340](https://github.com/paperclipai/paperclip/pull/10340)) as Research A1 proof for the single-stream round-trip reduction. Its two real-shell uploads duplicate the existing transport owner cases. + +Before removal, strengthen the existing single-stream case at line 462 from 3 MiB to 9 MiB. This preserves the retired case's exact size witness. Its payload still contains every byte value, and it still checks exact file bytes, two total shell calls, cleanup, stdin transport, monotonic progress, and final progress. Change the existing base64 guard to allow exactly a 9 MiB source buffer. The guard checks decoded buffer length, not the resulting string length. + +The production threshold is **96 MiB of encoded base64**, not decoded payload bytes. A 9 MiB payload encodes to 12 MiB. Reverting to chunked uploads for this witness fails the surviving exact two-call assertion. Neither the retired case nor this keeper proves the exact 96 MiB boundary or the above-threshold branch. + +Strengthen the existing 12 MiB chunked-progress case from `calls.length > 2` to `3 + Math.ceil(payload.byteLength / (3 * 1024 * 1024))`: seven total shell calls. The three fixed calls initialize the staged file, rename it, and clean it up. The remaining four calls upload decoded 3 MiB chunks. Its binary-byte, bounded-stdin, monotonic-progress, and final-progress assertions stay intact. This is the retired chunk-count formula applied at the surviving larger payload. + +The 10 MiB partial-final-chunk case, omitted-capability case, cleanup-failure case, and all 22 other cases stay byte-for-byte unchanged. The retired cross-case greater-than assertion is implied by the retained exact counts of two and seven. + +## Cost and lines + +- Remove one case: owner file goes from 28 to 27 cases. +- Remove the 23-line declaration and its following blank line. Four existing lines change. Git reports 4 added / 28 removed physical test-file lines: net **24 fewer physical lines**. +- Production LOC and support LOC change: zero. This receipt is the only additional tracked file. +- Retired fixture performed eight real shell invocations: two single-stream and six chunked. Growing the surviving single-stream payload adds no invocations. Net reduction: **eight shell invocations**. +- Retired fixture uploaded 18 MiB. Growing the keeper adds 6 MiB. Net reduction: **12 MiB of decoded upload payload** per owner run. +- Prior three-file adapter-utils audit baseline: 87/87 passed. The retired case cost 228.125125 ms. Its original 3 MiB keeper cost 51.266500 ms. +- Candidate 9 MiB keeper cost 163.982042 ms. Prior owner duration: 1,495.937988 ms; candidate owner duration: 1,979.989990 ms. These are separate host runs, not a controlled performance comparison. They do **not** establish a suite speedup. The deterministic process and byte reductions are the supported savings. + +## Validation + +- `pnpm exec vitest run packages/adapter-utils/src/command-managed-runtime.test.ts --maxWorkers=1 --reporter=json --outputFile=/private/tmp/test-audit-r19-command-managed-runtime-candidate.json`: **27/27 passed**. Recorded command wall time: 7.421 s, including collection and startup. +- `pnpm --filter @paperclipai/adapter-utils typecheck`: passed. +- `git diff --check`: passed. +- Exact diff inspection confirmed the encoded threshold semantics and fixed overhead in the chunk count. A declaration comparison confirmed exactly one removed case, exactly two changed keepers, and all other 25 cases byte-for-byte unchanged. +- No production mutation experiments ran. No production code, dependencies, contracts, UI, or CI routing changed. +- Repo-wide typecheck, test suite, and build were not run locally for this narrow retirement. This is focused owner proof; broad CI gates and review remain pending before merge. + +## Risk and disposition + +Low risk with directly retained owner proof. The larger single-stream fixture costs more than the original 3 MiB keeper, and timing savings are modest and unproven at suite level. Independent transport, capability-default, partial-chunk, progress, atomic rename, and cleanup contracts remain. Open a stacked PR for review; do not merge. diff --git a/doc/plans/2026-10-13-agent-adapter-validation-route-fixture-audit.md b/doc/plans/2026-10-13-agent-adapter-validation-route-fixture-audit.md new file mode 100644 index 00000000000..44c7ce4c9eb --- /dev/null +++ b/doc/plans/2026-10-13-agent-adapter-validation-route-fixture-audit.md @@ -0,0 +1,21 @@ +# Agent adapter validation route fixture audit — 2026-10-13 + +Status: remove one duplicate positive test. Based on `github/master` `d67e3507b23041212fa26302ce6de078c5ebcc05`. Owner: `server/src/__tests__/agent-adapter-validation-routes.test.ts`. Production code is unchanged. + +## Fixture consolidation and removed declaration + +The dynamic external-adapter positive now sets an unrelated adapter as disabled. This same retained declaration proves the registered `external_test` adapter remains selectable and asserts both HTTP 201 and the returned adapter type. Remove the weaker duplicate positive that separately asserted only HTTP 201. Keep the disabled-selection rejection cases and the case that permits editing an existing disabled adapter. + +The audit also considered deleting the local `requestApp` helper and replacing its 38 uses with direct `request(app)` calls. Do not include that cleanup here: the direct-request version repeatedly approached or exceeded the 15-second first-test timeout locally, while the focused baseline passed. Keep the helper until a broader timing comparison shows no regression. + +The remaining route declarations continue to cover provider-specific model refresh, credentials, rollback, Runner rollout, Dot authorization, provider admission, validation failures, and partial edits. The production entry point remains `routes/agents.ts`; UI configuration and create/hire flows call these routes. CI discovers this filename in the serialized route/authz lane. + +## Local validation and savings + +The historical serialized weight is 24,906 ms and includes isolated process startup, imports, tests, and teardown. It is not a measured runtime reduction. The change removes one test declaration and about 13 test lines, including its separator. It removes no production lines, and no CI time reduction is claimed. + +Focused exact-head validation passed: `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/agent-adapter-validation-routes.test.ts` reported **44/44 passing** in 29.57 seconds with local bind access. `git diff --check` is also required before commit. Relevant adapter registry/environment siblings remain to be validated if later changes touch their behavior. + +## Integration with current master + +The disabled-adapter fixture moves into the existing dynamic-registration keeper. Newer saved-legacy, harness-selection and native-Codex rollout regression cases from master are retained unchanged. diff --git a/doc/plans/2026-10-13-agent-instructions-module-reuse-recheck.md b/doc/plans/2026-10-13-agent-instructions-module-reuse-recheck.md new file mode 100644 index 00000000000..fff6e8efc66 --- /dev/null +++ b/doc/plans/2026-10-13-agent-instructions-module-reuse-recheck.md @@ -0,0 +1,25 @@ +# Agent instruction route graph reuse recheck (2026-10-13) + +Owner: `server/src/__tests__/agent-instructions-routes.test.ts`. Rechecked in `/private/tmp/paperclip-test-audit-pr-stack` at `9b4d0cd872ca1fc897f320cb181fc060f0bd5bf1` without changing its branch. The owner blob is identical to both `codex/test-audit-r15-onboarding-duplicates` at `568ba003a5` and the parent of prior candidate `0e5043c65513ac470208152dd90e0e74b6dcd022`: `4f7a65449b3d5ed70539bec18b1865e18c7648b7`. + +The earlier 34-line candidate removes duplicated module setup, not duplicated endpoint executions. Applied its exact owner patch after the parent explicitly authorized this setup reduction. Test source changes by +16/-50 lines, 853 to 819 lines, net -34. Registered and expanded cases remain 24; removed executions and assertions: zero. Production files changed: zero. + +`registerModuleMocks` duplicated the existing hoisted mock registrations. The existing `hoistModuleGraph` helper loads route and middleware exports once; every test still constructs a fresh app/router and HTTP server. `vi.resetAllMocks` clears implementations and once queues as well as call histories; each test's defaults are rearmed, including the secret normalization implementation formerly preserved by `clearAllMocks`. The route's hire lock map is not used by these fixtures because their actor has no run ID. The existing helper inherits the server's 30-second hook timeout, replacing the previous explicit 60-second per-case preload timeout. + +The complete section from the first test declaration through the final lifecycle mock is byte-identical before and after, SHA256 `6b0b031908125729e8a135878a5d5245fa7a4137520df79ddf295c25ffcd2353`. JSON reports contain identical ordered names for all 24 expanded cases. Retained regressions cover external-root and mislabeled-managed-root instance-admin restrictions; generic patch, hire, direct creation and compatibility endpoints; own/peer read authorization; canonical revisions and forged identity/base-revision rejection; binary and empty downloads; candidate resolution scope; history/diff/restore; adapter merge/replacement and skill-selection preservation. + +Full owner command, with authorized local loopback access: + +```sh +node node_modules/vitest/vitest.mjs run --project @paperclipai/server server/src/__tests__/agent-instructions-routes.test.ts --reporter=json --outputFile=/private/tmp/agent-instructions-current-ARM.json +``` + +| Arm | Passed | Report start-to-suite-end duration | +| --- | --- | --- | +| Baseline | 24/24 | 20.816 s | +| Candidate | 24/24 | 7.268 s | +| Candidate repeat | 24/24 | 7.445 s | + +These are local observations on a concurrently used host, with one baseline and no reverse-order control. They support stable candidate behavior, not a quantified performance or CI speedup claim. Two sandboxed attempts stalled at HTTP fixtures and were cancelled (exit 130); they are excluded. Existing provider-trace cleanup warnings also appeared in passing baseline and candidate runs. + +Exact PR-branch rerun also passed 24/24 in 8.06 seconds with `node_modules/.bin/vitest run --project @paperclipai/server server/src/__tests__/agent-instructions-routes.test.ts --silent=true`. `git diff --check` passed. Reports: `/private/tmp/agent-instructions-current-baseline.json`, `/private/tmp/agent-instructions-current-candidate.json`, and `/private/tmp/agent-instructions-current-candidate-repeat.json`. No staging, commit, push, branch switch, or edits to other owners were performed. Repo-wide checks were not run for this narrow test-only worker handoff. diff --git a/doc/plans/2026-10-13-agent-skills-routes-duplicate-canonicalization-audit.md b/doc/plans/2026-10-13-agent-skills-routes-duplicate-canonicalization-audit.md new file mode 100644 index 00000000000..a568e2a5b10 --- /dev/null +++ b/doc/plans/2026-10-13-agent-skills-routes-duplicate-canonicalization-audit.md @@ -0,0 +1,18 @@ +# Agent skills route duplicate canonicalization audit — 2026-10-13 + +Status: duplicate-proof deletion, based on `github/master` `d67e3507b23041212fa26302ce6de078c5ebcc05`. Owner: `server/src/__tests__/agent-skills-routes.test.ts`. Production code and unrelated declarations are unchanged. + +## Removed declarations and retained keepers + +Remove two successful canonicalization replays: + +- The sync case that sends the `paperclip` alias with `mode: replace` and checks the canonical persisted singleton is covered by the retained complete-replacement case. That keeper starts with an existing assignment, performs the same request and checks the same canonical result. It additionally proves replacement removes the prior assignment. +- The hire case that checks canonical desired skills in the approval payload and requested configuration snapshot is covered by the retained detailed hire case. That keeper checks both canonical fields and also preserves agent creation, icon, actor attribution, and source-issue linkage. + +All other declarations remain. The broader suite retains adapter-specific configuration, secret handling, merge/remove/replace semantics, beta gates, built-in role defaults, onboarding trust, approval policy, actor attribution, and rejection boundaries. Production routes, services, and their seams are unchanged. Caller review covered Agent Skills and Company Skills UI, New Agent/onboarding hire flows, and CLI `skills agent sync`. + +## Local validation and savings + +The historical serialized route/authz weight is 24,106 ms and includes suite startup, imports, database setup, and shutdown. It is not a measurement of removable runtime. The edit removes two test declarations and about 48 test lines, including separators. It removes two HTTP requests and their repeated module-reset/import setup. No production lines are removed, and no CI time reduction is claimed. + +The focused exact-head owner suite passed **58/58** in 38.38 seconds. No baseline or mutation run is claimed. A faulty canonicalizer or approval serializer should fail the retained replacement and detailed-hire keepers. The existing skills-service and authorization-service owner suites continue to cover storage and the direct permission decision. diff --git a/doc/plans/2026-10-13-codex-reconciliation-test-audit.md b/doc/plans/2026-10-13-codex-reconciliation-test-audit.md new file mode 100644 index 00000000000..1a4a092e873 --- /dev/null +++ b/doc/plans/2026-10-13-codex-reconciliation-test-audit.md @@ -0,0 +1,123 @@ +# Codex crash reconciliation test audit + +Base: `c1ae082a7a49c239a22e5149e223a41325b0ba2d` (CLI ZIP content uploaded as PR #16118). + +Remove only `requires reconciliation for a classified Codex harness crash`, +`server/src/__tests__/heartbeat-retry-scheduling.test.ts:2034–2059`. +Saving: **26 test lines and one registered execution**. No runtime reduction is +claimed. Keeper: `requires reconciliation for an error-code-only Codex harness +crash`, originally lines 2061–2086 (candidate 2035–2060). + +## Static preservation evidence + +The full fixture helper at lines 186–253 writes the classification only into +`heartbeatRuns.resultJson.errorFamily`. Both cases then update that run with +`resultJson: null` before `heartbeat.scheduleBoundedRetry`. Both persist the same +Codex adapter, active agent, failed status, harness-crash error code, zero retry +attempts, context shape and company defaults. They call with the same random +callback and identical relative `now`/created/updated/finished timestamps. The +remaining absolute 12:00 versus 13:00 difference enters no clock comparison with +another fixture; no retry deadline, active holder or evidence receipt is seeded. + +Both have one identical literal disposition/code assertion: +`{ outcome: "not_scheduled", errorCode: "legacy_execution_requires_reconciliation" }`. +No unique assertion needs transfer. The retained error-code-only keeper correctly +names the effective input. The classified title no longer reflects classification +coverage after its result JSON is erased. + +Production: the exported heartbeat facade delegates `scheduleBoundedRetry` to +`server/src/services/heartbeat/retries.ts:1835–1850`, which loads the persisted +run with full result JSON and enters `scheduleBoundedRetryForRun`. The real +reconciliation guard is at line 501. Its classifier in +`server/src/services/legacy-execution-recovery.ts:20–64` requires reconciliation +for failed legacy runs without safe bootstrap evidence. Its decision does not +consult error family. The subsequent scheduler would create an actual queued +wake and scheduled-retry row if that guard were bypassed. It does not launch the +provider during this call. + +Non-test callers include run completion (`heartbeat/run-completion.ts:859, +1412`), recovery (`heartbeat/recovery.ts:405`), scheduling +(`heartbeat/scheduling.ts:668`) and the heartbeat facade. The existing general +server CI lane owns this test file; the recorded duration inventory lists it. +No new export, injection seam, workflow, production, telemetry or timeout change +is needed. + +History: commit `665408c6d01` (PR #10210) added two genuinely different retry +classification cases on 2026-07-24. Commit `35fdc0c66b8` (PR #13075) on 2026-09-09 +renamed both to reconciliation cases, reset both result JSON values and replaced +both original assertions with the same reconciliation expectation. That change +collapsed their original classification distinction. + +Excluded files from #16114/#16115/#16116/#16075 and the completed UUID-assignee +candidate are untouched. Recent audit receipts contain no prior retirement of +this pair. All test bytes outside the one removed body stay identical. + +## Complete proof on the exact child base + +| Complete owner/sibling phase | Passed | Failed | Skipped | +|---|---:|---:|---:| +| Baseline | 66 | 0 | 0 | +| Candidate | 65 | 0 | 0 | +| Full revert | 66 | 0 | 0 | +| Restored candidate | 65 | 0 | 0 | +| Final production restore | 65 | 0 | 0 | + +The integration owner has 52 cases before and 51 after. The unchanged extracted +retry sibling has 14 cases. All phases run serially in this isolated worktree on +exact parent `c1ae082a7a49c239a22e5149e223a41325b0ba2d`. No test execution overlaps +the ZIP proof; a process scan confirmed its test processes had exited first. + +```sh +node node_modules/vitest/vitest.mjs run --project @paperclipai/server server/src/__tests__/heartbeat-retry-scheduling.test.ts server/src/services/heartbeat/retries.test.ts --exclude '**/dist/**' --pool=forks --maxWorkers=1 +``` + +The unmutated selected keeper passes 1/1. The actual mutation replaces the one +`if (await legacyExecutionNeedsReconciliationWithEvidence(db, run))` guard at +`server/src/services/heartbeat/retries.ts:501` with +`if (false && await legacyExecutionNeedsReconciliationWithEvidence(db, run))`. +The same keeper then fails at its exact disposition/code matcher on candidate +`heartbeat-retry-scheduling.test.ts:2057:23`. The real scheduler returns an actual +scheduled successor instead of the required reconciliation result. That run +exits 1 with exactly one selected failure; the other 50 cases are filtered out +and are not claimed as complete coverage. The production file is restored +byte-for-byte before the final complete 65/65 run. + +Selected command: the integration file only with +`-t 'requires reconciliation for an error-code-only Codex harness crash'`. +The harness adds default and JSON reporters and a separate output file per phase. +Whole-file byte-slice equality proves every test byte outside the single removed +body is unchanged. No assertion transfers or production edits are committed. + +## Setup recovery and practical limits + +The first child baseline was rejected: 11 non-database cases passed and 55 +cases skipped because Postgres could not create its 56-byte SysV tracking +segment. An isolated initdb diagnostic confirmed shared-memory-ID exhaustion; +`kern.sysv.shmmni` was 32 and all 32 IDs were occupied. This was not a test failure +or disk-space failure. + +The authorized guarded recovery removed 18 stale segments. For each segment an +immediate `ipcs` read confirmed NATTCH=0 and a process scan confirmed both creator +and last-user PIDs absent. All 14 attached segments were preserved. No live +Postgres process was stopped, and no kernel limits were changed. Before/after +state and per-ID checks are saved in the proof packet's `ipc-cleanup.json`. +The exact baseline retry succeeded, followed by all accepted controls above. +The rejected setup run is preserved in `rejected-baseline-shm/`. + +This worktree uses its own frozen offline dependencies, native Postgres package +symlink hydration, and shared/SDK build prerequisites. The successful source +removal is 26 lines and one execution; no CI wall-time improvement is claimed. +`git diff --check` passes. Broad typecheck/build and unrelated suites were not run +for this narrow test deletion. CI, GitHub duplicate search and reviewer gates +remain open. Related prior audit: PR #16075; its files are excluded here. + +Model: OpenAI `gpt-6.1-sol` (GPT-6.1-Sol); context window unavailable. Tool use and +code execution supplied the source inspection and proof. + +Original test SHA-256: `92425b672e93810e0de2d21efdcb65c902b1c90926fee2a941322445172a65fe`. +Candidate test SHA-256: `1fcad08c3af36e539e7f2c62d706605f65a70c9394026b0beb4ffb8aecc7c867`. +Unchanged sibling SHA-256: `ad98a02f6112a21a8975235db6a9dec227020221bf048c8dfcbdbb55c6ce2047`. +Restored production-owner SHA-256: `9f29feb6d986f281cbfd849e0152f49cf56e18974c6df1bc801010dcce4c877a`. + +Full commands, source hashes, phase JSON/logs and exact failure stack: +`/private/tmp/codex-reconciliation-child-proof/receipt.json` and sibling files. diff --git a/doc/plans/2026-10-13-db-client-test-migration-pass-audit.md b/doc/plans/2026-10-13-db-client-test-migration-pass-audit.md new file mode 100644 index 00000000000..1084a59ee88 --- /dev/null +++ b/doc/plans/2026-10-13-db-client-test-migration-pass-audit.md @@ -0,0 +1,29 @@ +# DB client test migration-pass audit + +This test-only change removes 15 redundant initial `applyPendingMigrations(connectionString)` calls from `packages/db/src/client.test.ts`. Every test declaration and assertion stays unchanged. No production code, migration, import, or fixture helper changes. + +## Why the calls are redundant + +`createTempDatabase()` awaits `startEmbeddedPostgresTestDatabase()` before returning its connection string. The shared fixture awaits `ensurePostgresDatabase()` and `applyPendingMigrations()` before returning on both the fresh and template paths. Each removed call immediately followed `await createTempDatabase()`, with no intervening schema, migration-journal, or fixture-data mutation. It therefore inspected the same fully migrated database again. + +For this fixture's hash-based migration journal, the repeated pass enters `inspectMigrations()`, opens a one-connection DB utility pool, and loads the applied hashes. `mapHashesToMigrationFiles()` reads and SHA-256 hashes every migration SQL file. The directory contains 334 SQL files at this revision. Removing 15 such passes avoids 15 × 334 = **5,010 migration SQL file reads and hashes**, plus **15 DB utility pool passes**. These are operation counts derived from the source and migration directory, not measured seconds or a CI speedup estimate. The fixture's required migration pass remains. + +## Exact removal boundary + +The removed calls were at original lines 204, 274, 327, 371, 401, 473, 567, 633, 699, 773, 877, 1010, 1406, 1983, and 2011. The import stays used by the replay tests. + +All migration calls after journal or schema mutation remain, including the original calls at lines 191, 226, 295, 358, 433, 527, 598, 664, 735, 811, 971, 1045, 1297, 1384, 1510, 1604, 1930, 1998, and 2065. These preserve older-clone upgrades; renumbered recovery replay; inserted earlier migrations; 0044, 0046, 0047, 0048, 0050, and 0059 replay; built-in managed-resource history; 0134 inbox archives; all three 0135 repair/no-op passes; run-responsible-user repair; native persistence/status-versioning replay; provider-trace idempotence; and retired-model-profile cleanup. No replay call is removed from the board-key uniqueness test, which has no later mutation requiring migration. + +The fixture-helper tests remain unchanged, including `publishes a complete template and starts suites from a copy of it` and `runs the fresh initdb path when PAPERCLIP_TEST_POSTGRES_TEMPLATE=0`. Both assert that the returned database reports `upToDate`. + +## Verification + +```sh +node_modules/.bin/vitest run --project @paperclipai/db packages/db/src/client.test.ts packages/db/src/test-embedded-postgres.test.ts +``` + +Complete owner replay: **2 files passed, 35 tests passed, 0 failed, 0 skipped; 26.77 seconds**. Real embedded PostgreSQL executed, including the fresh and template fixture-owner tests. The first sandboxed invocation encountered `listen EPERM` on localhost and was interrupted; the successful identical command ran with local socket access. No baseline timing comparison was run. + +`git diff --check` passes. A byte-level reconstruction against HEAD confirms that the client test file retains every original line except the 15 specified call lines. It retains 19 `applyPendingMigrations(connectionString)` references and the corresponding import. + +Risk: these tests now rely directly on the fixture's existing fully-migrated return contract. The fresh/template owner tests cover that contract. Mutation and idempotence coverage remains in the same client suite. diff --git a/doc/plans/2026-10-13-execution-wrapper-test-audit-controls.json b/doc/plans/2026-10-13-execution-wrapper-test-audit-controls.json new file mode 100644 index 00000000000..74bce60b5b4 --- /dev/null +++ b/doc/plans/2026-10-13-execution-wrapper-test-audit-controls.json @@ -0,0 +1,112 @@ +{ + "auditBase": "e868e6645ce0d598d756d3458f0b3d71afd6db4a", + "historicalWeightsMs": { + "selection": 1173, + "phase6": 19600, + "finalizer": 19882 + }, + "owners": { + "runtimeMode": 27, + "evidenceClassifier": 4, + "controlPlanePort": 34 + }, + "removedAliasExecutions": 99, + "restoration": { + "owner": "server/src/services/native-runtime/paperclip-control-plane-port.ts", + "originalSha256": "621beee8a451e9cab4fc09f886bfb4d95512c1433f82b7f9cbd3ea83fd2f3d0c", + "restoredSha256": "621beee8a451e9cab4fc09f886bfb4d95512c1433f82b7f9cbd3ea83fd2f3d0c", + "restored": true + }, + "runs": { + "execution-audit-pure-baseline": { + "path": "/private/tmp/execution-audit-pure-baseline.json", + "sha256": "1b5a7f42848ccd40eabbe409e8eb6331f0f41f70c12c54c11715cc29402148fb", + "passed": 31, + "failed": 0, + "pending": 0, + "success": true + }, + "execution-audit-port-baseline": { + "path": "/private/tmp/execution-audit-port-baseline.json", + "sha256": "9976aed11c8310e5c7245565413b1b4582367b8ec072006b79e05084c6c5e3c5", + "passed": 0, + "failed": 0, + "pending": 34, + "success": false + }, + "execution-audit-port-baseline-repro": { + "path": "/private/tmp/execution-audit-port-baseline-repro.json", + "sha256": "ea16340d52952745b68ab8050e3c7406a6d61e1aaf142e1705a64066259b9e93", + "passed": 0, + "failed": 0, + "pending": 34, + "success": false + }, + "execution-audit-port-baseline-repaired": { + "path": "/private/tmp/execution-audit-port-baseline-repaired.json", + "sha256": "cb3cd844a49d88dd762630a0914817ac389fbde89370b2917b21c3d5e4a1ad73", + "passed": 34, + "failed": 0, + "pending": 0, + "success": true + }, + "execution-audit-selection-replay": { + "path": "/private/tmp/execution-audit-selection-replay.json", + "sha256": "96935cbb6530793b848728372233281d7945e0d6634ad19faa177ef518c3aefb", + "passed": 54, + "failed": 0, + "pending": 0, + "success": true + }, + "execution-audit-port-mutation": { + "path": "/private/tmp/execution-audit-port-mutation.json", + "sha256": "e9fcf577361e22373a9f07be42e53e5690290c57a630029ed78e7e9978da6cd5", + "passed": 0, + "failed": 1, + "pending": 33, + "success": false + }, + "execution-audit-port-control": { + "path": "/private/tmp/execution-audit-port-control.json", + "sha256": "59f030337cea965a4a135a62f0fe8315ed52f0bffd2ed5311040094d4517049e", + "passed": 1, + "failed": 0, + "pending": 33, + "success": true + } + }, + "deliveryBase": "770cc3be7e294d606d83599c6f6a3232d47d4c70", + "branch": "codex/prune-native-acceptance-wrappers", + "candidate": { + "jsonSha256": "d7fe27f05c8ea33682a69f336e196020502cb42ae04f4717d9c8ffa7ef18c31c", + "passed": 65, + "failed": 0, + "pending": 0 + }, + "retainedOwnerSha256": { + "server/src/services/native-runtime/runtime-mode.test.ts": "c32e6b5276fc30a78f5f91ad7c9b46438d4b08917a3d933bbb1d991ef2b4d7e1", + "server/src/services/native-runtime/evidence-classifier.test.ts": "4567ab0fa44b311d83110c1fe7a5d6051cf099131db019f1024858cc3c6e7d0c", + "server/src/services/native-runtime/paperclip-control-plane-port.test.ts": "d236a5239fd4c7e15447740b04d160a51d268d555c7fb43c42e99cca4c417985" + }, + "mutationFailure": { + "case": "revalidates every persisted native binding before open and completion", + "assertionLine": 1131, + "message": "promise resolved \"undefined\" instead of rejecting", + "selectedFailed": 1, + "filteredPending": 33 + }, + "defaultShardSelection": { + "shards": 12, + "selectedFiles": 886, + "uniqueFiles": 886, + "eachOwnerCount": 1, + "retiredAliasCount": 0, + "retainedInputAliasCount": 1 + }, + "stockShardTests": { + "passed": 29, + "failed": 0, + "skipped": 0, + "logSha256": "bd0026448f3025cc8166d9ef02e8c90252e66da45af693717ef744fdbb7e0576" + } +} diff --git a/doc/plans/2026-10-13-execution-wrapper-test-audit.md b/doc/plans/2026-10-13-execution-wrapper-test-audit.md new file mode 100644 index 00000000000..8ece9f0f00a --- /dev/null +++ b/doc/plans/2026-10-13-execution-wrapper-test-audit.md @@ -0,0 +1,48 @@ +# Execution wrapper test audit — 2026-10-13 + +Remove three import-only acceptance aliases while retaining their complete production-boundary owners. This retires **99 duplicate case registrations and nine test lines**, with **zero production behavior changes**. Complete retained-owner baselines and the final candidate each pass **65/65 cases**, with no failures or skips. + +Delivery base: `770cc3be7e294d606d83599c6f6a3232d47d4c70`, the merged root after #16188. Qualification began at `e868e6645ce0d598d756d3458f0b3d71afd6db4a`; all three owners, the mutated production owner, and affected ownership references are byte-identical between those bases. Followed the OpenClaw [test-audit skill](https://github.com/openclaw/openclaw/blob/main/.agents/skills/test-audit/SKILL.md) and its candidate-evidence and retention rules. + +## Retired aliases and retained contracts + +| Retired server acceptance alias | Complete retained owner | Repeated cases | Removed test lines | Historical CI weight | +|---|---|---:|---:|---:| +| `heartbeat-native-runner-selection.test.ts` | `services/native-runtime/runtime-mode.test.ts` | 27 | 2 | 1,173 ms | +| `native-runner-phase6.integration.test.ts` | `services/native-runtime/paperclip-control-plane-port.test.ts` | 34 | 3 | 19,600 ms | +| `native-run-finalizer.test.ts` | `services/native-runtime/paperclip-control-plane-port.test.ts` plus `services/native-runtime/evidence-classifier.test.ts` | 38 | 4 | 19,882 ms | + +Every retired executable line imports one of those test modules. There are no alias-owned assertions, fixtures, mocks, hooks, filters, configuration, or entry-point actions. All aliases originated unchanged in `25cf079ec5` (#12591, 2026-08-31), when server Vitest already included the colocated owners. The shared `src/**/*.test.ts` configuration, setup file, isolated forks, and default server walker execute the same owners. An actual current-Vitest run of runtime-mode plus its alias passes 54/54, demonstrating the duplicate registration. + +Retain every runtime-selection, compatibility, persisted binding, authorization, redaction, event/replay, result, governance, review, transactional ordering, rollback, workspace-failure, and durable-evidence assertion in the three owners. Their bytes, table rows, timers, awaits, and budgets are unchanged. Independent parent review confirmed that the six-file cleanup removes only these import aliases and redirects the affected pointers. + +Retain `native-runner-input-boundary.test.ts`: it imports a Runner package suite omitted from the root default project list. Deleting that superficially similar alias would remove root-suite input-contract coverage. Prior audit receipts and excluded adapter, native-session, recovery, workspace-runtime, UI, and E2E areas were not revisited. + +The protocol-coverage generator and generated artifact now name the actual runtime-mode owner. The standalone-thin documentation invokes the three canonical owners directly and removes retired aliases from its file inventories. These labels carry no separate executable matrix signal; no acceptance requirement, source assertion, CI route, or production seam changes. + +## Qualification and reversal + +- Baseline: runtime-mode 27/27, evidence-classifier 4/4, and the complete real-Postgres control-plane owner 34/34. Final candidate: the same three files, 65/65, zero failures/skips. +- Mutation: bypass only the persisted `run.nativeIssueId === this.#binding.issueId` comparison in `PaperclipControlPlanePort`. The retained `revalidates every persisted native binding before open and completion` fails at its rejection assertion (line 1131): the first corrupted task-binding open resolves `undefined` instead of rejecting. Setup completed; this was neither a timeout nor a harness failure. +- Reversal: restore the production owner byte for byte, then the exact selected guard passes. Its 33 excluded cases are intentional name-filter skips, not a complete-file control claim. Restoration SHA-256: `621beee8a451e9cab4fc09f886bfb4d95512c1433f82b7f9cbd3ea83fd2f3d0c`. +- Default routing: 886 unique general-server files across all 12 stock shards; each retained owner appears exactly once, retired aliases zero times, and the required package-input alias once. Stock shard-routing tests pass 29/29. +- Runner `build:typescript`, generated protocol-coverage `--check`, generator syntax check, owner byte comparisons, and `git diff --check` pass. + +Focused candidate command: + +```sh +pnpm --filter @paperclipai/server exec vitest run \ + src/services/native-runtime/runtime-mode.test.ts \ + src/services/native-runtime/evidence-classifier.test.ts \ + src/services/native-runtime/paperclip-control-plane-port.test.ts +``` + +## Cost and limits + +The **40.655 seconds** total above is historical raw suite weight from `scripts/general-server-shard-durations.json`, averaged from PR CI runs 38076844891 and 38076972444 (2026-10-10). It is not a fresh causal speed measurement or a prediction of critical-path job savings. No timing claim comes from removed line counts. The successful full port baseline took 16.472 seconds of test time (25.84-second invocation); the final three-owner candidate took 26.08 seconds total. Their scopes and host state differ, so they are not an A/B speed comparison. + +Two initial database attempts failed before any assertions because the isolated `--ignore-scripts` install omitted Postgres dylib symlinks. The dependency's standard hydration script repaired that isolated setup; subsequent real-DB baseline, mutation, reversal, and candidate runs reached their intended assertions. Read-only shared-memory inventories showed 29 then 30 allocations against a 32-entry limit. No unrelated process was inspected, stopped, or changed. These failed setup attempts are excluded from the passing counts. + +This is complete affected-owner proof. A combined execution of all retired aliases was not repeated; doing so would replay the expensive unchanged database owner. Vitest's combined `list` collection reported no-suite errors on import aliases because it reuses module registrations; collection is not used as runtime preservation proof. No complete repository typecheck, default suite, full build, Rust qualification, browser suite, or fresh remote CI pass was obtained for this local branch. Their absence is not represented as a full repository pass. Publication follows the parent's sequential stack disposition. + +[Compact controls and provenance](2026-10-13-execution-wrapper-test-audit-controls.json) retain source/run hashes, counts, and the intended mutation failure. Raw logs and run JSON remain temporary local audit evidence; digests identify them but do not make them durable payloads. diff --git a/doc/plans/2026-10-13-interaction-audience-duplicate-rows-audit.md b/doc/plans/2026-10-13-interaction-audience-duplicate-rows-audit.md new file mode 100644 index 00000000000..6203c11f7d4 --- /dev/null +++ b/doc/plans/2026-10-13-interaction-audience-duplicate-rows-audit.md @@ -0,0 +1,38 @@ +# Interaction audience duplicate-row audit — 2026-10-13 + +## Scope and decision + +Collapse the five-row `it.each` in +`server/src/services/issue-thread-interaction-resolution.test.ts` into one test. +Four expanded test invocations and six net test lines are removed. + +## Retained coverage + +Each former row only supplied an interaction-kind name to the test title. +The callback accepted no row argument. Every invocation supplied the same +`agent` actor and the same fresh `interaction()` fixture to +`evaluateIssueThreadInteractionResolverAudience`, then checked the same +`allowed: true`, `effectiveResolverPolicy: "anyone"`, and `reason: "allow_anyone"` +result. The retained test preserves that entire assertion. + +The evaluator's input contract has no interaction-kind property, and its +implementation evaluates actor identity, creator/run attribution, addressee, +resolver policy, and additional restrictions. The removed names could never +select different behavior. The fixture creates a plain object; neither the +test nor the evaluator mutates shared state or performs external operations. +This file has no per-row hooks or mocks. + +Keep the separate two-row `not_creator` test: its rows supply different creator +agent and source-run values and exercise independent exclusion conditions. +All other audience, governance, and attention assertions remain unchanged. + +## Validation and limits + +Static review confirmed identical inputs and assertions and no row-specific +side effects. Supervisor validation passed the complete owner suite: **12/12** +in 1.23 seconds. No production files changed. This is a small reduction in +redundant test invocations; no measured CI saving is claimed. + +## Authorship + +OpenAI `gpt-6.1-sol` audit worker, supervised as part of the test-removal campaign. diff --git a/doc/plans/2026-10-13-interaction-route-duplicate-boundaries-test-audit.md b/doc/plans/2026-10-13-interaction-route-duplicate-boundaries-test-audit.md new file mode 100644 index 00000000000..e9e60de6903 --- /dev/null +++ b/doc/plans/2026-10-13-interaction-route-duplicate-boundaries-test-audit.md @@ -0,0 +1,20 @@ +# Interaction route duplicate boundaries test audit — 2026-10-13 + +Status: candidate consolidation awaiting focused validation. Based on the clean PR stack at `ec4124ec76`. Owners: `issue-thread-interaction-routes.test.ts` and `interaction-resolution-cross-issue-cap-postgres.test.ts`. Production code is unchanged. + +## Removed executions and retained proof + +| Removed declaration | Retained keeper and transferred assertions | +| --- | --- | +| `allows the creator agent and creating run under anyone` | The existing five-kind `lets the creator and same run resolve $kind through anyone` matrix already uses the same creator agent, creating run, open policy, assigned issue, and successful HTTP boundary. Its service assertion now checks interaction ID, request body, and authenticated `agentId`/`runId` for every kind. | +| Four-row `fails a capped run closed on cross-issue $route with no interaction effect` | The real-routes/real-Postgres four-endpoint cap matrix checks the same 429 code, cap, count, enforcement mode and redaction, plus persisted pending interaction, no child task, no wake, and exactly the rejection activity. It now also checks actionable `Try this:` copy and rejection receipt `kind: interaction_resolution` / `allowed: false`. | +| `does not charge a same-issue resolution even past the cap` | The Postgres `leaves same-issue and board resolutions outside the counter` keeper seeds a fully spent run, resolves a same-issue interaction successfully, and verifies the observation count stays at the cap and the rejected count stays zero. | +| `leaves board resolutions outside the per-run counter` | The same Postgres keeper resolves a board request on a different issue successfully while preserving both persisted counter totals. | + +The four positive cross-issue route wiring cases remain: each of `accept`, `reject`, `respond`, and `verdicts` must dispatch its own service exactly once and write one shared observation with source/target issue IDs. Postgres currently proves positive charging through `respond`; it therefore does not replace the other positive route wiring cases. Creator/run exclusion, addressed-agent authorization, human-only policy, trust/watchdog gates, native delivery, governed actions, interaction effects, and all remaining route cases remain. + +## Savings and validation + +Four declarations and seven expanded test executions are removed. The mocked route file changes from 3,404 to 3,295 lines (68 to 64 declarations); the Postgres file gains five net lines. Test files total **104 fewer lines** (12 inserted, 116 deleted). Seven repeated route requests and their per-test reset/import setup disappear. No production lines are removed. No wall-clock or CI saving is claimed without a comparable timing run. + +Static review confirmed that the three dispatched service methods accept the asserted `(issue, interactionId, body, actor)` arguments. `git diff --check` passes. Supervisor validation passed both complete owner suites serially: route suite **79/79** in 43.41 seconds; real embedded-Postgres cap suite **7/7** in 15.63 seconds. The PostgreSQL suite ran (not skipped), including each capped endpoint and persisted no-effect checks. No mutation validation or runtime savings claim is made. diff --git a/doc/plans/2026-10-13-issue-comment-wakeup-test-audit.md b/doc/plans/2026-10-13-issue-comment-wakeup-test-audit.md new file mode 100644 index 00000000000..52a24310285 --- /dev/null +++ b/doc/plans/2026-10-13-issue-comment-wakeup-test-audit.md @@ -0,0 +1,56 @@ +# Issue update/comment wake-up test audit — 2026-10-13 + +## Scope and baseline + +Owned file: `server/src/__tests__/issue-update-comment-wakeup-routes.test.ts`. +Worktree: `/private/tmp/paperclip-test-audit-pr-stack`. +Baseline HEAD: `9b4d0cd872ca1fc897f320cb181fc060f0bd5bf1` on +`codex/test-audit-r13-db-migration-passes`. The owned file is byte-identical +at `codex/test-audit-r15-onboarding-duplicates` (`568ba003a559f6a3527c36b715bbf521648518d0`). +The coordinator authorized proceeding on the shared checkout without switching branches. + +## Candidate and retained owners + +Delete the standalone own-run top-level comment no-wake case. The retained +`keeps $method mentions as context for $scenario` matrix's POST +`assignee_comment` case has the same run-authenticated actor, running source +run, agent assignee, and in-progress status, plus an explicitly mentioned +second agent. It checks successful comment persistence and zero wake-ups; +the PATCH counterpart preserves the other route boundary. + +Delete the standalone plain-text agent-name human-owned comment no-wake case. +The matrix's POST/PATCH `human_owned` cases preserve the null agent assignee, +board user owner, in-progress status, successful comment persistence, and zero +wake-ups with a stronger explicit agent mention. The route no longer calls +`findMentionedAgents`; the plain-text fixture cannot select a distinct +mention-dispatch branch. The matrix also checks updates stay on the original issue. + +No other assertions or fixtures are removed. Retained boundaries include +POST/PATCH inherited pause rejection before mutations; done/cancelled assignment +suppression; reopening with/without reassignment; normal/external assignment and +comment wake context; shared transaction identity and rollback; interrupt agent +handoff versus human handoff; structured-goal deferral and invalid status change; +PATCH comment-only wakes; Slack session preservation across reopen; explicit resume +from a completed run; review-path consumption; self/board/human/unassigned/blocked/ +done/closing mention scenarios. Response-before-wake waits remain unchanged. + +## Exact reduction + +- Test declarations: 17 → 15. +- Expanded cases: 36 → 34 (the 15-case mention matrix stays intact). +- Test file lines: 898 → 841. +- Test diff: 0 additions, 57 deletions, net −57 lines. +- The prior approximate 61-line estimate was not used as a reduction target. + +## Verification + +Owner suite: **34/34 passed**, one file, 27.93 seconds (start 08:37:13). Exact PR-branch command from the repository root: +`node_modules/.bin/vitest run --project @paperclipai/server server/src/__tests__/issue-update-comment-wakeup-routes.test.ts --silent=true`. +`git diff --check` passed. Mock-backed external sync/sandbox cleanup warnings remain +non-failing; no warning-related fixture edits were made. + +Sandbox execution reached all 34 cases but +could not bind Supertest's TCP listener (`listen EPERM`); this is an environment +failure before route assertions. Rerun outside the sandbox uses the existing +installed Vitest directly because pnpm's version switch could not fetch registry +signatures. No production source, other tests, staging, commits, or branch changes. diff --git a/doc/plans/2026-10-13-issue-work-product-attribution-test-audit.md b/doc/plans/2026-10-13-issue-work-product-attribution-test-audit.md new file mode 100644 index 00000000000..658ba59a1a1 --- /dev/null +++ b/doc/plans/2026-10-13-issue-work-product-attribution-test-audit.md @@ -0,0 +1,33 @@ +# Issue work-product attribution test audit — 2026-10-13 + +## Change and remaining proof + +Remove the success-only `stores the authenticated agent run id when creating work products` case from `server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts`. It POSTed an artifact with omitted `createdByRunId` and asserted that the service received the authenticated run ID. + +The retained `adds the authenticated run diff summary to PR work products` case exercises the same route with omitted attribution. It asserts the same `createdByRunId: ownerRunId`, plus the authenticated run's diff lookup and metadata. Forged create/update run attribution and foreign-company board attribution cases also remain. + +The route's `resolveWorkProductCreatedByRunId` helper takes no work-product type. The POST route assigns `createInput.createdByRunId` unconditionally before type-specific metadata enrichment. The removed case used provider `test`, so it did not exercise the separate Paperclip artifact attachment metadata path. The service persists attribution generically across work-product types. + +## Audit scope and evidence + +Read all 83 original declarations, including the human self-handoff cases added in #16082. Before removal, the suite had 64 plain `it` declarations and 19 `it.each` declarations. After removal, it has 63 plain declarations and 19 `it.each` declarations; the 19 tables expand to 79 rows, for 142 runtime cases. Retain all other declarations: their route registrations, actor classes, recovery authority, admission, workspace defaults, or observable ordering differ. In particular, independently registered write routes must each preserve their authorization gate, even when they share a helper. + +Reviewed the owning route attribution helper and POST handler, work-product service persistence and metadata helper, sibling `work-products.test.ts`, production route mounting, CLI work-product create/update commands, board API calls, test helper and Vitest configuration, CI selection, and relevant history. The removed case originated in `86b0fe7d01` on June 5; the stronger keeper arrived in `87d05e194b` (#12717) on September 2. + +The suite already hoists its module graph (#13524). Its historical 50,945ms shard weight comes from September 12 and includes isolated process startup, imports, collection, tests, and shutdown. It is not a runtime savings estimate. CI still selects the file through the serialized route/authz matcher; PR and release workflows invoke `pnpm test:run:serialized` with shard parameters. + +## Savings + +- Test code: 16 lines removed, including its separator. +- Declarations: 83 → 82; expanded cases: 143 → 142. +- Requests: one fewer HTTP request per owner-suite run. +- Production and shared support: zero lines changed. +- Runtime savings: not measured. + +## Validation + +Focused exact-head validation passed with local bind access: `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts` reported **142/142 passing** in 10.59 seconds (start 08:18:35; finish 08:18:45). Expected warning logs are produced by cases that inject revalidation failures. The corrected run was serialized and completed successfully. No timing savings are inferred from this test duration. + +`git diff --check` passed for the changed test file; its exact source diff is zero added lines and 16 removed lines. No production mutation was performed in this change. A recommended preservation mutation is to temporarily remove the POST route's `createInput.createdByRunId = createdByRunId` assignment, require the retained PR case to fail its attribution assertion, then restore production byte-for-byte. + +Whole-repository typecheck, test, build, and PR checks remain campaign-level gates. diff --git a/doc/plans/2026-10-13-issues-service-fixture-replay-audit.md b/doc/plans/2026-10-13-issues-service-fixture-replay-audit.md new file mode 100644 index 00000000000..7f544fa19c1 --- /dev/null +++ b/doc/plans/2026-10-13-issues-service-fixture-replay-audit.md @@ -0,0 +1,31 @@ +# Issues service fixture replay audit — 2026-10-13 + +## Scope and savings + +This candidate changes only `server/src/__tests__/issues-service.test.ts`. It consolidates repeated setup while retaining the observable list, count, description-preview, dependency-readiness, and deep-ancestor contracts. Production code, database fixture ownership, cleanup hooks, and `ensureIssueRelationsTable` are unchanged. The existing `seedIssueCompany`, `seedWorkspaceProject`, and `seedCodexCoder` helpers remain intact; the candidate uses existing company seed calls and preserves the Codex coder seed in the dependency keeper. + +Against the existing stack, the test-file diff is 63 lines added and 218 removed: **155 net test lines removed**, including the reviewer-requested removal of an inline comment. The existing stack already shares seed helpers, so savings are smaller than the earlier candidate. Seven standalone test declarations are removed. All four description inputs and all three count calls remain. The 60-node ancestor chain remains; its descendants use one insert statement instead of 60. These are source and operation counts, not a measured CI speedup. + +## Removal and keeper ledger + +| Removed standalone case | Keeper and retained proof | +| --- | --- | +| `counts only unassigned issues for assigneeAgentId='null'` | `filters and counts unassigned issues with assigneeAgentId='null'` uses the assigned/unassigned fixture and explicitly asserts count `1` as well as the selected issue ID. | +| `counts UUID-assigned issues with assigneeAgentId` | `filters and counts only issues assigned to the requested UUID` explicitly asserts count `1`. Its fixture includes the requested assignee, another assignee, and an added unassigned row, preserving the removed case's unassigned exclusion while exercising another-assignee exclusion. | +| `rejects malformed assigneeAgentId filter values in count` | `rejects malformed assigneeAgentId filters in list and count` retains both independent calls and `/assigneeAgentId/i` rejection assertions. | +| `marks list descriptions as not truncated when they fit the preview limit` | `trims large list payloads and preserves description preview boundaries` retains an exact 1,200-character input and asserts length `1200` and `descriptionTruncated: false`. | +| `marks null list descriptions as not truncated` | The same preview keeper retains a null input and asserts null output and `descriptionTruncated: false`. | +| `does not let description preview truncation split multibyte characters` | The same preview keeper retains 1,199 ASCII characters followed by an em dash and a suffix. It asserts length `1200`, the final em dash, and `descriptionTruncated: true`. The original 5,000-character input and null large-payload field assertions remain. Results are selected by ID rather than relying on row order. | +| `treats blockers with no executionWorkspaceId as not subject to the workspace-finalize barrier` | `only returns dependents once every blocker is done` explicitly seeds both blocker workspace IDs as null. It retains the not-ready assertion with a remaining open blocker, then the exact dependent/assignee result after both finish, plus both blocker IDs and an explicit two-ID length check to retain the removed case’s exact-set proof. This success still fails if the workspace-finalize barrier starts requiring a workspace for unattached blockers. | + +## Deep-chain setup + +`finds the ancestor through an arbitrarily deep chain` retains all 60 descendants, each parent link, the selected leaf, and the expected ancestor ID. IDs and rows are assembled locally, then inserted together. Only redundant database round trips are removed; the traversal depth and cycle/closed-ancestor sibling cases are unchanged. The owner suite must confirm the batched self-referencing inserts against the real embedded PostgreSQL database. + +## Validation + +`git diff --check` passed. The adapted candidate passed the complete owner suite with embedded PostgreSQL enabled: **130/130 passing** in **29.51 seconds** (start 08:19:20), using `node_modules/.bin/vitest run --project @paperclipai/server server/src/__tests__/issues-service.test.ts`. This exact-head rerun validates the 60-row self-referencing batch insert after removing the inline comment. An initial sandboxed attempt could not open localhost sockets and was stopped; the completed run used permission to enable the PostgreSQL fixture. No baseline/candidate timing comparison was performed, so no runtime savings are claimed. Repository-wide checks and CI remain campaign-level gates. + +## Deferred work + +Database fixture pooling is deliberately deferred. Sharing the existing 11 database bootstraps across groups requires separate cleanup and ordering evidence, baseline/candidate timing, and a reversible experiment. This candidate changes none of that ownership. diff --git a/doc/plans/2026-10-13-onboarding-route-duplicate-test-audit.md b/doc/plans/2026-10-13-onboarding-route-duplicate-test-audit.md new file mode 100644 index 00000000000..425838e1beb --- /dev/null +++ b/doc/plans/2026-10-13-onboarding-route-duplicate-test-audit.md @@ -0,0 +1,36 @@ +# Onboarding route duplicate-test audit — 2026-10-13 + +## Scope and keeper map + +This change removes two duplicate tests from `ui/src/lib/onboarding-route.test.ts`. The production owner is `ui/src/lib/onboarding-route.ts`. No production code changes. + +| Removed test | Retained keeper and preserved proof | +| --- | --- | +| `takes no mission input, so no goal state can route around the agent step` | `resolveRouteOnboardingOptions — the agent step > opens an existing company on the agent step` uses the same path, prefix, and company array. Its exact result includes `ONBOARDING_AGENT_STEP`; the removed negative mission-step assertion follows from this full equality. | +| `never opens on the mission step, whatever the company's goals say` | `onboardingStepForCompany > opens an existing company on the agent step` calls the same zero-argument function and asserts the exact returned step. The removed negative assertion was weaker. | + +Neither removed test accepts a distinct input or protects an additional route branch. CompanyContext provider and resolver tests remain unchanged because they exercise a separate integration boundary. + +## Savings + +- Onboarding route declarations: **32 → 30**. +- Onboarding route file: **364 → 331 lines**, a net reduction of **33 test lines**, including the unused mission-step import and blank separators. +- The two focused files execute **52 → 50 cases**. No CI timing reduction is claimed. + +## Validation + +Baseline command, run from `ui/`: + +```sh +pnpm exec vitest run src/lib/onboarding-route.test.ts src/context/CompanyContext.test.tsx +``` + +Baseline result: **2 files, 52 tests passed**, in 4.48 seconds. + +Candidate command, run from `ui/`: + +```sh +node_modules/.bin/vitest run src/lib/onboarding-route.test.ts src/context/CompanyContext.test.tsx +``` + +Candidate result: **2 files, 50 tests passed**, in 4.15 seconds. The modified onboarding route suite alone also passed **30/30**. `git diff --check` passed. No runtime savings are claimed. diff --git a/doc/plans/2026-10-13-pipeline-bulk-review-test-audit.md b/doc/plans/2026-10-13-pipeline-bulk-review-test-audit.md new file mode 100644 index 00000000000..f102aaa4239 --- /dev/null +++ b/doc/plans/2026-10-13-pipeline-bulk-review-test-audit.md @@ -0,0 +1,52 @@ +# Pipeline bulk review test audit — 2026-10-13 + +## Scope and parent + +This change owns only `server/src/__tests__/pipelines-routes.test.ts` and this receipt. Its exact parent is `e09990e009b1042c9fff212e24429a732853c324`, the current head of [PR #16175](https://github.com/paperclipai/paperclip/pull/16175), on `codex/test-audit-r19-adapter-utils-upload-case`. The dedicated branch is `codex/test-audit-pipeline-bulk-fixture`. No production file, lockfile, workflow, or held audit branch is changed. Do not merge this PR. + +## Repeated fixture and retained proof + +`bulk reviews partial successes without aborting stale items` previously created 50 independent cases and transitioned each to review. Rows 0–2 then received stale versions. Row 3 requested changes. Rows 4–49 all approved an otherwise identical case. + +Keep rows 0–4. Change the fixture bound from 50 to 5 and the successful result count from 47 to 2. Preserve the HTTP 200 response, all three failures, every failure's `version_conflict` code, and the persisted request-changes stage `in_progress`. Add a persisted `done` stage assertion for row 4 after the bulk response. Thus both success paths still execute after multiple stale failures. The same actor, company, pipeline stages, versions, and complete request shape are retained. + +This removes 90 setup HTTP mutations and 45 per-item review service calls. The new approval-detail read makes the net difference 134 operations. This is an operation count, not a measured time saving. The owner retains all 19 test cases. Test source changes by four inserted and two deleted lines; it gains two net lines. + +## Historical intent and limits + +Commit `43b005b704f80aea3ce37cab8136920749c73ee6` introduced the same fixture in [PR #7903](https://github.com/paperclipai/paperclip/pull/7903). Its title and assertions describe partial success. The introduction records no stress-test requirement or timing target. Both the original schema and current `server/src/routes/pipelines-schemas.ts:134` limit bulk review to 100 items. The 50-row fixture is not that limit. + +The current route at `server/src/routes/pipelines.ts:827` processes each item independently and catches each item's error. `reviewCase` receives no batch-size input. Cases in this fixture have no parent, blocker, or linked execution relationships. Rows 5–49 differ only in unique case key/title/id and repeat row 4's approval input state. + +A cross-check of all named pipeline tests in server, CLI, UI, shared, DB-schema, and tutorial E2E code found no independent 50-item bulk-review contract. The owner has explicit event-pagination and large-history tests at lines 727 and 776. Those tests remain unchanged. The original bulk fixture gave incidental medium-batch execution coverage; the smaller fixture preserves its asserted functional proof. + +## Local validation and database blocker + +Local database tests and production mutations were deliberately not started. The host still has 31 allocated SysV shared-memory IDs against `kern.sysv.shmmni = 32`. A prior isolated initdb failed with `could not create shared memory segment: No space left on device`. One remaining slot is not clear test headroom. No global IPC resource or other owner's process was changed. + +Passed: `pnpm --config.manage-package-manager-versions=false --filter @paperclipai/server typecheck` (exit 0), including required Runner vendor and shared/plugin build dependencies. Passed: TypeScript AST/syntax validation, all 19 cases retained, all other 18 declarations AST-equivalent, and reversal of the two scalar edits plus the two added assertion lines exactly reconstructs the original test bytes. Passed: `git diff --check` and exact two-file scope review. Production source remains unchanged. The server typecheck excludes test files; the AST/syntax check separately covers this test edit. + +Local owner, sibling, full tests, mutation proofs, and causal timing remain unverified. The authorized stacked PR delegates owner/sibling runtime validation to hosted CI. Repo-wide local tests and build were not run because the request permits non-DB verification and requires avoiding local database tests. + +## Exact parent CI baseline + +[PR CI run 38411804432](https://github.com/paperclipai/paperclip/actions/runs/38411804432) tested the exact parent SHA. Its [serialized shard 4/9](https://github.com/paperclipai/paperclip/actions/runs/38411804432/job/115345896667) passed the owner with 19 tests: + +- Bulk-review case: 1,134 ms. +- Owner tests and hooks: 4,239 ms. +- Owner Vitest duration: 24.60 s. +- Serial owner interval between suite-start markers: 25.519 s. +- Shard test window from its start banner to final suite duration line: 309.323 s. +- Full job interval from GitHub check timestamps: 375 s. + +The [general server shard 12/12](https://github.com/paperclipai/paperclip/actions/runs/38411804432/job/115345897543) passed `pipelines-service.test.ts`, 32 tests, 4,272 ms for tests and hooks. Its grouped execution makes an isolated owner-duration comparison unavailable from that completion line alone. + +The old serialized manifest weight is 20,408 ms, sampled on September 12. The general manifest sibling weight is 13,114 ms, averaged on October 10. These weights are historical balancing inputs, not a local baseline or a causal estimate of this change. + +Capture candidate owner/shard logs from hosted CI and compare with this parent. Report tests/hooks, isolated owner duration, serial marker interval, and shard/job intervals separately. Unchanged sibling/neighbor costs can help expose runner variability. A single parent/candidate CI pair cannot establish causal speedup; transforms, imports, scheduling, runner load, and startup dominate owner wall time. The PR's Verification section and follow-up report will carry the candidate result once available. + +## Deferred mutation proof + +When local capacity permits, use the original and candidate bulk case with a clean owner/sibling run. Deliberately abort the bulk loop on the first stale item, and separately stop before the final retained approval. Require failures at the intended partial-success and approval assertions. Restore production byte-for-byte after each mutant. These mutation experiments are not claimed as completed by this PR. + +Model: OpenAI `gpt-6.1-sol`; context window unavailable; repository analysis, shell tooling, TypeScript AST inspection, and non-DB verification. diff --git a/doc/plans/2026-10-13-shared-access-null-dsn-test-audit.md b/doc/plans/2026-10-13-shared-access-null-dsn-test-audit.md new file mode 100644 index 00000000000..ec820c76679 --- /dev/null +++ b/doc/plans/2026-10-13-shared-access-null-dsn-test-audit.md @@ -0,0 +1,50 @@ +# Shared access null DSN test consolidation + +Date: 2026-10-13. Baseline commit: `9b4d0cd872ca1fc897f320cb181fc060f0bd5bf1`. + +## Scope and keeper map + +Only `packages/shared/src/validators/access.test.ts` changes for this consolidation. Retire `authSessionSchema > accepts a null sentryDsn` after moving its output assertion into `authSessionSchema > parses a session where user has a real name`. + +Both cases used exactly the same input: + +```ts +{ + session: { id: "s1", userId: "u1" }, + user: { id: "u1", email: "a@b.com", name: "Jane", image: null }, + sentryDsn: null, +} +``` + +The keeper retains successful parsing and the exact real name, and now also asserts `result.success && result.data.sentryDsn` is `null`. Rejection or transformation of the null DSN remains detectable through the same schema call and input. + +The missing-DSN rejection case and real-DSN acceptance/value case remain unchanged. Name/email normalization, malformed-email rejection and all four optional Sentry-environment rows remain unchanged. No production code changed; no runtime behavior changed. + +## Exact reduction + +- Retire **1 `it` declaration** and its **10-line block**, including the separating blank line. +- Add **1 assertion line** to the existing keeper: **1 added / 10 removed**, or **9 net test lines removed**. +- Access file: **185 lines before / 176 after**; **20 cases before / 19 after**. Its two `describe` declarations and its test-file isolation unit remain. +- These are case and source-line reductions. No CI time saving was measured or claimed. + +## Reproducible verification + +Commands ran from `/private/tmp/paperclip-test-audit-pr-stack`, using the root Vitest project configuration and the package-shared project. Both commands ran complete files without a test-name filter. + +Baseline: + +```sh +node_modules/.bin/vitest run --project @paperclipai/shared packages/shared/src/telemetry/client.test.ts packages/shared/src/telemetry/client-contract.test.ts packages/shared/src/validators/access.test.ts +``` + +Result: **3 files passed, 53 cases passed**, zero failed/skipped/todo; Vitest duration **214 ms** (start 08:15:17). Access contributed 20 cases. + +Candidate, after the access and telemetry consolidations: + +```sh +node_modules/.bin/vitest run --project @paperclipai/shared packages/shared/src/telemetry/client.test.ts packages/shared/src/validators/access.test.ts +``` + +Result: **2 files passed, 44 cases passed**, zero failed/skipped/todo; Vitest duration **214 ms** (start 08:15:50). Access contributes 19 surviving cases, and telemetry contributes 25. + +Both runs emitted the existing Vite native-config warning about `__dirname`; neither run failed. `git diff --check` passes. No production mutation experiment was run. Repo-wide typecheck, tests, build and CI were not run for this focused consolidation; this receipt does not claim PR readiness. diff --git a/doc/plans/2026-10-13-shared-zip-test-audit.md b/doc/plans/2026-10-13-shared-zip-test-audit.md new file mode 100644 index 00000000000..a1f69d336cb --- /dev/null +++ b/doc/plans/2026-10-13-shared-zip-test-audit.md @@ -0,0 +1,85 @@ +# ZIP codec owner consolidation + +Date: 2026-10-13. Exact local proof parent: `c1ae082a7a49c239a22e5149e223a41325b0ba2d`. + +## Integrated PR scope + +PR #16118 now includes the already-reviewed child rollup from #16119, which was +squash-merged into this branch. Its 47-file diff against #16116 also consolidates +adapter upload, DB migration, telemetry/access, server route/interaction/issue, +worktree and onboarding tests. It removes import-only native-runner aliases and +updates their generator/design ownership references. No production behavior or +exports change in this rollup. Each owner has its own dated receipt in +`doc/plans/`; this ZIP receipt documents only the original incremental ZIP proof. +Current-head CI and review cover the integrated PR; the focused historical counts +below do not stand in for integrated validation. Newer master legacy/native +adapter-selection regressions remain intact during the integration. + +## Historical incremental ZIP scope + +Proof parent is local CLI ZIP delivery commit `c1ae082a7a49c239a22e5149e223a41325b0ba2d`, the candidate for predecessor #16116. The original ZIP child changed only `packages/shared/src/portability-zip.test.ts` and this receipt. The parent already removed the CLI codec replay file and migrated nested blob classification, opaque ASCII blob and invalid archive bytes into shared keepers. + +This child adds the exact Unicode path/text to that existing archive keeper, then retires two direct conversion cases. It contains no further CLI deletion and does not alter the predecessor PR. The final shared test bytes match the separately verified combined candidate based on #16075, but the controls below were rerun on this exact child parent. + +## Candidate + +Retire these shared direct-helper cases: + +- `keeps blobs/ entries as base64 octet streams regardless of extension`. +- `decodes valid UTF-8 entries to text and falls back to base64 for invalid bytes`. + +Their primary keeper becomes the existing shared `round-trips STORE, DEFLATE, and base64 blob entries byte-exactly and strips the shared root` case. The archive keeper calls the real conversion function through the real reader. It now contains: + +- Opaque blob bytes `[0x68, 0x69]`, which must become base64 `aGk=` despite being valid UTF8. This is stronger classification evidence than the removed invalid-byte fixture, which could produce base64 through the decoder fallback after classification broke. +- The exact retired Unicode input, path and expected output: `tasks/pap-1/TASK.md` and `# Notes\n\ncafé ✅\n`. +- Invalid non-blob bytes `[0x68, 0x69, 0xff, 0xfe, 0xc0]`, expected as base64 `aGn//sA=` at `notes/raw`. + +The shared classifier also retains the CLI's unique nested-path negative, `blobs/nested/file`, in addition to its original positive and negative paths. Existing STORE, DEFLATE, root stripping and all ZIP security/default/limit cases remain. CLI filesystem filtering and transfer tests remain. + +## Reproducible proof + +Incremental test diff: 2 added, 23 removed, **21 net test lines removed**. Two registered shared conversion cases are retired. Shared cases go from 15 to 13. Production reduction is zero. `git diff --check` passes. + +| Complete-file positive phase | Shared owner | +| --- | --- | +| Untouched parent baseline | 15/15 | +| Incremental candidate | 13/13 | +| Shared test file fully reverted | 15/15 | +| Restored incremental candidate | 13/13 | +| After production mutations restored | 13/13 | + +All complete-file positive controls have zero failed, skipped/pending and todo tests. Runs were serial. No active Vitest or Playwright process remained when the supervisor released the test slot. No baseline failures were hidden or disabled. + +From a dependency-ready checkout, run the complete owner command for each baseline/candidate/revert/restored phase: + +```sh +node node_modules/vitest/vitest.mjs run --no-file-parallelism --maxWorkers=1 \ + packages/shared/src/portability-zip.test.ts +``` + +CLI transport was not rerun for this incremental child. This child does not touch transport files or production code. The separate combined proof on #16075 recorded 18/18 transport cases before, with and after the same final shared test bytes. Those results are related evidence, not exact-child test results. + +## Production falsification and restoration + +All four mutations change `packages/shared/src/portability-zip.ts` and are restored byte-for-byte separately. Focused negative controls each select one case, fail that case and skip 12 unrelated cases. Failures reach the selected keeper assertion. The same final test and production bytes also produced the detailed observation diffs in the separate combined proof. + +1. Broaden `/(^|\/)blobs\/[^/]+$/` to `/(^|\/)blobs\/.+$/`. Run the shared owner with `-t 'matches blobs/'`. `blobs/nested/file` fails: expected false, received true. +2. Change `if (isBlobStorePath(pathValue)) {` to `if (false) {`. Run with `-t 'round-trips STORE'`. Opaque blob output becomes plain `"hi"` instead of base64 `{ data: "aGk=", encoding: "base64", contentType: "application/octet-stream" }`. +3. Change archive conversion `body: bytesToPortableFileEntry(archivePath, entryBytes),` to `body: isBlobStorePath(archivePath) ? bytesToPortableFileEntry(archivePath, entryBytes) : textDecoder.decode(entryBytes),`. Run with `-t 'round-trips STORE'`. `notes/raw` becomes lossy `"hi���"` instead of base64 `aGn//sA=`. Opaque-blob behavior remains intact, so this witnesses the invalid non-blob archive contract. +4. Change `return Buffer.from(text, "utf8").equals(Buffer.from(bytes)) ? text : null;` to `return text.normalize("NFD");`. Run with `-t 'round-trips STORE'`. The Unicode text fails its exact comparison: composed `café` becomes decomposed `café`. This proves Unicode bytes remain protected after retiring the direct conversion cases. + +Final restored production SHA256 matches the original. The final complete shared owner control passes 13/13. Repo-wide typecheck, tests, build, CI and reviewer checks remain pending; no PR-ready claim is made from this focused proof. + +## Ownership, history and routing + +Production consumers remain in CLI `resolveInlineSourceFromPath`, `resolveChunkedImportZip`, folder conversion/filtering and server `readImportZipArchive`. No tested production seam is dead. #10507 introduced CLI codec tests. #10531 moved the codec to shared and left CLI re-exports. #16075 removes earlier duplicate cases; the predecessor then removes the retained CLI replay file. This incremental child consolidates only the shared conversion cases. + +CI runs shared in `general-workspaces-b` and CLI in `general-workspaces-a` through `scripts/run-vitest-stable.mjs` and `.github/workflows/pr-trusted.yml`. This child removes two shared executions. The predecessor already removed three CLI executions and one file. No measured CI time is available. + +## SHA256 guards + +- Parent shared test: `1664aca9760a18f33ce2657a4ac878a6015c8a6a43b4290dd88cd2b78663b6db`. +- Candidate shared test: `64548931fa808612f3dad4889acb5265cd9344dba8414fc853bc7ed478fc6fc7`. +- Parent and unchanged production owner: `b5860afd9ab301fb9cd44f3b3799c28d6fe6297fc891ebbe330b4b62bc53bd4e`. + +- Test patch SHA256: `b0e9fcfbc198d19d679f7a6dd0613b7752ac9e37fcd383143af1631b00890f4f`. diff --git a/doc/plans/2026-10-13-telemetry-client-contract-test-audit.md b/doc/plans/2026-10-13-telemetry-client-contract-test-audit.md new file mode 100644 index 00000000000..ad612d58c3d --- /dev/null +++ b/doc/plans/2026-10-13-telemetry-client-contract-test-audit.md @@ -0,0 +1,54 @@ +# Telemetry client contract test consolidation + +Date: 2026-10-13. Baseline commit: `9b4d0cd872ca1fc897f320cb181fc060f0bd5bf1`. + +## Scope and keeper map + +Retire only `packages/shared/src/telemetry/client-contract.test.ts` after moving its unique assertions into the complete owner suite, `packages/shared/src/telemetry/client.test.ts`. + +| Retired contract case | Surviving owner keeper and protected regression | +| --- | --- | +| `emits only allow-set keys with schemaVersion '1' on every POST` | The count-split keeper now checks every envelope key against `CONTRACT_KEYS` (`app`, `schemaVersion`, `installId`, `version`, `events`, `batchId`) and every envelope's schema version against `"1"`. Extra wire fields and wrong schema versions remain detectable. | +| `never exceeds the injected maxEventsPerBatch on any POST` | `splits more than maxEventsPerBatch events into multiple compliant POSTs` retains the injected count cap of 2 and exact `[2, 2, 1]` distribution for five queued events. Count violations and event loss remain detectable. | +| `never exceeds the injected maxBodyBytes on any POST` | `splits a chunk whose serialized bytes exceed maxBodyBytes` retains the injected 900-byte cap and checks every raw POST body. It now also requires exactly six aggregate events, protecting against loss during byte splitting. | +| `a large flush splits into N envelopes that are each count- and byte-compliant` | The count-split keeper now injects `maxBodyBytes = 4096` and checks every raw POST body against that same injected limit, alongside the count cap and exact five-event distribution. | +| `retries a 429 and re-sends the identical batchId (server idempotency)` | `retries a 429 with the same batchId and eventually succeeds` now compares the retry's events with the original events, alongside equal batch IDs and exactly two attempts. Retry event remixing remains detectable. | +| `does not retry a terminal 400 or 413` | The separate owner cases `does not retry a terminal 400` and `does not retry a terminal 413` remain unchanged. | +| `derives a different batchId for the same events under a different installId` | The same-named owner case remains unchanged with the clock frozen across clients, isolating install identity from timestamp differences. | +| `emits a batchId of at least 32 hex chars — 128-bit collision floor` | `emits a batchId of at least 32 hex chars (128-bit collision floor)` retains both the minimum length and lowercase hexadecimal format checks. | + +The remaining owner controls for event registration, dynamic events, configuration, oversized/circular events, backoff, endpoint fallback and retry-store bounds are unchanged. No production code or generated telemetry contract changed; no runtime behavior changed. The generated contract was checked first and remains byte-identical because no event names, dimensions, or schema version changed. The telemetry README now records `client.test.ts` as the durable owner for transport contract assertions. + +## Privacy review + +Independent review of this change found no production telemetry client, event, config, type, generated-contract, retention, or payload construction changes. Event names, dimensions, schema version, identity and privacy transforms, destination, opt-out behavior, and retention remain unchanged. The tests use the existing localhost endpoint and stubbed fetch; the assertions preserve the wire allow-set, schema version, count/byte limits, and retry payload boundaries. Privacy impact: none in emitted behavior. + +## Exact reduction + +- Candidate file removed: **205 lines**, **8 `it` declarations**, **3 `describe` declarations**, and one Vitest test-file isolation unit. +- Owner additions: 15 lines added and 1 replaced line removed, for 14 net owner lines added. No owner test declaration was added or removed. +- Combined telemetry test diff: **15 added / 206 removed**, or **191 net test lines removed**. +- Telemetry cases: **33 before / 25 after**. The separately consolidated access suite accounts for the other case removed from the combined focused run. +- These are case, file and source-line reductions. No CI time saving was measured or claimed. + +## Reproducible verification + +Commands ran from `/private/tmp/paperclip-test-audit-pr-stack`, using the root Vitest project configuration and the package-shared project. Both commands ran complete files without a test-name filter. + +Baseline: + +```sh +node_modules/.bin/vitest run --project @paperclipai/shared packages/shared/src/telemetry/client.test.ts packages/shared/src/telemetry/client-contract.test.ts packages/shared/src/validators/access.test.ts +``` + +Result: **3 files passed, 53 cases passed**, zero failed/skipped/todo; Vitest duration **214 ms** (start 08:15:17). + +Candidate, after both consolidations: + +```sh +node_modules/.bin/vitest run --project @paperclipai/shared packages/shared/src/telemetry/client.test.ts packages/shared/src/validators/access.test.ts +``` + +Result: **2 files passed, 44 cases passed**, zero failed/skipped/todo; Vitest duration **214 ms** (start 08:15:50). The telemetry owner has 25 surviving cases; access has 19. + +Both runs emitted the existing Vite native-config warning about `__dirname`; neither run failed. `git diff --check` passes. No production mutation experiment was run. Repo-wide typecheck, tests, build and CI were not run for this focused consolidation; this receipt does not claim PR readiness. diff --git a/doc/plans/2026-10-13-untracked-worktree-duplicate-test-audit.md b/doc/plans/2026-10-13-untracked-worktree-duplicate-test-audit.md new file mode 100644 index 00000000000..61937f5111e --- /dev/null +++ b/doc/plans/2026-10-13-untracked-worktree-duplicate-test-audit.md @@ -0,0 +1,108 @@ +# Test audit: remove duplicate untracked-worktree execution + +Date: 2026-10-13 +Discovery base: `418eb4623ae767b5c9ce28c1c3e9fcb98f63702e` (PR #16173). +Stack replay base: `4a7d8ae4fc05acbdc5d2eefaf909fd3b29ab822c`. + +This candidate removes one 21-line test and one redundant execution from +`server/src/__tests__/workspace-runtime.test.ts`. It changes zero production +lines. Complete owner/sibling proof passes 190 cases before and 189 after, +with zero failures or skips in the accepted runs. The whole owner run was +**36.124 seconds slower** after the change. No whole-file CI speedup is claimed. + +## Retired case and keeper + +Retire `does not reset a reused worktree with untracked changes` (baseline +line 1172). Keep `does not reset a reused worktree with untracked changes when +status.showUntrackedFiles=no` unchanged (candidate line 1172). + +The keeper repeats all ten original statements, in the same order, and adds +one real Git configuration call. An AST comparison preserves identifiers, +string bytes, expressions, and assertion order; it ignores source offsets and +comments. Both cases create a real repository, bare remote, clone and managed +worktree, write the same scratch bytes, advance the remote, and realize reuse. +All four receipts remain: reused rather than created, unchanged HEAD, exact +scratch bytes, and the one-commit base-drift warning. The keeper also verifies +that the guard works when Git configuration hides untracked files. + +`refreshUnstartedWorktreeToBase` always requests `--untracked-files=all` and +branches on returned dirtiness, not the configuration value. Thus the added +configuration does not select a separate production decision. Its caller is +`realizeExecutionWorkspace`, reached by heartbeat workspace preparation. +Both cases were introduced together in PR #8412. No test-only seam becomes +unused. The clean fast-forward, task-commit protection, exact-branch no-reset, +authentication and runtime lifecycle cases remain separate and unchanged. +An independent preservation review approved this exact deletion before edits. + +## Regression control + +The unmodified keeper passes. Changing only the owner's dirty condition from +`status.trim().length > 0` to the impossible `< 0` makes the same keeper fail +at its direct unchanged-HEAD assertion. The failed check reads the actual Git +HEAD; it is not a timeout or setup failure. Production bytes were restored +exactly, with SHA-256 `e45729293762be132b74f8eb0efe1e4b8fbefaf96839c28dcc1ef255fe97d56c`. +The replay parent's owner and original test hashes match the measured base. + +## Observed cost and limitations + +The accepted complete baseline passes 175 owner plus 15 scheduler cases. +The accepted complete candidate passes 174 owner plus the same 15 scheduler +cases. Owner windows are 157.794 seconds before and 193.918 seconds after. +This pair does not establish a whole-suite runtime reduction. + +Six completed runs select only the two untracked cases before deletion and the +one keeper after deletion. The order is baseline/candidate/candidate/baseline/ +baseline/candidate. All selected cases pass. Median selected execution drops +from 2.007489 seconds to 0.755868 seconds: an observed 1.251621-second reduction. +The retired case alone takes 0.958–1.129 seconds, median 1.081 seconds, in those +baseline samples (0.633 seconds in the accepted complete baseline). +Process wall time includes large variable import costs; its median drops from +19.028 to 14.482 seconds, but that difference is not attributed to this deletion. + +| Sample | Variant | Selected execution seconds | Process wall seconds | +| --- | --- | ---: | ---: | +| 1 | baseline | 2.007 | 19.028 | +| 2 | candidate | 1.249 | 14.482 | +| 3 | candidate | 0.742 | 9.412 | +| 4 | baseline | 2.291 | 14.911 | +| 5 | baseline | 1.726 | 19.374 | +| 6 | candidate | 0.756 | 15.809 | + +The first sandbox baseline was invalid because socket/process fixtures lacked +host permissions. An initial permitted candidate attempt passed 179 assertions, +failed before ten startup-reconciliation cases could run, and exposed no failed +test assertion. Its JSON did not preserve the hook error. The unchanged group +then passed all ten cases with full reporters, and the final complete candidate +passed all 189. Other concurrent work reported exhausted Postgres shared-memory +identifiers. That is consistent with setup contention, not proof of the precise +cause of this initial failure. Neither invalid run supplies accepted proof or +speedup evidence. No timeouts, retries, skips, fixtures or production code changed. + +## Reproduction and provenance + +Run from `server/` with the repository Vitest executable: + +```sh +pnpm exec vitest run src/__tests__/workspace-runtime.test.ts src/services/workspace-git-operation-scheduler.test.ts +pnpm exec vitest run src/__tests__/workspace-runtime.test.ts -t 'does not reset a reused worktree with untracked changes' +``` + +The isolated checkout uses external dependency links and candidate-local +workspace links; the shared checkout's sources were not edited. Node 26.4.0 +and Vitest 5.0.3 produced the local evidence. Broad workspace typecheck, build +and default full tests were not repeated locally for this 21-line deletion; +PR checks provide those gates. The targeted server no-emit typecheck and +`git diff --check` pass. Browser suites did not change. + +Raw reports stay in the temporary audit directory. Their SHA-256 digests are: + +| Receipt | SHA-256 | +| --- | --- | +| Accepted complete baseline | `c232815e2cda75bea004a9025cd5f8e3dbaeb110cba9821e72df20bc164337cb` | +| Accepted complete candidate | `182c030d0385e36d7f16f4ad2821124391daf16597cc7e6098f468a5d2f927d8` | +| Keeper regression control | `e182bb2e8297dc1630dfb379eaf7a8fc32714f7e5269d5e9fd17a596c0b03cff` | +| Six timing samples | `09ac6c640a67d8db20928c338f93f865fa5180b1c7fb7691a43a6c364caee3ac` | +| Original owner test | `89a5ad0dd1400880695942e049a91b18a92e0469dc6a8c2cafc5f27e5747688a` | +| Candidate owner test | `d7f09f62d92a853de3ed61a14fe941fcdc934d41d3672da0c72a3143cdef6ad2` | + +This follows the [test-audit workflow](https://github.com/openclaw/openclaw/blob/main/.agents/skills/test-audit/SKILL.md). diff --git a/doc/plans/2026-10-13-worktree-config-fixture-audit.md b/doc/plans/2026-10-13-worktree-config-fixture-audit.md new file mode 100644 index 00000000000..a09f1c2dcf3 --- /dev/null +++ b/doc/plans/2026-10-13-worktree-config-fixture-audit.md @@ -0,0 +1,55 @@ +# Worktree configuration fixture audit receipt + +Date: 2026-10-13 +Base: `origin/codex/test-audit-r17-agent-instructions-module-reuse` at `97e65c60b665123ca51941293c70b546a74e4476` (PR #16171). +Owner: `server/src/__tests__/worktree-config.test.ts`. + +## Decision and scope + +Replace five repeated configuration literals with local `buildLegacyConfig` results and explicit database/server port overrides. This is test setup de-duplication. No cases, assertions, persistence calls, file writes, or environment isolation are removed. Production files are unchanged. No runtime or CI speed improvement is claimed. + +The original expression ranges are 396–417, 713–757, 776–797, 833–877, and 923–967. They protect sibling port avoidance, duplicate-port rebalancing, explicit-port auth URL updates, and no-port public auth URL preservation. Each fixture retains backups enabled. The first three keep the helper's default auth URL, including port 3100, while their server port remains 3101. The last two retain their explicit auth URLs. `buildIsolatedConfig` is unsuitable here because it disables backups and derives the auth port from the server port. + +The transient-home expression at original lines 602–646 remains inline and byte-identical. Its path layout is intentionally different from the other fixtures. + +## Evidence + +Read the full owner suite, production `worktree-config.ts`, caller references in `config.ts`/`index.ts`, the shared Vitest setup, server Vitest configuration, stable test routing, and recent owner history. The production owner performs real config/env writes, worktree port reservations, and auth URL updates. Every owner regression remains in this suite; no replacement proof or test-only production seam is needed. + +The five baseline and candidate expressions were evaluated using the unchanged helper and representative root values. Their actual `JSON.stringify(value, null, 2) + "\n"` outputs were byte-identical in all five comparisons. Backup flags, auth URLs, server ports, and database ports were preserved. Masking only the five expressions and removing their five new local declarations left all remaining source bytes identical. SHA-256 of that remaining source: `7e7c4299674671b032b2a6dd3242992bae8b5e695202769e1213c19e0132d40f`. + +## Validation and cost + +Existing dependency directories were linked into the isolated worktree without changing shared dependency installs. Baseline finished before candidate edits. + +Command: `node_modules/.bin/vitest run --project @paperclipai/server server/src/__tests__/worktree-config.test.ts --silent=true`. + +- Baseline: 1 file passed, 16/16 tests passed, duration 1.47s. +- Candidate: 1 file passed, 16/16 tests passed, duration 1.43s. +- Serialized fixture comparisons: 5/5 byte-identical. +- Remaining-source comparison: identical, including the transient-home fixture. +- `git diff --check`: passed. +- Final diff review: no behavior change found outside the audited setup. + +One local observation per version does not establish a timing benefit. Repository-wide typecheck, test, and build were not run locally for this bounded test-only setup change; broad PR checks remain pending. No merge-readiness claim is made. + +## Counts and retained proof + +Production LOC: 0 added, 0 removed. Test LOC: 20 added, 139 removed, net 119 removed. Test file: 1111 → 992 lines. Cases: 16 → 16. The receipt is documentation, counted separately from test LOC. + +- repairs legacy repo-local worktree config and env files into an isolated instance +- disables backups in an otherwise isolated existing worktree config +- preserves an externally supplied PORT while repairing worktree config +- never rewrites a main-instance env when ambient worktree flags leak into the process +- does not persist runtime ports into a main-instance config when ambient worktree flags leak in +- does not adopt a .paperclip config whose own env does not declare a worktree +- avoids sibling worktree ports when repairing legacy configs +- serializes and persists cross-repo worktree port reservations +- ignores stale migrated env paths when the dev runner resolved the local config +- does not persist transient runtime home overrides over repo-local worktree env +- rebalances duplicate ports for already isolated worktree configs +- persists runtime-selected worktree ports back into explicit-port auth URLs +- does not rewrite no-port public auth URLs when persisting runtime-selected ports +- preserves top-level and nested config extensions while persisting runtime ports +- can update the in-memory config when auth URL already includes a port +- does not rewrite the in-memory config when auth URL has no explicit port diff --git a/packages/adapter-utils/src/command-managed-runtime.test.ts b/packages/adapter-utils/src/command-managed-runtime.test.ts index 33f933afebf..f51855f2927 100644 --- a/packages/adapter-utils/src/command-managed-runtime.test.ts +++ b/packages/adapter-utils/src/command-managed-runtime.test.ts @@ -464,15 +464,14 @@ describe("command managed runtime", () => { cleanupDirs.push(rootDir); const remotePath = path.join(rootDir, "nested", "payload.bin"); - // ~3 MB of every byte value so the test catches any non-binary-safe handling. - const payload = Buffer.alloc(3 * 1024 * 1024); + const payload = Buffer.alloc(9 * 1024 * 1024); for (let i = 0; i < payload.length; i++) payload[i] = i % 256; const { runner, calls } = makeSpawnRunner({ supportsSingleStreamStdinProgress: true }); const client = createCommandManagedRuntimeClient({ runner, commandCwd: "/", timeoutMs: 30_000 }); const progress: Array<{ done: number; total: number | null }> = []; - await withBase64StringByteLimit(4 * 1024 * 1024, async () => { + await withBase64StringByteLimit(9 * 1024 * 1024, async () => { await client.writeFile(remotePath, toArrayBuffer(payload), { onProgress: (done, total) => { progress.push({ done, total }); @@ -918,30 +917,6 @@ describe("command managed runtime", () => { expect(executed.some((s) => s.includes("SHOULD-NOT-RUN"))).toBe(false); }); - it("test_single_stream_writeFile_collapses_roundtrips_under_96MiB", async () => { - // Research A1: with single-stream enabled a ≤96 MiB write is ONE round-trip; - // without it, the chunked path is `2 + ceil(bytes / 3 MiB)`. Same payload, - // same client API — only the runner capability flag differs. - const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-single-stream-collapse-")); - cleanupDirs.push(rootDir); - const payload = Buffer.alloc(9 * 1024 * 1024, 7); // 9 MiB → chunked = 2 + 3 = 5 execs - - const single = makeSpawnRunner({ supportsSingleStreamStdinProgress: true }); - const singleClient = createCommandManagedRuntimeClient({ runner: single.runner, commandCwd: "/", timeoutMs: 30_000 }); - await singleClient.writeFile(path.join(rootDir, "single.bin"), toArrayBuffer(payload)); - expect(single.calls.length).toBe(2); - - const chunked = makeSpawnRunner({ supportsSingleStreamStdinProgress: false }); - const chunkedClient = createCommandManagedRuntimeClient({ runner: chunked.runner, commandCwd: "/", timeoutMs: 30_000 }); - await chunkedClient.writeFile(path.join(rootDir, "chunked.bin"), toArrayBuffer(payload)); - // 3 (init temp + final mv + cleanup) + ceil(9MiB / 3MiB) = 6 round-trips. - expect(chunked.calls.length).toBe(3 + Math.ceil(payload.byteLength / (3 * 1024 * 1024))); - expect(chunked.calls.length).toBeGreaterThan(single.calls.length); - - expect((await readFile(path.join(rootDir, "single.bin"))).equals(payload)).toBe(true); - expect((await readFile(path.join(rootDir, "chunked.bin"))).equals(payload)).toBe(true); - }); - it("falls back to chunked upload progress when the runner cannot report mid-stream stdin progress", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-command-write-fallback-")); cleanupDirs.push(rootDir); @@ -965,7 +940,7 @@ describe("command managed runtime", () => { // Provider-backed sandbox runners cannot surface mid-flight progress for a // single stdin RPC, so we intentionally use several large append commands. - expect(calls.length).toBeGreaterThan(2); + expect(calls.length).toBe(3 + Math.ceil(payload.byteLength / (3 * 1024 * 1024))); const stdinCalls = calls.filter((call) => call.stdin != null); expect(stdinCalls.length).toBeGreaterThan(2); expect(stdinCalls.every((call) => Buffer.byteLength(call.stdin ?? "", "utf8") <= 4.1 * 1024 * 1024)).toBe(true); diff --git a/packages/db/src/client.test.ts b/packages/db/src/client.test.ts index 5b941c12965..59af56803b3 100644 --- a/packages/db/src/client.test.ts +++ b/packages/db/src/client.test.ts @@ -201,7 +201,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { it("upgrades renumbered recovery migrations and replays their schema idempotently", async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const recoveryFiles = [ "0250_exotic_dakota_north.sql", "0251_narrow_mastermind.sql", "0252_friendly_kate_bishop.sql", "0253_real_firebrand.sql", @@ -271,7 +270,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -324,7 +322,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -368,7 +365,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -398,7 +394,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -470,7 +465,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -564,7 +558,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -630,7 +623,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -696,7 +688,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -770,7 +761,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const builtInResourcesHash = await migrationHash( "0140_built_in_managed_resources.sql", @@ -874,7 +864,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -1007,7 +996,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const repairSweepHash = await migrationHash( "0135_repair_run_responsible_user_updated_at_sweep.sql", @@ -1403,7 +1391,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const sql = postgres(connectionString, { max: 1, onnotice: () => {} }); try { @@ -1980,7 +1967,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { "replays the idempotent provider trace migration", async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const hash = await migrationHash( "0234_provider_trace_records.sql", ); @@ -2008,7 +1994,6 @@ describeEmbeddedPostgres("applyPendingMigrations", () => { "removes retired model profiles from live records and configuration revisions", async () => { const connectionString = await createTempDatabase(); - await applyPendingMigrations(connectionString); const hash = await migrationHash("0236_remove_cheap_model_profiles.sql"); const companyId = "10000000-0000-4000-8000-000000000236"; const agentId = "20000000-0000-4000-8000-000000000236"; diff --git a/packages/paperclip-runner/docs/design/standalone-thin-paperclip-adapter.md b/packages/paperclip-runner/docs/design/standalone-thin-paperclip-adapter.md index c0354eb2a63..751745afc3a 100644 --- a/packages/paperclip-runner/docs/design/standalone-thin-paperclip-adapter.md +++ b/packages/paperclip-runner/docs/design/standalone-thin-paperclip-adapter.md @@ -676,11 +676,9 @@ server/src/routes/issues.ts server/src/routes/agents.ts server/src/routes/openapi.ts server/src/__tests__/native-runner-standalone.integration.test.ts (new) -server/src/__tests__/heartbeat-native-runner-selection.test.ts (new) server/src/__tests__/heartbeat-native-runner-cancellation.test.ts (new) server/src/__tests__/heartbeat-run-event-sequencing.test.ts (new) server/src/__tests__/native-runner-input-boundary.test.ts (new) -server/src/__tests__/native-run-finalizer.test.ts (new) server/src/__tests__/native-status-arbiter-corpus.test.ts (new) server/src/__tests__/native-finalization-recovery.test.ts (new) server/src/__tests__/native-finalization-migration.test.ts (new) @@ -988,13 +986,11 @@ packages/shared/src/validators/native-finalization.ts scripts/check-runner-sdk-spec.mjs server/package.json server/src/__tests__/heartbeat-native-runner-cancellation.test.ts -server/src/__tests__/heartbeat-native-runner-selection.test.ts server/src/__tests__/heartbeat-run-event-sequencing.test.ts server/src/__tests__/legacy-finalization-regression.test.ts server/src/__tests__/native-finalization-migration.test.ts server/src/__tests__/native-finalization-recovery.test.ts server/src/__tests__/native-interaction-bridge.test.ts -server/src/__tests__/native-run-finalizer.test.ts server/src/__tests__/native-runner-input-boundary.test.ts server/src/__tests__/native-runner-standalone.integration.test.ts server/src/__tests__/native-session-resumption.test.ts @@ -1065,11 +1061,12 @@ Targeted real integration proof: ```sh pnpm --filter @paperclipai/server exec vitest run \ src/__tests__/native-runner-standalone.integration.test.ts \ - src/__tests__/heartbeat-native-runner-selection.test.ts \ + src/services/native-runtime/runtime-mode.test.ts \ src/__tests__/heartbeat-native-runner-cancellation.test.ts \ src/__tests__/heartbeat-run-event-sequencing.test.ts \ src/__tests__/native-runner-input-boundary.test.ts \ - src/__tests__/native-run-finalizer.test.ts \ + src/services/native-runtime/paperclip-control-plane-port.test.ts \ + src/services/native-runtime/evidence-classifier.test.ts \ src/__tests__/native-status-arbiter-corpus.test.ts \ src/__tests__/native-finalization-recovery.test.ts \ src/__tests__/native-finalization-migration.test.ts \ diff --git a/packages/paperclip-runner/scripts/generate-protocol-coverage.mjs b/packages/paperclip-runner/scripts/generate-protocol-coverage.mjs index ba1d7829fdc..7030b5e4bd2 100644 --- a/packages/paperclip-runner/scripts/generate-protocol-coverage.mjs +++ b/packages/paperclip-runner/scripts/generate-protocol-coverage.mjs @@ -46,7 +46,7 @@ const lifecycleRequirements = [ ["secrets", "secret isolation and network restrictions", "src/live/clean-room-server.test.ts; runner/crates/runner-core/src/durable/runner.rs"], ["transcript-accounting", "assistant transcript capture, token usage, and cost inputs", "src/live/live-session.test.ts; src/cli/eval-session.ts"], ["state-reconstruction", "fixture and post-run state reconstruction", "src/scenarios/scenario-explorer.test.ts; src/live/live-session.test.ts"], - ["paperclip-adapter-selection", "paperclip_runner is selectable without changing legacy adapter behavior", "server/src/services/native-runtime/runtime-mode.test.ts; server/src/__tests__/heartbeat-native-runner-selection.test.ts"], + ["paperclip-adapter-selection", "paperclip_runner is selectable without changing legacy adapter behavior", "server/src/services/native-runtime/runtime-mode.test.ts"], ["real-control-plane-binding", "advertised runner tools re-authorize the live company, issue, agent, and run before using real Paperclip services", "server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts; server/src/services/native-runtime/paperclip-runner-real-server.integration.test.ts"], ["shared-paperclip-prp-route", "Rust runnerd authenticates to the shared Paperclip server route and completes a real bound semantic tool call", "server/src/realtime/runner-prp-ws.test.ts; server/src/services/native-runtime/paperclip-runner-real-server.integration.test.ts"], ["real-control-plane-idempotency", "real service mutations are replay-safe and reject conflicting idempotency-key reuse", "server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts"], diff --git a/packages/paperclip-runner/spec/capability/protocol-coverage.json b/packages/paperclip-runner/spec/capability/protocol-coverage.json index 2a23747b2e1..7e6653bf525 100644 --- a/packages/paperclip-runner/spec/capability/protocol-coverage.json +++ b/packages/paperclip-runner/spec/capability/protocol-coverage.json @@ -3955,7 +3955,7 @@ { "id": "paperclip-adapter-selection", "requirement": "paperclip_runner is selectable without changing legacy adapter behavior", - "deterministicOwner": "server/src/services/native-runtime/runtime-mode.test.ts; server/src/__tests__/heartbeat-native-runner-selection.test.ts" + "deterministicOwner": "server/src/services/native-runtime/runtime-mode.test.ts" }, { "id": "real-control-plane-binding", diff --git a/packages/shared/src/portability-zip.test.ts b/packages/shared/src/portability-zip.test.ts index c9e4c1c78c1..f95df784e11 100644 --- a/packages/shared/src/portability-zip.test.ts +++ b/packages/shared/src/portability-zip.test.ts @@ -1,7 +1,6 @@ import { deflateRawSync } from "node:zlib"; import { describe, expect, it } from "vitest"; import { - bytesToPortableFileEntry, isBlobStorePath, readZipArchive, } from "./portability-zip.js"; @@ -95,28 +94,6 @@ describe("isBlobStorePath", () => { }); }); -describe("bytesToPortableFileEntry", () => { - it("keeps blobs/ entries as base64 octet streams regardless of extension", () => { - const bytes = new Uint8Array([0x00, 0x01, 0x80, 0xfe, 0xff]); - expect(bytesToPortableFileEntry("blobs/4f2d1c9a", bytes)).toEqual({ - encoding: "base64", - data: Buffer.from(bytes).toString("base64"), - contentType: "application/octet-stream", - }); - }); - - it("decodes valid UTF-8 entries to text and falls back to base64 for invalid bytes", () => { - const text = new TextEncoder().encode("# Notes\n\ncafé ✅\n"); - expect(bytesToPortableFileEntry("tasks/pap-1/TASK.md", text)).toBe("# Notes\n\ncafé ✅\n"); - const invalid = new Uint8Array([0x68, 0x69, 0xff, 0xfe, 0xc0]); - expect(bytesToPortableFileEntry("tasks/pap-1/raw", invalid)).toEqual({ - encoding: "base64", - data: Buffer.from(invalid).toString("base64"), - contentType: "application/octet-stream", - }); - }); -}); - describe("readZipArchive", () => { it("round-trips STORE, DEFLATE, and base64 blob entries byte-exactly and strips the shared root", async () => { const blobBytes = new Uint8Array([0x68, 0x69]); @@ -130,6 +107,7 @@ describe("readZipArchive", () => { { path: "COMPANY.md", bytes: new TextEncoder().encode("---\nname: Demo\n---\n"), method: 0 }, { path: "reports/weekly.md", bytes: new TextEncoder().encode(deflated), method: 8 }, { path: "blobs/4f2d1c9a", bytes: blobBytes, method: 0 }, + { path: "tasks/pap-1/TASK.md", bytes: new TextEncoder().encode("# Notes\n\ncafé ✅\n"), method: 0 }, { path: "notes/raw", bytes: invalidUtf8, method: 0 }, ], "paperclip-demo", @@ -140,6 +118,7 @@ describe("readZipArchive", () => { files: { "COMPANY.md": "---\nname: Demo\n---\n", "reports/weekly.md": deflated, + "tasks/pap-1/TASK.md": "# Notes\n\ncafé ✅\n", "blobs/4f2d1c9a": { encoding: "base64", data: Buffer.from(blobBytes).toString("base64"), diff --git a/packages/shared/src/telemetry/README.md b/packages/shared/src/telemetry/README.md index e6d0fd9d7fc..da0403c260b 100644 --- a/packages/shared/src/telemetry/README.md +++ b/packages/shared/src/telemetry/README.md @@ -174,7 +174,9 @@ For stable event work: should have a stable helper API. 7. Update tests for helper behavior, including raw pass-through for enum-like values when that is the intended boundary. -8. Update this README only when the contributor workflow, source-of-truth +8. Keep the telemetry transport contract assertions in `client.test.ts`, which + owns the emitted-envelope, retry, and payload-size boundaries. +9. Update this README only when the contributor workflow, source-of-truth pointers, or durable invariants change. Do not add an event catalog here. Before opening a pull request, verify that the emitted code, typed helpers, and diff --git a/packages/shared/src/telemetry/client-contract.test.ts b/packages/shared/src/telemetry/client-contract.test.ts deleted file mode 100644 index 523bb640188..00000000000 --- a/packages/shared/src/telemetry/client-contract.test.ts +++ /dev/null @@ -1,205 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { TelemetryClient } from "./client.js"; -import type { TelemetryConfig, TelemetryState } from "./types.js"; - -/** - * Wire-contract suite for the telemetry client. - * - * These tests assert the client honors the backend wire contract using ONLY - * relative invariants derived from injected config — never the server's literal - * threshold values. There are intentionally no literal `50` / `512` / `524288` - * / `2` assertion targets in this file: caps are injected small and every bound - * is checked against the injected config value. This keeps the client - * "compatible, not a mirror" — server numbers live in config, not test logic. - */ - -// The server envelope allow-set (`validators.mjs` ENVELOPE_FIELDS). Any extra -// top-level key would be rejected, so every emitted envelope must be a subset. -const CONTRACT_KEYS = new Set(["app", "schemaVersion", "installId", "version", "events", "batchId"]); - -const TEST_STATE: TelemetryState = { - installId: "contract-install", - salt: "contract-salt", - createdAt: "2026-01-01T00:00:00Z", - firstSeenVersion: "0.0.0", -}; - -function makeClient( - config?: Partial, - stateFactory: () => TelemetryState = () => TEST_STATE, - // 0.5 => zero jitter; deterministic backoff. - random: () => number = () => 0.5, -) { - return new TelemetryClient( - { enabled: true, endpoint: "http://localhost:9999/ingest", ...config }, - stateFactory, - "0.0.0-test", - random, - ); -} - -function sentBodies(): Array> { - return vi.mocked(fetch).mock.calls.map((call) => { - const requestInit = call[1] as RequestInit | undefined; - return JSON.parse(String(requestInit?.body ?? "{}")); - }); -} - -function sentRawBodies(): string[] { - return vi.mocked(fetch).mock.calls.map((call) => String((call[1] as RequestInit).body)); -} - -describe("telemetry client wire contract", () => { - afterEach(() => { - vi.restoreAllMocks(); - }); - - it("emits only allow-set keys with schemaVersion '1' on every POST", async () => { - vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: true })); - const maxEventsPerBatch = 3; - const client = makeClient({ maxEventsPerBatch }); - - for (let i = 0; i < 7; i++) client.trackDynamic("plugin.telemetry.evt", { i }); - await client.flush(); - - for (const body of sentBodies()) { - for (const key of Object.keys(body)) { - expect(CONTRACT_KEYS.has(key)).toBe(true); - } - expect(body.schemaVersion).toBe("1"); - } - }); - - it("never exceeds the injected maxEventsPerBatch on any POST", async () => { - vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: true })); - const maxEventsPerBatch = 4; - const client = makeClient({ maxEventsPerBatch }); - - for (let i = 0; i < 10; i++) client.trackDynamic("plugin.telemetry.evt", { i }); - await client.flush(); - - const bodies = sentBodies(); - expect(bodies.length).toBeGreaterThan(1); // proves it actually chunked - for (const body of bodies) { - expect((body.events as unknown[]).length).toBeLessThanOrEqual(maxEventsPerBatch); - } - }); - - it("never exceeds the injected maxBodyBytes on any POST", async () => { - vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: true })); - const maxBodyBytes = 1500; - const client = makeClient({ maxEventsPerBatch: 1000, maxBodyBytes }); - - const blob = "y".repeat(300); - for (let i = 0; i < 12; i++) client.trackDynamic("plugin.telemetry.evt", { i, blob }); - await client.flush(); - - const raw = sentRawBodies(); - expect(raw.length).toBeGreaterThan(1); // proves byte-splitting happened - for (const body of raw) { - expect(Buffer.byteLength(body)).toBeLessThanOrEqual(maxBodyBytes); - } - }); - - it("a large flush splits into N envelopes that are each count- and byte-compliant", async () => { - vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: true })); - const maxEventsPerBatch = 5; - const maxBodyBytes = 4096; - const client = makeClient({ maxEventsPerBatch, maxBodyBytes }); - - for (let i = 0; i < 23; i++) client.trackDynamic("plugin.telemetry.evt", { i }); - await client.flush(); - - const bodies = sentBodies(); - const raw = sentRawBodies(); - const total = bodies.reduce((sum, b) => sum + (b.events as unknown[]).length, 0); - expect(total).toBe(23); // no events lost - bodies.forEach((body, idx) => { - expect((body.events as unknown[]).length).toBeLessThanOrEqual(maxEventsPerBatch); - expect(Buffer.byteLength(raw[idx] as string)).toBeLessThanOrEqual(maxBodyBytes); - }); - }); -}); - -describe("telemetry client wire contract — retry semantics", () => { - beforeEach(() => { - vi.useFakeTimers(); - }); - afterEach(() => { - vi.useRealTimers(); - vi.restoreAllMocks(); - }); - - it("retries a 429 and re-sends the identical batchId (server idempotency)", async () => { - const fetchMock = vi - .fn() - .mockResolvedValueOnce({ ok: false, status: 429 }) - .mockResolvedValue({ ok: true }); - vi.stubGlobal("fetch", fetchMock); - const client = makeClient({ - backoff: { baseDelayMs: 1_000, maxDelayMs: 30_000, maxAttempts: 5, jitterRatio: 0.25 }, - }); - - client.trackDynamic("plugin.telemetry.evt", { a: 1 }); - await client.flush(); - await vi.advanceTimersByTimeAsync(1_000); - - const bodies = sentBodies(); - expect(bodies).toHaveLength(2); - expect(bodies[0]?.batchId).toBe(bodies[1]?.batchId); - expect(bodies[1]?.events).toEqual(bodies[0]?.events); // no re-mix - client.stop(); - }); - - it("does not retry a terminal 400 or 413", async () => { - for (const status of [400, 413]) { - const fetchMock = vi.fn().mockResolvedValue({ ok: false, status }); - vi.stubGlobal("fetch", fetchMock); - const client = makeClient(); - - client.trackDynamic("plugin.telemetry.evt", { a: 1 }); - await client.flush(); - await vi.advanceTimersByTimeAsync(120_000); - - expect(fetchMock).toHaveBeenCalledTimes(1); - client.stop(); - vi.restoreAllMocks(); - } - }); -}); - -describe("telemetry client wire contract — batchId identity", () => { - afterEach(() => { - vi.restoreAllMocks(); - }); - - it("derives a different batchId for the same events under a different installId", async () => { - vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: true })); - - const a = makeClient(undefined, () => ({ ...TEST_STATE, installId: "install-a" })); - a.trackDynamic("plugin.telemetry.evt", { a: 1 }); - await a.flush(); - const idA = sentBodies().at(-1)?.batchId; - - vi.mocked(fetch).mockClear(); - - const b = makeClient(undefined, () => ({ ...TEST_STATE, installId: "install-b" })); - b.trackDynamic("plugin.telemetry.evt", { a: 1 }); - await b.flush(); - const idB = sentBodies().at(-1)?.batchId; - - expect(idA).not.toBe(idB); - }); - - it("emits a batchId of at least 32 hex chars — 128-bit collision floor", async () => { - vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: true })); - const client = makeClient(); - - client.trackDynamic("plugin.telemetry.evt", { a: 1 }); - await client.flush(); - - const id = sentBodies().at(-1)?.batchId as string; - expect(id.length).toBeGreaterThanOrEqual(32); - expect(id).toMatch(/^[0-9a-f]+$/); - }); -}); diff --git a/packages/shared/src/telemetry/client.test.ts b/packages/shared/src/telemetry/client.test.ts index 9bcd1e85761..3ba900cdab5 100644 --- a/packages/shared/src/telemetry/client.test.ts +++ b/packages/shared/src/telemetry/client.test.ts @@ -3,6 +3,8 @@ import { TelemetryClient } from "./client.js"; import { resolveTelemetryConfig } from "./config.js"; import type { TelemetryConfig, TelemetryDimensions, TelemetryState } from "./types.js"; +const CONTRACT_KEYS = new Set(["app", "schemaVersion", "installId", "version", "events", "batchId"]); + const TEST_STATE: TelemetryState = { installId: "test-install", salt: "test-salt", @@ -174,7 +176,8 @@ describe("TelemetryClient chunking (count + bytes)", () => { const fetchMock = vi.fn().mockResolvedValue({ ok: true }); vi.stubGlobal("fetch", fetchMock); const cap = 2; - const { client } = makeClient(undefined, { maxEventsPerBatch: cap }); + const maxBodyBytes = 4096; + const { client } = makeClient(undefined, { maxEventsPerBatch: cap, maxBodyBytes }); for (let i = 0; i < 5; i++) client.track("install.started", {}); await client.flush(); @@ -185,6 +188,14 @@ describe("TelemetryClient chunking (count + bytes)", () => { expect(bodies.map((b) => (b.events as unknown[]).length)).toEqual([2, 2, 1]); for (const body of bodies) { expect((body.events as unknown[]).length).toBeLessThanOrEqual(cap); + for (const key of Object.keys(body)) { + expect(CONTRACT_KEYS.has(key)).toBe(true); + } + expect(body.schemaVersion).toBe("1"); + } + for (const call of fetchMock.mock.calls) { + const body = String((call[1] as RequestInit).body); + expect(Buffer.byteLength(body)).toBeLessThanOrEqual(maxBodyBytes); } }); @@ -200,6 +211,7 @@ describe("TelemetryClient chunking (count + bytes)", () => { await client.flush(); expect(fetchMock.mock.calls.length).toBeGreaterThan(1); + expect(sentBodies().reduce((sum, body) => sum + (body.events as unknown[]).length, 0)).toBe(6); for (const call of fetchMock.mock.calls) { const body = String((call[1] as RequestInit).body); expect(Buffer.byteLength(body)).toBeLessThanOrEqual(maxBodyBytes); @@ -394,6 +406,7 @@ describe("TelemetryClient batched retry + backoff", () => { const bodies = sentBodies(); expect(bodies[0]?.batchId).toBe(bodies[1]?.batchId); // identical id on retry + expect(bodies[1]?.events).toEqual(bodies[0]?.events); client.stop(); }); diff --git a/packages/shared/src/validators/access.test.ts b/packages/shared/src/validators/access.test.ts index e2d7ec3d995..0df0e096658 100644 --- a/packages/shared/src/validators/access.test.ts +++ b/packages/shared/src/validators/access.test.ts @@ -120,6 +120,7 @@ describe("authSessionSchema", () => { }); expect(result.success).toBe(true); expect(result.success && result.data.user.name).toBe("Jane"); + expect(result.success && result.data.sentryDsn).toBe(null); }); it("parses a session where user name is null", () => { @@ -150,16 +151,6 @@ describe("authSessionSchema", () => { expect(result.success).toBe(false); }); - it("accepts a null sentryDsn", () => { - const result = authSessionSchema.safeParse({ - session: { id: "s1", userId: "u1" }, - user: { id: "u1", email: "a@b.com", name: "Jane", image: null }, - sentryDsn: null, - }); - expect(result.success).toBe(true); - expect(result.success && result.data.sentryDsn).toBe(null); - }); - it.each([undefined, null, "staging", "production"])( "preserves the optional Sentry environment (%s)", (environment) => { diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index 1efdbe6f9a5..3cc09d62e74 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -361,6 +361,7 @@ describe("agent routes adapter validation", () => { it("creates agents for dynamically registered external adapter types", async () => { const { registerServerAdapter } = await import("../adapters/index.js"); registerServerAdapter(externalAdapter); + mockAdapterPluginStore.getDisabledAdapterTypes.mockReturnValue(["some_other_adapter"]); const app = await createApp(); const res = await requestApp(app, (baseUrl) => @@ -729,21 +730,6 @@ describe("agent routes adapter validation", () => { } finally { await unregisterTestAdapter("paperclip_runner"); } }); - it("still creates an agent on an adapter that is registered and enabled", async () => { - const { registerServerAdapter } = await import("../adapters/index.js"); - registerServerAdapter(externalAdapter); - mockAdapterPluginStore.getDisabledAdapterTypes.mockReturnValue(["some_other_adapter"]); - - const app = await createApp(); - const res = await requestApp(app, (baseUrl) => - request(baseUrl) - .post("/api/companies/company-1/agents") - .send({ name: "Enabled Harness", adapterType: "external_test" }), - ); - - expect(res.status, JSON.stringify(res.body)).toBe(201); - }); - it("defaults new Codex on a qualified Linux target while the experimental flag is off", async () => { const selection = await import("../services/agent-runner-selection.js"); vi.spyOn(selection, "resolveNewAgentRunnerForCompany").mockImplementation(async (_db, _companyId, input) => diff --git a/server/src/__tests__/agent-instructions-routes.test.ts b/server/src/__tests__/agent-instructions-routes.test.ts index 4f7a65449b3..843ffe9a2b4 100644 --- a/server/src/__tests__/agent-instructions-routes.test.ts +++ b/server/src/__tests__/agent-instructions-routes.test.ts @@ -2,6 +2,7 @@ import express from "express"; import request from "supertest"; import { Readable } from "node:stream"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { hoistModuleGraph } from "./helpers/hoist-module-graph.js"; const mockAgentService = vi.hoisted(() => ({ create: vi.fn(), @@ -90,39 +91,6 @@ vi.mock("../adapters/index.js", () => ({ listAdapterModels: vi.fn(), })); -function registerModuleMocks() { - vi.doMock("../services/index.js", () => ({ - agentService: () => mockAgentService, - agentInstructionsService: () => mockAgentInstructionsService, - accessService: () => mockAccessService, - approvalService: () => ({}), - builtInAgentService: () => mockBuiltInAgentService, - companySkillService: () => ({ listRuntimeSkillEntries: vi.fn() }), - budgetService: () => ({}), - heartbeatService: () => ({}), - issueApprovalService: () => ({}), - issueService: () => ({}), - logActivity: mockLogActivity, - secretService: () => mockSecretService, - syncInstructionsBundleConfigFromFilePath: mockSyncInstructionsBundleConfigFromFilePath, - workspaceOperationService: () => ({}), - })); - - vi.doMock("../services/secrets.js", () => ({ - secretService: () => mockSecretService, - })); - - vi.doMock("../services/environments.js", () => ({ - environmentService: () => mockEnvironmentService, - })); - - vi.doMock("../adapters/index.js", () => ({ - findServerAdapter: mockFindServerAdapter, - findActiveServerAdapter: mockFindServerAdapter, - listAdapterModels: vi.fn(), - })); -} - function boardActor() { return { type: "board", @@ -133,19 +101,27 @@ function boardActor() { }; } -async function createApp(actor: Record = boardActor(), db: Record = {}) { +const routeModules = hoistModuleGraph(() => { + vi.doUnmock("../routes/agents.js"); + vi.doUnmock("../routes/authz.js"); + vi.doUnmock("../middleware/index.js"); +}, async () => { const [{ agentRoutes }, { errorHandler }] = await Promise.all([ vi.importActual("../routes/agents.js"), vi.importActual("../middleware/index.js"), ]); + return { agentRoutes, errorHandler }; +}); + +async function createApp(actor: Record = boardActor(), db: Record = {}) { const app = express(); app.use(express.json()); app.use((req, _res, next) => { (req as any).actor = actor; next(); }); - app.use("/api", agentRoutes(db as any)); - app.use(errorHandler); + app.use("/api", routeModules.value.agentRoutes(db as any)); + app.use(routeModules.value.errorHandler); return app; } @@ -211,13 +187,9 @@ function makeReflectionCoachAgent(overrides: Record = {}) { } describe("agent instructions bundle routes", () => { - beforeEach(async () => { - vi.resetModules(); - vi.doUnmock("../routes/agents.js"); - vi.doUnmock("../routes/authz.js"); - vi.doUnmock("../middleware/index.js"); - registerModuleMocks(); - vi.clearAllMocks(); + beforeEach(() => { + vi.resetAllMocks(); + mockSecretService.normalizeAdapterConfigForPersistence.mockImplementation(async (_companyId: string, config: Record) => config); mockAuthorizeInstructionRead.mockImplementation(async (_db, actor) => actor); mockInstructionRevisions.readCurrent.mockResolvedValue(null); mockInstructionRevisions.commit.mockResolvedValue({ revision: { id: "33333333-3333-4333-8333-333333333333", entryFile: "AGENTS.md", byteLength: 18 }, content: "# Updated Agent\n", changed: true, materialization: "current" }); @@ -288,13 +260,7 @@ describe("agent instructions bundle routes", () => { instructionsFilePath: "/tmp/agent-1/AGENTS.md", }, }); - // Cold route compilation belongs to fixture setup. Timing it as a request - // let a timed-out import resume under the next case's reset mocks. - await Promise.all([ - vi.importActual("../routes/agents.js"), - vi.importActual("../middleware/index.js"), - ]); - }, 60_000); + }); it("returns bundle metadata", async () => { const res = await requestApp( diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts index ddb6803e643..6e4f29ddf10 100644 --- a/server/src/__tests__/agent-skills-routes.test.ts +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -902,27 +902,6 @@ describe("agent skill routes", () => { ); }); - it("canonicalizes desired skill references before syncing", async () => { - mockAgentService.getById.mockResolvedValue(makeAgent("claude_local")); - - const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) - .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") - .send({ desiredSkills: ["paperclip"], mode: "replace" })); - - expect(res.status, JSON.stringify(res.body)).toBe(200); - expect(mockAgentService.update).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ - adapterConfig: expect.objectContaining({ - paperclipSkillSync: expect.objectContaining({ - desiredSkills: ["paperclipai/paperclip/paperclip"], - }), - }), - }), - expect.any(Object), - ); - }); - it("persists canonical desired skills when creating an agent directly", async () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/companies/company-1/agents") @@ -1286,33 +1265,6 @@ describe("agent skill routes", () => { expect(mockApprovalService.create).toHaveBeenCalledTimes(1); }); - it("includes canonical desired skills in hire approvals", async () => { - const db = createDb(true); - - const res = await request(await createApp(db)) - .post("/api/companies/company-1/agent-hires") - .send({ - name: "QA Agent", - role: "engineer", - adapterType: "claude_local", - desiredSkills: ["paperclip"], - adapterConfig: {}, - }); - - expect(res.status, JSON.stringify(res.body)).toBe(201); - expect(mockApprovalService.create).toHaveBeenCalledWith( - "company-1", - expect.objectContaining({ - payload: expect.objectContaining({ - desiredSkills: ["paperclipai/paperclip/paperclip"], - requestedConfigurationSnapshot: expect.objectContaining({ - desiredSkills: ["paperclipai/paperclip/paperclip"], - }), - }), - }), - ); - }); - it("gives a CEO hire the core paperclip skills when none are requested", async () => { const res = await request(await createApp(createDb(true))) .post("/api/companies/company-1/agent-hires") diff --git a/server/src/__tests__/heartbeat-native-runner-selection.test.ts b/server/src/__tests__/heartbeat-native-runner-selection.test.ts deleted file mode 100644 index 83247ec8b28..00000000000 --- a/server/src/__tests__/heartbeat-native-runner-selection.test.ts +++ /dev/null @@ -1,2 +0,0 @@ -// Acceptance-matrix entry point for default-off, opt-in, and eligibility rules. -import "../services/native-runtime/runtime-mode.test.js"; diff --git a/server/src/__tests__/heartbeat-retry-scheduling.test.ts b/server/src/__tests__/heartbeat-retry-scheduling.test.ts index 2da71fe3112..1bb89e34429 100644 --- a/server/src/__tests__/heartbeat-retry-scheduling.test.ts +++ b/server/src/__tests__/heartbeat-retry-scheduling.test.ts @@ -2031,32 +2031,6 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { } }); - it("requires reconciliation for a classified Codex harness crash", async () => { - const companyId = randomUUID(); - const agentId = randomUUID(); - const runId = randomUUID(); - const now = new Date("2026-07-24T12:00:00.000Z"); - - await seedRetryFixture({ - runId, - companyId, - agentId, - now, - errorCode: "codex_harness_crash", - errorFamily: "transient_upstream", - }); - - await db.update(heartbeatRuns).set({ resultJson: null }).where(eq(heartbeatRuns.id, runId)); - - const scheduled = await heartbeat.scheduleBoundedRetry(runId, { - now, - random: () => 0.5, - }); - - expect(scheduled).toMatchObject({ outcome: "not_scheduled", errorCode: "legacy_execution_requires_reconciliation" }); - - await cleanupRetryFixture(); - }); it("requires reconciliation for an error-code-only Codex harness crash", async () => { const companyId = randomUUID(); diff --git a/server/src/__tests__/interaction-resolution-cross-issue-cap-postgres.test.ts b/server/src/__tests__/interaction-resolution-cross-issue-cap-postgres.test.ts index c132c13e8f7..77ec8bc62f3 100644 --- a/server/src/__tests__/interaction-resolution-cross-issue-cap-postgres.test.ts +++ b/server/src/__tests__/interaction-resolution-cross-issue-cap-postgres.test.ts @@ -273,6 +273,7 @@ describeEmbeddedPostgres("cross-issue interaction resolution cap (routes + postg count: CROSS_ISSUE_INFLUENCE_LIMIT + 1, mode: "enforce", }); + expect(res.body.error).toContain("Try this:"); // The refusal must not leak the run's own (possibly inaccessible) source // issue or the resolver policy it passed. expect(JSON.stringify(res.body)).not.toContain(sourceIssueId); @@ -297,12 +298,16 @@ describeEmbeddedPostgres("cross-issue interaction resolution cap (routes + postg expect(wakes).toEqual([]); const resolutionActivity = await db - .select({ action: activityLog.action }) + .select({ action: activityLog.action, details: activityLog.details }) .from(activityLog) .where(and(eq(activityLog.companyId, companyId), eq(activityLog.entityId, targetIssueId))); expect(resolutionActivity.map((row) => row.action)).toEqual([ "issue.cross_issue_influence_cap_rejected", ]); + expect(resolutionActivity[0].details).toMatchObject({ + kind: "interaction_resolution", + allowed: false, + }); expect(await countInfluenceRows(companyId, runId, "issue.cross_issue_influence_observed")) .toBe(CROSS_ISSUE_INFLUENCE_LIMIT); }, diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 4afb8bad37c..45c95d9a764 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -1132,22 +1132,6 @@ describe("agent issue mutation checkout ownership", () => { ); }); - it("stores the authenticated agent run id when creating work products", async () => { - const app = await createApp(ownerActor()); - - await request(app).post(`/api/issues/${issueId}/work-products`).send({ - type: "artifact", - provider: "test", - title: "Artifact", - }).expect(201); - - expect(mockWorkProductService.createForIssue).toHaveBeenCalledWith( - issueId, - companyId, - expect.objectContaining({ createdByRunId: ownerRunId }), - ); - }); - it("adds the authenticated run diff summary to PR work products", async () => { mockWorkProductService.latestRunDiffSummary.mockResolvedValue({ additions: 17, diff --git a/server/src/__tests__/issue-thread-interaction-routes.test.ts b/server/src/__tests__/issue-thread-interaction-routes.test.ts index 6e9146d7f05..a15bb19b894 100644 --- a/server/src/__tests__/issue-thread-interaction-routes.test.ts +++ b/server/src/__tests__/issue-thread-interaction-routes.test.ts @@ -2882,42 +2882,6 @@ describe("issue thread interaction routes", () => { expect(mockInteractionService.answerQuestions).not.toHaveBeenCalled(); }); - it("allows the creator agent and creating run under anyone", async () => { - mockInteractionService.getForIssue.mockResolvedValueOnce({ - id: "interaction-open", - kind: "ask_user_questions", - createdByAgentId: CREATED_AGENT_ID, - sourceRunId: RUN_9, - requestedResolverPolicy: "anyone", - effectiveResolverPolicy: "anyone", - resolverPolicyProvenance: "inherited", - effectiveResolverPolicySource: "requested", - payload: { version: 1, questions: [] }, - }); - mockIssueService.getById.mockResolvedValueOnce(createIssue({ - status: "todo", - assigneeAgentId: CREATED_AGENT_ID, - })); - const app = await createApp({ - type: "agent", - agentId: CREATED_AGENT_ID, - companyId: "company-1", - runId: RUN_9, - }); - - const res = await request(app) - .post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-open/respond") - .send({ answers: [] }); - - expect(res.status).toBe(200); - expect(mockInteractionService.answerQuestions).toHaveBeenCalledWith( - expect.anything(), - "interaction-open", - expect.anything(), - expect.objectContaining({ agentId: CREATED_AGENT_ID, runId: RUN_9 }), - ); - }); - it.each([ { kind: "suggest_tasks", @@ -2992,7 +2956,12 @@ describe("issue thread interaction routes", () => { .send(body); expect(res.status).toBe(200); - expect(mockInteractionService[service as keyof typeof mockInteractionService]).toHaveBeenCalled(); + expect(mockInteractionService[service as keyof typeof mockInteractionService]).toHaveBeenCalledWith( + expect.anything(), + `interaction-${kind}`, + body, + expect.objectContaining({ agentId: CREATED_AGENT_ID, runId: RUN_9 }), + ); }); it("enforces same-run exclusion under not_creator and requires a resolver run id", async () => { @@ -3291,48 +3260,6 @@ describe("issue thread interaction routes", () => { }); } - it.each(CROSS_ISSUE_RESOLUTION_ROUTES)( - "fails a capped run closed on cross-issue $route with no interaction effect", - async ({ route, kind, body, payload, service }) => { - stubCrossIssueResolution({ kind, payload }); - mockCrossIssueInfluence.sourceIssueId = OTHER_ISSUE_ID; - mockCrossIssueInfluence.priorCount = 20; - const app = await createApp({ - type: "agent", - agentId: ASSIGNEE_AGENT_ID, - companyId: "company-1", - runId: RUN_2, - }); - - const res = await request(app) - .post(`/api/issues/${ISSUE_ID}/interactions/interaction-cross-${kind}/${route}`) - .send(body); - - expect(res.status).toBe(429); - expect(res.body.details).toMatchObject({ - code: "cross_issue_influence_cap_exceeded", - cap: 20, - count: 21, - mode: "enforce", - }); - // The refusal names the boundary and the way forward without naming the - // run's own inaccessible source issue or the resolver policy. - expect(res.body.error).toContain("Try this:"); - expect(res.body.error).not.toContain(OTHER_ISSUE_ID); - expect(res.body.details.code).not.toContain("resolver"); - // No terminal mutation, activity receipt, continuation, or wake. - expect(mockInteractionService[service]).not.toHaveBeenCalled(); - expect(mockLogActivity).not.toHaveBeenCalled(); - expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); - expect(mockCrossIssueInfluence.inserted).toEqual([ - expect.objectContaining({ - action: "issue.cross_issue_influence_cap_rejected", - details: expect.objectContaining({ kind: "interaction_resolution", allowed: false }), - }), - ]); - }, - ); - it.each(CROSS_ISSUE_RESOLUTION_ROUTES)( "charges one shared cross-issue observation for an allowed $route", async ({ route, kind, body, payload, service }) => { @@ -3365,40 +3292,4 @@ describe("issue thread interaction routes", () => { ]); }, ); - - it("does not charge a same-issue resolution even past the cap", async () => { - stubCrossIssueResolution({ kind: "ask_user_questions", payload: { version: 1, questions: [] } }); - mockCrossIssueInfluence.sourceIssueId = ISSUE_ID; - mockCrossIssueInfluence.priorCount = 20; - const app = await createApp({ - type: "agent", - agentId: ASSIGNEE_AGENT_ID, - companyId: "company-1", - runId: RUN_2, - }); - - const res = await request(app) - .post(`/api/issues/${ISSUE_ID}/interactions/interaction-cross-ask_user_questions/respond`) - .send({ answers: [] }); - - expect(res.status).toBe(200); - expect(mockInteractionService.answerQuestions).toHaveBeenCalledTimes(1); - expect(mockCrossIssueInfluence.inserted).toEqual([]); - }); - - it("leaves board resolutions outside the per-run counter", async () => { - stubCrossIssueResolution({ kind: "ask_user_questions", payload: { version: 1, questions: [] } }); - mockCrossIssueInfluence.sourceIssueId = OTHER_ISSUE_ID; - mockCrossIssueInfluence.priorCount = 20; - const app = await createApp(); - - const res = await request(app) - .post(`/api/issues/${ISSUE_ID}/interactions/interaction-cross-ask_user_questions/respond`) - .send({ answers: [] }); - - expect(res.status).toBe(200); - expect(mockInteractionService.answerQuestions).toHaveBeenCalledTimes(1); - expect(mockDbTransaction).not.toHaveBeenCalled(); - expect(mockCrossIssueInfluence.inserted).toEqual([]); - }); }); diff --git a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts index 217b07f0692..ee6fdcada94 100644 --- a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts +++ b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts @@ -699,37 +699,6 @@ describe("issue update comment wakeups", () => { })); }); - it("does not wake the assignee for its own run-authenticated top-level comment", async () => { - const existing = makeIssue({ - assigneeAgentId: ASSIGNEE_AGENT_ID, - assigneeUserId: null, - status: "in_progress", - }); - mockIssueService.getById.mockResolvedValue(existing); - mockIssueService.addComment.mockResolvedValue({ - id: "comment-self-top-level", - issueId: existing.id, - companyId: existing.companyId, - body: "Plan ready for review.", - createdByRunId: SOURCE_RUN_ID, - }); - mockHeartbeatService.getRun.mockResolvedValue({ - id: SOURCE_RUN_ID, - companyId: existing.companyId, - agentId: ASSIGNEE_AGENT_ID, - status: "running", - }); - - const res = await request(await createApp()) - .post(`/api/issues/${existing.id}/comments`) - .set("X-Paperclip-Run-Id", SOURCE_RUN_ID) - .send({ body: "Plan ready for review." }); - - expect(res.status).toBe(201); - await new Promise((resolve) => setImmediate(resolve)); - expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); - }); - it.each((["post", "patch"] as const).flatMap((method) => ["assignee_comment", "board_comment", "human_owned", "unassigned", "blocked_parent", "done_parent", "done_comment", ...(method === "patch" ? ["closing_comment"] : [])].map((scenario) => ({ method, scenario })), ))("keeps $method mentions as context for $scenario", async ({ method, scenario }) => { @@ -869,30 +838,4 @@ describe("issue update comment wakeups", () => { ); }); - it("does not route a plain-text agent name on a human-owned issue comment", async () => { - const existing = makeIssue({ - assigneeAgentId: null, - assigneeUserId: "local-board", - status: "in_progress", - }); - mockIssueService.getById.mockResolvedValue(existing); - mockIssueService.addComment.mockResolvedValue({ - id: "comment-plain-agent-name", - issueId: existing.id, - companyId: existing.companyId, - body: "QA please take the screenshot", - }); - mockIssueService.findMentionedAgents.mockResolvedValue([]); - - const res = await request(await createApp()) - .post(`/api/issues/${existing.id}/comments`) - .send({ - body: "QA please take the screenshot", - }); - - expect(res.status).toBe(201); - await new Promise((resolve) => setImmediate(resolve)); - expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); - }); - }); diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index ec0cb82b9f1..5e4df792486 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -1293,7 +1293,7 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { expect(result.map((issue) => issue.id)).toEqual([matchedIssueId]); }); - it("treats assigneeAgentId='null' as an explicit unassigned filter", async () => { + it("filters and counts unassigned issues with assigneeAgentId='null'", async () => { const companyId = randomUUID(); const assigneeAgentId = randomUUID(); const assignedIssueId = randomUUID(); @@ -1334,14 +1334,16 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { const result = await svc.list(companyId, { assigneeAgentId: "null" }); expect(result.map((issue) => issue.id)).toEqual([unassignedIssueId]); + await expect(svc.count(companyId, { assigneeAgentId: "null" })).resolves.toBe(1); }); - it("keeps UUID assignee filtering behavior unchanged", async () => { + it("filters and counts only issues assigned to the requested UUID", async () => { const companyId = randomUUID(); const assigneeAgentId = randomUUID(); const otherAgentId = randomUUID(); const assignedIssueId = randomUUID(); const otherIssueId = randomUUID(); + const unassignedIssueId = randomUUID(); await seedIssueCompany(db, companyId); @@ -1387,13 +1389,22 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { priority: "medium", assigneeAgentId: otherAgentId, }, + { + id: unassignedIssueId, + companyId, + title: "Unassigned issue", + status: "todo", + priority: "medium", + assigneeAgentId: null, + }, ]); const result = await svc.list(companyId, { assigneeAgentId }); expect(result.map((issue) => issue.id)).toEqual([assignedIssueId]); + await expect(svc.count(companyId, { assigneeAgentId })).resolves.toBe(1); }); - it("rejects malformed assigneeAgentId filter values", async () => { + it("rejects malformed assigneeAgentId filters in list and count", async () => { const companyId = randomUUID(); await seedIssueCompany(db, companyId); await db.insert(issues).values({ @@ -1407,95 +1418,6 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { await expect( svc.list(companyId, { assigneeAgentId: "not-a-uuid" }), ).rejects.toThrow(/assigneeAgentId/i); - }); - - it("counts only unassigned issues for assigneeAgentId='null'", async () => { - const companyId = randomUUID(); - const assigneeAgentId = randomUUID(); - const assignedIssueId = randomUUID(); - const unassignedIssueId = randomUUID(); - - await seedIssueCompany(db, companyId); - - await db.insert(agents).values({ - id: assigneeAgentId, - companyId, - name: "Assignee", - role: "engineer", - status: "active", - adapterType: "codex_local", - adapterConfig: {}, - runtimeConfig: {}, - permissions: {}, - }); - - await db.insert(issues).values([ - { - id: assignedIssueId, - companyId, - title: "Assigned issue", - status: "todo", - priority: "medium", - assigneeAgentId, - }, - { - id: unassignedIssueId, - companyId, - title: "Unassigned issue", - status: "todo", - priority: "medium", - assigneeAgentId: null, - }, - ]); - - await expect(svc.count(companyId, { assigneeAgentId: "null" })).resolves.toBe(1); - }); - - it("counts UUID-assigned issues with assigneeAgentId", async () => { - const companyId = randomUUID(); - const assigneeAgentId = randomUUID(); - const assignedIssueId = randomUUID(); - const otherIssueId = randomUUID(); - - await seedIssueCompany(db, companyId); - - await db.insert(agents).values({ - id: assigneeAgentId, - companyId, - name: "Assignee", - role: "engineer", - status: "active", - adapterType: "codex_local", - adapterConfig: {}, - runtimeConfig: {}, - permissions: {}, - }); - - await db.insert(issues).values([ - { - id: assignedIssueId, - companyId, - title: "Assigned issue", - status: "todo", - priority: "medium", - assigneeAgentId, - }, - { - id: otherIssueId, - companyId, - title: "Other issue", - status: "todo", - priority: "medium", - }, - ]); - - await expect(svc.count(companyId, { assigneeAgentId })).resolves.toBe(1); - }); - - it("rejects malformed assigneeAgentId filter values in count", async () => { - const companyId = randomUUID(); - await seedIssueCompany(db, companyId); - await expect( svc.count(companyId, { assigneeAgentId: "not-a-uuid" }), ).rejects.toThrow(/assigneeAgentId/i); @@ -2796,26 +2718,45 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { expect(byId.get(unblockedId)?.blockedBy).toEqual([]); }); - it("trims list payload fields that can grow large on issue index routes", async () => { + it("trims large list payloads and preserves description preview boundaries", async () => { const companyId = randomUUID(); const issueId = randomUUID(); const longDescription = "x".repeat(5_000); + const exactPreviewId = randomUUID(); + const nullDescriptionId = randomUUID(); + const multibyteBoundaryId = randomUUID(); await seedIssueCompany(db, companyId); - await db.insert(issues).values({ - id: issueId, - companyId, - title: "Large issue", - description: longDescription, - status: "todo", - priority: "medium", - executionPolicy: { stages: Array.from({ length: 20 }, (_, index) => ({ index, kind: "review", notes: "y".repeat(400) })) }, - executionState: { history: Array.from({ length: 20 }, (_, index) => ({ index, body: "z".repeat(400) })) }, - executionWorkspaceSettings: { notes: "w".repeat(2_000) }, - }); + await db.insert(issues).values([ + { + id: issueId, + companyId, + title: "Large issue", + description: longDescription, + status: "todo", + priority: "medium", + executionPolicy: { stages: Array.from({ length: 20 }, (_, index) => ({ index, kind: "review", notes: "y".repeat(400) })) }, + executionState: { history: Array.from({ length: 20 }, (_, index) => ({ index, body: "z".repeat(400) })) }, + executionWorkspaceSettings: { notes: "w".repeat(2_000) }, + }, + { + id: exactPreviewId, companyId, title: "Exact preview issue", + description: "x".repeat(1200), status: "todo", priority: "medium", + }, + { + id: nullDescriptionId, companyId, title: "Null description issue", + description: null, status: "todo", priority: "medium", + }, + { + id: multibyteBoundaryId, companyId, title: "Multibyte boundary issue", + description: `${"x".repeat(1199)}— still valid after truncation`, + status: "todo", priority: "medium", + }, + ]); - const [result] = await svc.list(companyId); + const byId = new Map((await svc.list(companyId)).map((issue) => [issue.id, issue])); + const result = byId.get(issueId); expect(result).toBeTruthy(); expect(result?.description).toHaveLength(1200); @@ -2823,72 +2764,14 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { expect(result?.executionPolicy).toBeNull(); expect(result?.executionState).toBeNull(); expect(result?.executionWorkspaceSettings).toBeNull(); - }); - it("marks list descriptions as not truncated when they fit the preview limit", async () => { - const companyId = randomUUID(); - const issueId = randomUUID(); - const description = "x".repeat(1200); - - await seedIssueCompany(db, companyId); - - await db.insert(issues).values({ - id: issueId, - companyId, - title: "Exact preview issue", - description, - status: "todo", - priority: "medium", - }); - - const [result] = await svc.list(companyId); - - expect(result?.description).toHaveLength(1200); - expect(result?.descriptionTruncated).toBe(false); - }); - - it("marks null list descriptions as not truncated", async () => { - const companyId = randomUUID(); - const issueId = randomUUID(); - - await seedIssueCompany(db, companyId); - - await db.insert(issues).values({ - id: issueId, - companyId, - title: "Null description issue", - description: null, - status: "todo", - priority: "medium", - }); - - const [result] = await svc.list(companyId); - - expect(result?.description).toBeNull(); - expect(result?.descriptionTruncated).toBe(false); - }); - - it("does not let description preview truncation split multibyte characters", async () => { - const companyId = randomUUID(); - const issueId = randomUUID(); - const description = `${"x".repeat(1199)}— still valid after truncation`; - - await seedIssueCompany(db, companyId); - - await db.insert(issues).values({ - id: issueId, - companyId, - title: "Multibyte boundary issue", - description, - status: "todo", - priority: "medium", - }); - - const [result] = await svc.list(companyId); - - expect(result?.description).toHaveLength(1200); - expect(result?.description?.endsWith("—")).toBe(true); - expect(result?.descriptionTruncated).toBe(true); + expect(byId.get(exactPreviewId)?.description).toHaveLength(1200); + expect(byId.get(exactPreviewId)?.descriptionTruncated).toBe(false); + expect(byId.get(nullDescriptionId)?.description).toBeNull(); + expect(byId.get(nullDescriptionId)?.descriptionTruncated).toBe(false); + expect(byId.get(multibyteBoundaryId)?.description).toHaveLength(1200); + expect(byId.get(multibyteBoundaryId)?.description?.endsWith("—")).toBe(true); + expect(byId.get(multibyteBoundaryId)?.descriptionTruncated).toBe(true); }); }); @@ -2983,9 +2866,10 @@ describeEmbeddedPostgres("issueService.findOpenAncestorCreatedByAgent", () => { // Extend the chain 60 levels below the delegator-created ancestor so the // match sits far above the new child's parent. let parentId = midId; + const descendants: (typeof issues.$inferInsert)[] = []; for (let index = 0; index < 60; index += 1) { const childId = randomUUID(); - await db.insert(issues).values({ + descendants.push({ id: childId, companyId, title: `Deep child ${index}`, @@ -2998,6 +2882,8 @@ describeEmbeddedPostgres("issueService.findOpenAncestorCreatedByAgent", () => { parentId = childId; } + await db.insert(issues).values(descendants); + const found = await svc.findOpenAncestorCreatedByAgent(parentId, delegatorId); expect(found?.id).toBe(midId); }); @@ -4413,8 +4299,8 @@ describeEmbeddedPostgres("issueService blockers and dependency wake readiness", const blockerB = randomUUID(); const blockedIssueId = randomUUID(); await db.insert(issues).values([ - { id: blockerA, companyId, title: "Blocker A", status: "done", priority: "medium" }, - { id: blockerB, companyId, title: "Blocker B", status: "todo", priority: "medium" }, + { id: blockerA, companyId, title: "Blocker A", status: "done", priority: "medium", executionWorkspaceId: null }, + { id: blockerB, companyId, title: "Blocker B", status: "todo", priority: "medium", executionWorkspaceId: null }, { id: blockedIssueId, companyId, @@ -4431,13 +4317,15 @@ describeEmbeddedPostgres("issueService blockers and dependency wake readiness", await svc.update(blockerB, { status: "done" }); - await expect(svc.listWakeableBlockedDependents(blockerA)).resolves.toEqual([ + const wakeable = await svc.listWakeableBlockedDependents(blockerA); + expect(wakeable).toEqual([ expect.objectContaining({ id: blockedIssueId, assigneeAgentId, blockerIssueIds: expect.arrayContaining([blockerA, blockerB]), }), ]); + expect(wakeable[0]?.blockerIssueIds).toHaveLength(2); }); it("treats done blockers on a shared workspace as ready while a foreign issue is in-flight", async () => { @@ -4611,49 +4499,6 @@ describeEmbeddedPostgres("issueService blockers and dependency wake readiness", }); }); - it("treats blockers with no executionWorkspaceId as not subject to the workspace-finalize barrier", async () => { - const companyId = randomUUID(); - const assigneeAgentId = randomUUID(); - - await seedIssueCompany(db, companyId); - await db.insert(agents).values({ - id: assigneeAgentId, - companyId, - name: "QA", - role: "qa", - status: "active", - adapterType: "claude_local", - adapterConfig: {}, - runtimeConfig: {}, - permissions: {}, - }); - - const blockerId = randomUUID(); - const dependentId = randomUUID(); - await db.insert(issues).values([ - // Done blocker with no execution workspace ever attached (e.g. closed manually). - { id: blockerId, companyId, title: "Manual done blocker", status: "done", priority: "medium" }, - { - id: dependentId, - companyId, - title: "Dependent", - status: "blocked", - priority: "medium", - assigneeAgentId, - }, - ]); - await svc.update(dependentId, { blockedByIssueIds: [blockerId] }); - - // No executionWorkspaceId → no barrier → dependent should be wakeable. - await expect(svc.listWakeableBlockedDependents(blockerId)).resolves.toEqual([ - expect.objectContaining({ - id: dependentId, - assigneeAgentId, - blockerIssueIds: [blockerId], - }), - ]); - }); - it("reports dependency readiness for blocked issue chains", async () => { const companyId = randomUUID(); await seedIssueCompany(db, companyId); diff --git a/server/src/__tests__/native-run-finalizer.test.ts b/server/src/__tests__/native-run-finalizer.test.ts deleted file mode 100644 index d63320d1335..00000000000 --- a/server/src/__tests__/native-run-finalizer.test.ts +++ /dev/null @@ -1,4 +0,0 @@ -// Acceptance-matrix entry point for evidence, finalization, CAS, and atomic -// status/liveness effects exercised by the real database-backed port suite. -import "../services/native-runtime/paperclip-control-plane-port.test.js"; -import "../services/native-runtime/evidence-classifier.test.js"; diff --git a/server/src/__tests__/native-runner-phase6.integration.test.ts b/server/src/__tests__/native-runner-phase6.integration.test.ts deleted file mode 100644 index dca1c21f51c..00000000000 --- a/server/src/__tests__/native-runner-phase6.integration.test.ts +++ /dev/null @@ -1,3 +0,0 @@ -// Acceptance-matrix entry point. The database-backed suite remains colocated -// with the production port so its package conformance fixtures stay adjacent. -import "../services/native-runtime/paperclip-control-plane-port.test.js"; diff --git a/server/src/__tests__/pipelines-routes.test.ts b/server/src/__tests__/pipelines-routes.test.ts index 4e5a7e36a15..3f94e41c362 100644 --- a/server/src/__tests__/pipelines-routes.test.ts +++ b/server/src/__tests__/pipelines-routes.test.ts @@ -1237,7 +1237,7 @@ describeEmbeddedPostgres("pipeline routes", () => { }) .expect(201); const caseIds: string[] = []; - for (let index = 0; index < 50; index += 1) { + for (let index = 0; index < 5; index += 1) { const created = await http .post(`/api/pipelines/${pipeline.body.id}/cases`) .send({ caseKey: `bulk-${index}`, title: `Bulk ${index}` }) @@ -1258,12 +1258,14 @@ describeEmbeddedPostgres("pipeline routes", () => { }) .expect(200); - expect(bulk.body.results.filter((item: { ok: boolean }) => item.ok)).toHaveLength(47); + expect(bulk.body.results.filter((item: { ok: boolean }) => item.ok)).toHaveLength(2); const failed = bulk.body.results.filter((item: { ok: boolean }) => !item.ok); expect(failed).toHaveLength(3); expect(failed.every((item: { error: { code: string } }) => item.error.code === "version_conflict")).toBe(true); const requestChangesDetail = await http.get(`/api/cases/${caseIds[3]}`).expect(200); expect(requestChangesDetail.body.stage.key).toBe("in_progress"); + const approvalDetail = await http.get(`/api/cases/${caseIds[4]}`).expect(200); + expect(approvalDetail.body.stage.key).toBe("done"); }); it("returns conflict bodies with code, current version, and stage", async () => { diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index b3718143a16..3e193c74a16 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -1204,27 +1204,6 @@ describe("realizeExecutionWorkspace", () => { ]); }); - it("does not reset a reused worktree with untracked changes", async () => { - const { sourceRepo, remotePath, repoRoot } = await createClonedRepoWithRemote(); - - const initial = await realizeWorktreeForTest(repoRoot, null); - const initialHead = await readGit(initial.cwd, ["rev-parse", "HEAD"]); - await fs.writeFile(path.join(initial.cwd, "scratch.txt"), "uncommitted scratch\n", "utf8"); - - await advanceRemoteMaster(sourceRepo, remotePath, "auth-fix.txt"); - - const reused = await realizeWorktreeForTest(repoRoot, null); - - expect(reused.created).toBe(false); - expect(await readGit(reused.cwd, ["rev-parse", "HEAD"])).toBe(initialHead); - await expect(fs.readFile(path.join(reused.cwd, "scratch.txt"), "utf8")).resolves.toBe( - "uncommitted scratch\n", - ); - expect(reused.warnings).toEqual([ - expect.stringContaining("is behind origin/master by 1 commit"), - ]); - }); - it("does not reset a reused worktree with untracked changes when status.showUntrackedFiles=no", async () => { const { sourceRepo, remotePath, repoRoot } = await createClonedRepoWithRemote(); diff --git a/server/src/__tests__/worktree-config.test.ts b/server/src/__tests__/worktree-config.test.ts index 39ac47576d8..f57c2193a5e 100644 --- a/server/src/__tests__/worktree-config.test.ts +++ b/server/src/__tests__/worktree-config.test.ts @@ -390,29 +390,19 @@ describe("worktree config repair", () => { ].join("\n"), "utf8", ); + const siblingConfig = buildLegacyConfig(siblingInstanceRoot); await fs.writeFile( path.join(siblingInstanceRoot, "config.json"), JSON.stringify( { - ...buildLegacyConfig(siblingInstanceRoot), + ...siblingConfig, database: { - mode: "embedded-postgres", - embeddedPostgresDataDir: path.join(siblingInstanceRoot, "db"), + ...siblingConfig.database, embeddedPostgresPort: 54330, - backup: { - enabled: true, - intervalMinutes: 60, - retentionDays: 30, - dir: path.join(siblingInstanceRoot, "data", "backups"), - }, }, server: { - deploymentMode: "local_trusted", - exposure: "private", - host: "127.0.0.1", + ...siblingConfig.server, port: 3101, - allowedHostnames: [], - serveUi: true, }, }, null, @@ -707,52 +697,19 @@ describe("worktree config repair", () => { await fs.mkdir(paperclipDir, { recursive: true }); await fs.mkdir(path.dirname(siblingConfigPath), { recursive: true }); + const currentConfig = buildLegacyConfig(currentInstanceRoot); await fs.writeFile( configPath, JSON.stringify( { - ...buildLegacyConfig(currentInstanceRoot), + ...currentConfig, database: { - mode: "embedded-postgres", - embeddedPostgresDataDir: path.join(currentInstanceRoot, "db"), + ...currentConfig.database, embeddedPostgresPort: 54330, - backup: { - enabled: true, - intervalMinutes: 60, - retentionDays: 30, - dir: path.join(currentInstanceRoot, "data", "backups"), - }, - }, - logging: { - mode: "file", - logDir: path.join(currentInstanceRoot, "logs"), }, server: { - deploymentMode: "local_trusted", - exposure: "private", - host: "127.0.0.1", + ...currentConfig.server, port: 3101, - allowedHostnames: [], - serveUi: true, - }, - storage: { - provider: "local_disk", - localDisk: { - baseDir: path.join(currentInstanceRoot, "data", "storage"), - }, - s3: { - bucket: "paperclip", - region: "us-east-1", - prefix: "", - forcePathStyle: false, - }, - }, - secrets: { - provider: "local_encrypted", - strictMode: false, - localEncrypted: { - keyFilePath: path.join(currentInstanceRoot, "secrets", "master.key"), - }, }, }, null, @@ -770,29 +727,19 @@ describe("worktree config repair", () => { ].join("\n"), "utf8", ); + const siblingConfig = buildLegacyConfig(siblingInstanceRoot); await fs.writeFile( siblingConfigPath, JSON.stringify( { - ...buildLegacyConfig(siblingInstanceRoot), + ...siblingConfig, database: { - mode: "embedded-postgres", - embeddedPostgresDataDir: path.join(siblingInstanceRoot, "db"), + ...siblingConfig.database, embeddedPostgresPort: 54330, - backup: { - enabled: true, - intervalMinutes: 60, - retentionDays: 30, - dir: path.join(siblingInstanceRoot, "data", "backups"), - }, }, server: { - deploymentMode: "local_trusted", - exposure: "private", - host: "127.0.0.1", + ...siblingConfig.server, port: 3101, - allowedHostnames: [], - serveUi: true, }, }, null, @@ -827,52 +774,19 @@ describe("worktree config repair", () => { const instanceRoot = path.join(isolatedHome, "instances", "pap-878-create-a-mine-tab-in-inbox"); await fs.mkdir(paperclipDir, { recursive: true }); + const config = buildLegacyConfig(instanceRoot, "http://my-host.ts.net:3100"); await fs.writeFile( configPath, JSON.stringify( { - ...buildLegacyConfig(instanceRoot, "http://my-host.ts.net:3100"), + ...config, database: { - mode: "embedded-postgres", - embeddedPostgresDataDir: path.join(instanceRoot, "db"), + ...config.database, embeddedPostgresPort: 54331, - backup: { - enabled: true, - intervalMinutes: 60, - retentionDays: 30, - dir: path.join(instanceRoot, "data", "backups"), - }, - }, - logging: { - mode: "file", - logDir: path.join(instanceRoot, "logs"), }, server: { - deploymentMode: "local_trusted", - exposure: "private", - host: "127.0.0.1", + ...config.server, port: 3101, - allowedHostnames: [], - serveUi: true, - }, - storage: { - provider: "local_disk", - localDisk: { - baseDir: path.join(instanceRoot, "data", "storage"), - }, - s3: { - bucket: "paperclip", - region: "us-east-1", - prefix: "", - forcePathStyle: false, - }, - }, - secrets: { - provider: "local_encrypted", - strictMode: false, - localEncrypted: { - keyFilePath: path.join(instanceRoot, "secrets", "master.key"), - }, }, }, null, @@ -917,52 +831,19 @@ describe("worktree config repair", () => { const instanceRoot = path.join(isolatedHome, "instances", "pap-125-public-base-url"); await fs.mkdir(paperclipDir, { recursive: true }); + const config = buildLegacyConfig(instanceRoot, "https://paperclip.example"); await fs.writeFile( configPath, JSON.stringify( { - ...buildLegacyConfig(instanceRoot, "https://paperclip.example"), + ...config, database: { - mode: "embedded-postgres", - embeddedPostgresDataDir: path.join(instanceRoot, "db"), + ...config.database, embeddedPostgresPort: 54331, - backup: { - enabled: true, - intervalMinutes: 60, - retentionDays: 30, - dir: path.join(instanceRoot, "data", "backups"), - }, - }, - logging: { - mode: "file", - logDir: path.join(instanceRoot, "logs"), }, server: { - deploymentMode: "local_trusted", - exposure: "private", - host: "127.0.0.1", + ...config.server, port: 3101, - allowedHostnames: [], - serveUi: true, - }, - storage: { - provider: "local_disk", - localDisk: { - baseDir: path.join(instanceRoot, "data", "storage"), - }, - s3: { - bucket: "paperclip", - region: "us-east-1", - prefix: "", - forcePathStyle: false, - }, - }, - secrets: { - provider: "local_encrypted", - strictMode: false, - localEncrypted: { - keyFilePath: path.join(instanceRoot, "secrets", "master.key"), - }, }, }, null, diff --git a/server/src/services/issue-thread-interaction-resolution.test.ts b/server/src/services/issue-thread-interaction-resolution.test.ts index b8a262acf94..94b67cec33d 100644 --- a/server/src/services/issue-thread-interaction-resolution.test.ts +++ b/server/src/services/issue-thread-interaction-resolution.test.ts @@ -19,13 +19,7 @@ function interaction(overrides: Record = {}) { } describe("issue-thread interaction resolver audience", () => { - it.each([ - "suggest_tasks", - "ask_user_questions", - "request_confirmation", - "request_checkbox_confirmation", - "request_item_verdicts", - ])("allows the creator and creating run under anyone for %s", () => { + it("allows the creator and creating run under anyone", () => { expect(evaluateIssueThreadInteractionResolverAudience({ actor: agent, interaction: interaction(), diff --git a/ui/src/lib/onboarding-route.test.ts b/ui/src/lib/onboarding-route.test.ts index 4db18bf5926..2e62865e651 100644 --- a/ui/src/lib/onboarding-route.test.ts +++ b/ui/src/lib/onboarding-route.test.ts @@ -8,7 +8,6 @@ import { shouldRedirectCompanylessRouteToOnboarding, shouldRouteAgentlessCompanyToOnboarding, ONBOARDING_AGENT_STEP, - ONBOARDING_MISSION_STEP, } from "./onboarding-route"; describe("isOnboardingPath", () => { @@ -277,24 +276,6 @@ describe("resolveRouteOnboardingOptions — the agent step", () => { ).toEqual({ initialStep: ONBOARDING_AGENT_STEP, companyId: "c1" }); }); - it("takes no mission input, so no goal state can route around the agent step", () => { - // Three cases used to live here — mission present, absent, and in flight — - // and two of them routed to the mission step. The resolver no longer - // accepts the input at all, which is what collapses them into one: there is - // no value a caller could pass that reaches a different step. - // - // Asserted on the accepted keys rather than by example, because the - // property being defended is that nothing can be passed. A test that tried - // would not fail, it would not compile. - const resolved = resolveRouteOnboardingOptions({ - pathname: "/PC1/onboarding", - companyPrefix: "PC1", - companies, - }); - expect(resolved).toEqual({ initialStep: ONBOARDING_AGENT_STEP, companyId: "c1" }); - expect(resolved!.initialStep).not.toBe(ONBOARDING_MISSION_STEP); - }); - it("never resolves into the create wizard on a managed stack", () => { // POST /companies is a 403 floor on Cloud-managed stacks — a create wizard // there is a dead end wearing a form. A managed stack holds exactly one @@ -347,18 +328,4 @@ describe("onboardingStepForCompany", () => { it("opens an existing company on the agent step", () => { expect(onboardingStepForCompany()).toBe(ONBOARDING_AGENT_STEP); }); - - it("never opens on the mission step, whatever the company's goals say", () => { - // This used to branch on whether the company had a mission, and a company - // without one was sent to ask for it first. Onboarding stopped asking — the - // mission is collected later, in the tenant app — but the branch outlived - // the question, and because Cloud's naming screen had dropped its own - // mission field, *every* Cloud-created company arrived looking mission-less - // and took the detour. - // - // There is no argument left to vary, which is the point: the assertion is - // that the mission step is unreachable from here rather than merely - // unlikely. - expect(onboardingStepForCompany()).not.toBe(ONBOARDING_MISSION_STEP); - }); });