mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 05:56:19 +01:00
44a4ca1c6148a2bed33601415bd7cb0ad4e339b4
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nicky LeachandClaude Opus 5.5
|
11ac90e31f |
fix(build): typecheck server/src/__tests__ and fix every type error (#18739)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server has about 1,100 test and helper files in `server/src/__tests__` > - `server/tsconfig.json` excludes that directory, Vitest does not check types, and no script runs the tsconfig in that directory > - So a test can call a function with the wrong arguments and still pass, and no tool reports it > - This gap hid 2,168 real type errors, and some of them hid tests that did not test what they claim > - This pull request fixes all of these errors and adds the directory to the server layers from paperclipai/paperclip#18737 > - The benefit is that a type error in a server test now fails the typecheck, with no baseline and no backlog ## Linked Issues or Issue Description Refs paperclipai/paperclip#18737, which is merged and added the server layers. Refs paperclipai/paperclip#18738, which is merged and checks the test projects outside `pnpm -r`, so it is not a duplicate. Refs paperclipai/paperclip#18137, which records that this directory has no check. **What happened?** No tool checked the types of 1,074 of the 1,101 files in `server/src/__tests__`. I searched `package.json`, the workflows, and all tsconfig files. 27 files are in the program of another tsconfig, and these 27 have 0 errors. 11 of the 27 are in `helpers/maintenance/tsconfig.json`. No script ran that project when I measured this. paperclipai/paperclip#18738 now runs it. **Expected behavior** The server typecheck checks these files, and a type error fails the check. **Steps to reproduce** 1. On the base commit, run `tsc -p server/src/__tests__/tsconfig.json --noEmit`. It reports 6,536 errors and needs 12.3 GiB. 2. Add a type error to any file in `server/src/__tests__`. 3. Run `pnpm --filter @paperclipai/server typecheck`. It passes. **Paperclip version or commit** I measured the error breakdown below on commit `8dfd632452`, with TypeScript 7.0.2. The branch now includes master up to paperclipai/paperclip#18865. **Deployment mode** Not applicable. This issue applies to development and CI. No production source file in the server changes. ### Error breakdown Each row adds one change to the row above. | Step | Total errors | Change | Cause | | --- | --- | --- | --- | | Config as committed | 6,536 | | | | Add `src/types/*.d.ts` | 4,447 | -2,089 | Config. The Express request types were missing. | | Set `rootDir` to the repository root | 2,436 | -2,011 | Config. | | Set `allowImportingTsExtensions` | 2,181 | -255 | Config. Vitest accepts `.ts` imports. | | Add `ES2024.Promise` to `lib` | 2,138 | -43 | Config. Node 24 has `Promise.withResolvers`. | | Set `allowJs` for `.mjs` imports | 2,148 | +10 | Config. It removes 8 `TS7016` errors and shows new real errors. | | Set `declaration` to false | 2,142 | -6 | Config. Tests emit nothing. | | Count only files in `server/src` | 2,058 | -84 | Other packages: 53 Daytona, 29 Runner test, 2 UI. | So 4,478 of the 6,536 errors (69%) came from config or scope, and 2,058 are real type errors in 314 files. The 29 Runner test errors are also real, which gives 2,087 errors. Tests that master added after this measurement had 81 more errors, so this pull request fixes 2,168 errors. This table classifies the first 2,058 errors. | Kind of real error | Errors | | --- | --- | | Wrong argument or assignment type | 1,023 | | Stale or partial fixture objects | 525 | | Property that the type does not have | 317 | | Missing narrowing of null or unknown | 101 | | Implicit any | 60 | | Missing name or module | 16 | | Other | 16 | ### What the type errors hid Most errors were stale fixtures and loose mocks. These errors hid real test problems, and this pull request fixes them: - **A test that never ran.** `instrumentation.test.ts` required `sdk-trace-base`, which is not installed. So 3 span tests were skipped. They now run and pass. - **Assertions that could not fail.** - `issue-thread-interactions-service` used a company ID as an issue ID. - `status-card-evaluation-work` compared `[]` with `[]` in a rollback check. - `slack-conversation-lifecycle` read `item.issueId`, which does not exist. - `email-channels` used a permission key that does not exist. - `heartbeat-project-env` imported `LOW_TRUST_REVIEW_PRESET`, which is not exported. The value was undefined. - **Postgres processes that did not stop.** 11 test files called `stop` on the test database helper. That method does not exist, and the helper method is `cleanup`. - **Arguments that the service ignored.** - `methodKey` is now `connectionMethodKey` in the Daytona, Unikraft, 1Password, and Railway tests. - 108 calls passed `"owner-user"` to a parameter that expects an options object. - 9 chat wizard fixtures passed `managementContext` as `source`, so they ran `existing_app` and not `manifest`. - 2 tests passed a `{ userId }` actor that the function ignores. - Inserts into `environments` passed a `companyId` column that the table does not have. - **Problems in tests that master added during this work.** - `sandbox-creation-operations` read `database.connectionString` when an external test database was set. In that mode `database` is undefined, so the fixture would stop with a TypeError. It now uses the external URL. - `idle-completed-history` set a Dot operation to `dispatched`. That is a Muse status, and the Dot type does not have it. The fixture now uses `admitted`, which is a real in-flight Dot status. ### Possible product problems, not changed These may be real problems in production code or types. I did not change them, because this pull request does not change production behavior: - `decisions-service` `service.create` does not validate the lenient `cancel_issue_tree` input. - `createConnection` ignores `input.connectionPurpose`. - The `createConnection` route schema default stores `{}` for `transportConfig`. - `prepareManagedAiRuntime` accepted the invalid value `mode: "connection"`. - `createBinding` accepts invalid `projectionClass` strings outside class 3. - `ensureManagedProjectWorkspace` forwards an untyped `repoRef` by spread. - `routineVariableSchema.label` is optional, but `RoutineVariable.label` is required. - `ToolRunContext.projectId` may be too strict. - `withdrawInteraction` reads `issue.status`, which its type omits. - `issueService.list` and `reconcileNativeFinalizations` omit fields that they return at runtime. - `createGitHubExternalObjectProvider` handles `tokenProvider: null`, but its type forbids this value. ## What Changed - Fixed the 2,168 type errors. The diff changes 322 test and helper files. - Corrected `server/src/__tests__/tsconfig.json`. Editors and the typecheck now read the same options and report the same errors. - Added three `test-tree` layers to `server/scripts/typecheck-layers.mjs`. They read the server layers as declarations, and any error fails them. A hash of the path selects the shard, so a new file does not move other files. - Added `server/scripts/typecheck-stubs/daytona-sdk.d.ts`. Server tests reach Daytona plugin source, and the root install does not have `@daytonaio/sdk`. - Changed two imports in `ui/src/lib/issue-execution-policy.ts` to use `.js` extensions, because server tests import this file. - Made `declarationsConfig` and `--declarations-only` skip the `test-tree` layers, because these layers emit nothing. Other projects read only the source layers. Added a test for this. - Updated `doc/DEVELOPING.md`. ## Verification Run these commands: ```sh pnpm --filter @paperclipai/server typecheck pnpm exec vitest run server/scripts/typecheck-layers.test.mjs ``` Results on macOS (arm64), Node 24, 16 cores. Each check ran after the merge of paperclipai/paperclip#18738 unless the row says otherwise: | Check | Commit | Result | | --- | --- | --- | | Server typecheck, `PAPERCLIP_TYPECHECK_LOW_MEMORY=1`, all 7 layers | `6e45a08573` | Exit 0, 0 errors, 62 seconds, peak 3.52 GiB | | `node scripts/typecheck-test-projects.mjs --low-memory` | `a3306bd13e` | Exit 0, 0 errors, 42 seconds, peak 3.25 GiB | | The same script with `--reuse-declarations` | `1b878aabc0` | Exit 0, 0 errors | | `typecheck-layers.test.mjs` and `live-recovery-work.test.ts` | `01db5634e4` | 90 passed. These include real compiler runs for a pass, a type error, and an invalid tsconfig. | | `tsconfig.declarations.json` after a full server typecheck | `0fb1e4fb0a` | It references only `below`, `cycle`, and `above`. No generated config names a `tsconfig.layers.json` file. | | Owned runtime test in `idle-completed-history` | `e8f2f448ac` | Passed, with the fix that master later replaced | | `idle-incoming-wake`, `plugin-idle-manager`, and the changed `chat-channels` test | `6e45a08573` | 30 passed | | Runner `native-execution.test.ts` | `eab4dcc1f4` | 114 passed | | The 19 server test files that changed since `eab4dcc1f4`, and `typecheck-layers.test.mjs` | `259737b700` | 4,275 of 4,277 passed. The 2 failures are in `chat-channels` and are in the list of 7 below. | The peak is the largest single process, from `/usr/bin/time -l`. Peaks for the server typecheck were between 3.42 GiB and 3.56 GiB across 8 runs. Peaks for the test projects were 3.25 GiB and 3.37 GiB across 2 runs. I did not measure a cgroup. **Runtime check of the fixes.** Types do not prove that a test still works. So I ran all 309 changed server test files, 11,929 tests, before the rebase onto paperclipai/paperclip#18737: | Result | Tests | Detail | | --- | --- | --- | | Passed | 11,913 | Includes the 2 tests and the 54 `dot-runner` tests that passed in a later run, as described below. | | Failed | 7 | The same 7 tests also fail on the commit before the fixes. | | Skipped | 9 | These tests have their own skip conditions: `sentry` 5, and 1 each in `bundled-plugin-registration`, `customer-success-cloud`, `onepassword-connection`, and `workspace-runtime-https-live-exercise`. | The 7 tests that fail with and without this change, on my machine: - 3 tests in `chat-channels` and `email-channels` stop with `ENOENT` on a `SKILL.md` file. The path is in the parent checkout, above the git worktree that I used, so I expect that the worktree is the cause. I did not prove this. - `workspace-instance-cleanup` compares `/private/var/...` with `/var/...`. This is a macOS symlink difference. - 2 tests in `git-reproducible-managed-checkouts` get `unknown_base_git_files` and expect `unpushed_managed_checkout`. I did not find the cause. - 1 more test in `chat-channels`, `materializes direct external-chat finals once`, gets `undefined`. I did not find the cause. CI is the check for these 7 tests. `ci / verify` passed on `eab4dcc1f4`. Failures during this work: - My fix to `agent-runtime-connector-access` changed an invalid permission key in the setup, but not in the assertion. The runtime check found this. I corrected the assertion, and the file passes. - `heartbeat-plugin-environment` failed one wait in the full run. It passed three times alone, so I think that load caused the failure. It can fail again under load. - `dot-runner` skipped all 54 tests in the full run because `cargo` was not on my `PATH`. It then passed 54 of 54 with the toolchain on the `PATH`. - My first `lib` value removed the default DOM types, and the error count went up by 375. I added the DOM libraries. - My Daytona stub with bare `any` left 48 errors, because generic helpers then inferred `unknown`. I added shapes that return promises. - One fix to `agent-secrets-routes` used `class_3_static_lease`, and the test failed. That class allows only specific paths. I reverted to `unclassified`. - `ci / General tests (server (8/12))` failed on `259737b700` because of the flaky port in `idle-completed-history`. paperclipai/paperclip#18853 fixed this test on master, so this pull request keeps that fix. **Audit of the diff.** I counted these patterns in added and removed lines, to find fixes that only hide an error: | Pattern | Added | Removed | | --- | --- | --- | | `@ts-ignore` | 0 | 0 | | `@ts-expect-error` | 0 | 1 | | `.skip` | 0 | 0 | | `as any` | 9 | 9 | | `: any` | 11 | 2 | | `as never` | 8 | 2 | | `as unknown as` | 35 | 4 | | `Promise<unknown>` | 16 | 5 | | `expect(` | 128 | 113 | No hunk removes more `expect(` calls than it adds. Three removals of test content were intentional, and I checked each one: - `muse-control` removed `signalToken`. Line 1012 already asserts that it is absent. - `merged-pr-confirmation-sweep` removed legacy aliases. Other tests cover them. - `heartbeat-workspace-branch-containment` removed a `finalize` branch that no test row can reach. Some partial fixtures stay unchecked through `as unknown as` and `Promise<unknown>`, mostly in `issue-agent-mutation-ownership-routes` and `issue-comment-reopen-routes`. **Accuracy of the check.** One program over the tree and the three shards reported the same errors at the same positions. Memory, as the peak of the largest process on one thread, was 3.93 GiB for one program and 2.43 GiB for the largest shard. I measured these two values before the fixes. Not run or not proven: - I did not run the full `pnpm test:run`, or a root `pnpm build`. I expect CI to run the full suites. - I did not run the 4 GiB container check, or the full `scripts/typecheck-low-memory.mjs`. The peaks above come from macOS, so they are not a Linux cgroup measurement. - I did not run the UI typecheck after the merges. - `workspace-runtime-https-live-exercise` is skipped, so its `cleanup` fix did not run. ## Risks - **A large diff in test files.** 328 files change. A pull request that edits the same tests can get merge conflicts. New tests on master can also add type errors before this merges. Any such error fails this check and needs a fix here. - **A higher cost for changes to source types.** A change to a type can add errors in test files that the author did not touch. The author must then fix them. This is the purpose of the check, but it is new work. - **Changed test behavior.** Some tests now run paths that they did not run before, such as the 9 chat wizard fixtures and the 3 span tests. They pass here, but they can be more sensitive to timing or to the environment. - **The Daytona stub can go stale.** The stub is written by hand. If the plugin uses a new SDK member, the typecheck reports an error until someone updates the stub. - **The release path.** `release-verify.yml` and `scripts/release.sh` run `pnpm -r typecheck`, so they now run this check. - **The tsconfig change.** The test transform reads `server/src/__tests__/tsconfig.json`. The options that I changed are for type checks, and the runs above passed. - **A gap that remains.** Tsconfigs in `packages/paperclip-runner` and in most sandbox provider plugins exclude `*.test.ts`. This check does not read most of those test files, so they can still drift. That work is separate from this pull request. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Anthropic Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with reasoning, tool use, code execution, and subagents. The runtime does not show the context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [ ] I have run tests locally and they pass (7 tests fail locally, with and without this change. See Verification.) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
6dcf4160dd |
perf(chat): load recent messages and page history on scroll (#18358)
## Thinking Path > - Paperclip lets people manage work done by agents. > - A standing agent chat can hold months of conversation. > - The chat route forced full-history mode, bypassing compact task loading. > - A cold visit fetched lifetime run summaries and hydrated historical transcripts. > - This PR starts with 20 recent messages and loads earlier pages on upward scroll. > - It preserves the reader's position, current questions, and draft while history grows. ## Linked Issues or Issue Description **What happened?** Standing agent chats forced full-history loading. A long conversation fetched full activity and run history before it became readable. Earlier comments also used an explicit load control. **Expected behavior** Open a small recent conversation quickly. Load older messages when the reader scrolls upward. Show an inline loading state. Keep the visible message and composer mounted through paging, live updates, and Back navigation. **Steps to reproduce** 1. Open a standing agent chat with hundreds of messages and completed runs. 2. Use a fresh browser query cache. 3. Observe full activity and run reads plus transcript requests for historical runs. 4. Compare the recent window and upward paging after this change. Related work: #18312 and #18323 restored compact task and collection loading. #14848 added reopening of visited agent chats. A GitHub search found no open PR for this chat-history optimization. ## What Changed - Return 20 contiguous recent comments for standing chats, with a stable oldest cursor. Keep the existing compact task exchange contract. Use the existing company/issue/time index for cursor reads. - Keep chats compact even if their task view previously expanded history. Retain observed messages through live catch-up and cache-backed return visits. - Keep fetched history pages, acknowledged sends, and the observed recent cursor separate. Reconcile live replies without losing history after a send or deleted cursor. - Hydrate metadata only for newly revealed message/run IDs; preserve prior batches while paging. Slow-send recovery refreshes the recent window. Receipts failures have an independent retry. - Hydrate run summaries only for loaded messages and the latest turn. Validate batches of at most 100 run IDs and keep the existing issue/company access rules. - Reuse hydrated terminal native streams when only history membership changes. Preserve retry, resume, mode changes, reconnect recovery, and interrupted recovery reads. - Load older messages, decision receipts, and task handoffs through upward scrolling. Support wheel, touch, and keyboard input. Keep pending questions visible. - Reserve a status slot while loading. Preserve a logical message anchor and the mounted composer. Pause failed page reads until an inline Retry. - Add database, route, controller, and scroll regressions, plus an opt-in production-build browser rig for long chats on desktop and mobile. Update design and feature/access documentation. ## Verification - The real PostgreSQL cold-open suite passes all 12 tests, including contiguous pages with equal timestamps, complete history, company denial, and scoped run IDs. - Activity route and OpenAPI suites pass 53 tests. The route run allowed local cold-import overhead; CI retains the repository's normal timeouts. - Three new paging regressions fail against the unchanged source. The candidate's focused paging/window suite passes 26 tests. Broader UI verification is recorded below when complete. - Desktop and mobile browser acceptance covers recent-only startup, per-frame reveal geometry, delayed and failed older pages, stable reading anchors and drafts, Back, older comment links, and one persisted live reply. - Full workspace typecheck and build passed locally. The final UI source also passes its typecheck and production build. - Affected UI suites pass: 212 controller/history/paging tests, 352 thread/scroller tests, 26 native-stream tests, plus metadata batching and live-cache hydration coverage. The issue mutation/access route suite passes 179 tests. - All 22 final desktop/mobile browser journeys pass, including delayed image growth. The isolated second-page request proof also passes six repeated desktop/mobile runs. A separate 6,000-message, 3,000-run fixture passes cold-open and delayed-paging checks on both screen sizes, still opening 20 messages and loading one 50-message page. - Final matched production timings use baseline `aa58fa1073` (`index-BaRiMbxd.js`) and candidate UI source `f9a26b9783` (`index-DPWTbsFF.js`, identical application source at final head `bf71ffd5af`). Chromium 151.0.7922.34, 1440×900, 4× CPU throttle, no added network latency, browser HTTP cache disabled, fresh application cache for each cold visit. Both variants block service workers and use the same warm local API server and immutable 600-message/300-run fixture. Only document HTML is intercepted for both variants; API requests are real. The common API revision is `109981f8c1`; subsequent commits change only UI and tests. | Usable conversation | Baseline median (range) | Candidate median (range) | | --- | --- | --- | | Cold direct URL | 6.57 s (5.69–7.45) | 3.07 s (2.50–3.28) | | Warm in-app return | 3.76 s (3.54–4.76) | 0.67 s (0.58–1.46) | | Back return | 2.53 s (2.12–4.20) | 0.51 s (0.47–1.18) | Five pairs alternate order. Every paired difference favors the candidate for all three journeys. Readiness requires a revealed, fully visible latest reply above the composer and an editable composer. Return timing starts after the Tasks route renders. Cold API requests fall from 953–954 to 71; API wire bytes fall from 517–559 KB to about 49 KB. Separate CPU profiles and request waterfalls confirm the removal of lifetime event hydration and are excluded from the five timing pairs. Both variants have the same expected missing optional plan document (404) and no browser exceptions. Control recent-window API reads have medians of 37 ms / 32 ms, with shared-host outliers up to 376 ms. The 6,000-message recent-window SQL uses `issue_comments_company_issue_created_at_idx` (22 index rows read for a 21-row probe). Geometry assertions retain a 2-pixel tolerance for the reading anchor and first-reveal layout. These are local synthetic measurements, not deployed staging latency. Earlier timing probes are excluded because they did not isolate service workers or composer clearance. Final head `bf71ffd5afb5684ba9105a3127f617e7adf5139c` has 55 successful checks, three optional skips and one neutral result, with no failures or pending checks. Greptile is 5/5 with no open review findings. ### Failure record Early local attempts exposed an unconditional scroller wrapper that changed the old task layout, and automatic retries on chat page failures. Both were corrected. The original task scroller structure stays in place. Metadata request assertions wait for initial connection recovery to settle, then verify only the new page’s reads; retry/resume/reconnect revalidation remains intentional. The initial review caught a send-before-scroll cache defect; its controller regression failed before the fix. Later rapid-paging checks caught scroll readiness being gated by metadata after the first reveal; readiness now stays enabled once revealed. Live replies use the separate acknowledged-message cache, then catch up from the server cursor. Visual review exposed a pagination wrapper bypassing existing composer clearance. The body is again a direct scroller child; cold-open browser checks require the full latest reply above the composer. A resize-observer regression failed before the fix and now passes. Controlled delayed inline images prove both pinned following and a stable reading anchor on desktop/mobile. The first performance fixture lacked valid native event envelopes and caused missing-log fallbacks. It is excluded from latency claims. The final fixture uses saved native assistant events and conversation session generations. The live test uses a temporary fixture-agent key, attributes the reply to an existing synthetic run, and revokes the key. It starts no model or agent process. Local database attempts failed during a Docker shutdown. The retained disposable database was restored and its suite passed. Early fixture errors included a missing conversation identity field and session generation. Mobile test setup also selected an offscreen sidebar link and reused queued human input. Optional announcements could intercept input; the fixture now dismisses them before the journeys. The scroll observer now waits for the delivered wheel event and settled animation frames, including when the viewport is already at the top. A concurrently running live test briefly changed another probe’s row count; the final acceptance and timing runs were sequential on separate fixtures. The final rig uses the visible mobile navigation and a saved agent reply. Anchor observation waits for the user's scroll to settle before measuring the effect of live updates. The full local `pnpm test:run` attempt hit repeated embedded PostgreSQL initialization failures and failures in untouched native-session executor suites; it was stopped after the repeated environment failures. The targeted real-PostgreSQL suite passed, and final-head CI is required before handoff. The first CI run also caught an exact mock expectation missing the added company scope; the expectation is updated and its 179-test suite passes. The final-head first CI attempt also stalled an assigned child task in the existing plan-approval scenario, then hit a heartbeat-run row lock timeout during fixture cleanup. The failed shard and aggregate check passed on an unchanged rerun of the same commit. Scenario assertions and timeouts were unchanged. The failure and rerun are retained in [CI run 41116120430](https://github.com/paperclipai/paperclip/actions/runs/41116120430). ## Risks - Historical transcript diagnostics no longer load on a cold chat open. Saved messages and conversation events remain available; diagnostic run views keep their full records. - Cursor paging must not omit comments with equal timestamps. Database tests cover timestamp/UUID ordering and the complete set. - Prepended or late content can move the reader. Browser checks cover retained row identity, viewport offsets, mounted drafts, and Back. Previously loaded pages stay in the query cache for its existing lifetime. - Local synthetic evidence does not establish deployed staging latency. This PR does not merge or deploy the change. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, tool use, and code execution. The exact backend deployment ID and context window were not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (affected checks; limits are recorded above) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
7e27bc5007 |
fix(tasks): keep anchored opens compact and stabilize saved cards (#18092)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task pages must make the current instruction and recent conversation readable quickly. > - Compact startup avoids reading all earlier messages and run activity. > - Every URL fragment previously selected the full-history path, even for documents. > - Saved document cards also arrived after the conversation was visible. PR statistics could add another row later. > - This pull request keeps anchors compact and reveals the saved conversation structure together. > - The benefit is stable reading position without loading unrelated history. ## Linked Issues or Issue Description Related: #17593 introduced compact startup. #14667 covers earlier reveal and scroll work. This fix retains the bounded history contract. **What happened?** Opening a task document link loaded all task history. Ordinary cold opens could reveal messages before saved document cards and plans arrived. Later PR metadata added a statistics row and changed card height. **Expected behavior** Every URL fragment keeps compact startup. Document links open their pane. Older-comment links fetch only the target comment. Saved cards keep their geometry after first reveal. **Steps to reproduce** 1. Create a task with many comments, a saved plan, documents and saved PR work products. 2. Delay the enriched task response and open the task. Observe cards inserted after messages appear. 3. Open a document fragment. Observe full comment, activity and run reads. 4. Delay PR metadata refresh. Observe each card grow when diff counts arrive. ## What Changed - Remove fragment and tab checks from automatic history expansion. Resolve validated comment UUIDs through the authorized single-comment endpoint, encode their path components, retain fetched targets across document navigation, and preserve cancellation/deletion cache updates. - Include the saved plan, document metadata and local work-product projection in the core task read. Skip recovery, workspace and related-task enrichment. Do not refresh providers. Summary queries no longer select other document bodies. - Seed saved work-product and plan queries from core, including already pending or errored reads, without replacing known lists or an authoritative empty result. Coordinate the bounded recent reads, saved cards, latest transcript and settled initial pane width before first reveal. Keep the thread and composer mounted during refresh and explicit expansion. - Open continuation summaries in the document pane. Reserve PR statistics geometry while provider metadata loads. - Add a reusable synthetic local task, nine browser checks, frame measurements and video. Extend the PostgreSQL core-route fixture with a saved PR row and assert its identity, URL, state and stored metadata survive the response. Update design guidance and capability maps. ## Verification - Red: visible document/plan cards changed from 0 to 1 to 4; thread height grew from 690 to 1,432 pixels. Green: all seven saved cards appear before reveal, with constant thread height and zero per-card movement in the final reviewed browser run. The final 1,492-pixel layout includes the reserved PR statistics lines. - Red: delayed PR diff counts changed all three cards from 62 to 82 pixels. Green: their heights stay at 82 pixels before and after metadata refresh. - Nine browser journeys pass: delayed enrichment, document/comment/unknown/continuation anchors, document clicks and explicit history expansion with a retained draft, missing-comment retry, delayed PR statistics, and retained older-comment position after document navigation. - 583 focused UI tests passed on `c91fa107ba`. After the plan-cache follow-up, all 212 affected page/cache/plan-hook tests and UI typecheck pass, including 14 plan-hook cases. The original 16 focused PostgreSQL tests passed for the server changes. - Merge-review red: holding the duplicate stored work-products read blocked the first reveal past 10 seconds despite core already containing the saved cards. Both empty and populated core projections now reveal while that request remains pending. Malformed-comment tests prove no target request is made; API tests prove path encoding and abort-signal forwarding. - The populated core-route test now inserts a real saved PR work product alongside the plan and documents, then checks the complete saved projection through HTTP. All ten task cold-open PostgreSQL tests pass. - Plan-cache review red: an existing pending or errored plan query prevented the saved core plan from reaching the consumer. Four real-hook cases failed before the data-presence fix and pass afterward, for both present and absent plans. A separately fetched newer plan or known absent plan remains authoritative. - Further layout red: the initial properties pane reduced visible thread width from 775 to 742 pixels after reveal. The reveal now waits for two stable width frames; the browser check includes width, height, card counts and per-card geometry. - `pnpm -r typecheck` and `pnpm build` passed for the original change; UI typecheck and build pass after review fixes. Token, module-boundary and map checks pass. `pnpm test:run` was stopped after about 15 minutes in the serialized server lane. No full local test-suite pass is claimed. On the earlier head, hosted build/typecheck and all Vitest lanes passed; one E2E shard failed because the agent-configuration clipboard helper copied page text into its YAML parser. On `c91fa107ba`, Greptile was 5/5 and every E2E shard passed, including the earlier clipboard case. One unchanged Discord chat-binding test in server shard 8 returned false after 185 ms; its 16-test integration file passes locally without changes. The CI cause remains unestablished. On `be436ce492`, both server shard 8 and E2E shard 7 passed without changing those tests or their product code. On final head `e357b3bced`, all CI jobs pass, including full general and serialized tests, all eight E2E shards, typecheck, build and the aggregate verification gate. Exact-head Greptile is 5/5 with no findings. The overall status rollup contains 55 successes, two intentionally skipped Storybook checks and a neutral Paperclip Review. That advisory review ended without a complete assessment; all previously posted findings are resolved and it added no current findings. - Test startup exceeded an initial 5-second deadline once while development assets and other checks were active. The final browser checks allow 10 seconds. These runs establish geometry and bounded requests; they do not establish a reliable latency improvement. An initial 503 error fixture exercised global reconnect/backoff. The target-comment failure test uses a terminal 404. - Browser data is synthetic. Provider metadata is simulated for the delayed-response check. No staging deployment or live provider execution is claimed. ## Risks - Core task reads now include local saved-card queries and the saved plan body. Other document bodies and all historical run activity remain outside that path. - First reveal waits for the bounded current structure. The existing 15-second incomplete-history fallback and inline retry remain available. - Continuation-summary links open in the document pane in the current task interface. Classic tasks keep their existing surface. - No migration or mutation permission change. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, code editing, shell tools and browser testing. The exact serving model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
143d20e466 |
fix(server): retain native Codex Plan write boundaries (#18034)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Codex task chat keeps a saved Plan inside the turn that wrote it. > - The compact conversation API omits ordinary tool history. > - That omission also removes the write receipt needed to place the Plan. > - This pull request retains a small successful write receipt in conversation reads. > - The UI can then keep the saved Plan at its write boundary after readback. ## Linked Issues or Issue Description **What happened?** A real native Codex Product E2E attempt saved its Plan and displayed it inside a settled turn. The Plan used the fallback position. The browser's final conversation-only API response contained no `write_document` receipt. The full event snapshot retained the successful write. The native placement assertion failed and remains failed in the original report. **Expected behavior** Conversation readback must retain enough evidence to place the saved Plan at its successful write boundary. It must keep arbitrary tool bodies and ordinary tool history out of the compact response. **Steps to reproduce** 1. Save a canonical Plan through a native `write_document` call. 2. Read the run with `view=conversation`. 3. Before this fix, the response omits the write receipt and the UI places the Plan at the end-of-turn fallback. Related eval evidence repair: Refs #18026. This is a separate product readback fix. It does not resolve the failed human-input call or the Claude tool-availability failure from those attempts. ## What Changed - Include only successful canonical `plan` document results, matched to a native `write_document` call in the same run, turn, session and source instance. Ordinary document writes stay excluded. - Build the small receipt inside PostgreSQL before its body crosses the database connection. Preserve only fixed event metadata, tool identity, the `plan` key and mutation disposition. Keep the existing payload decoder for ordinary conversation events. - Keep cursor paging, run scope, ordinary-tool exclusion, and full event reads unchanged. - Document the compact read contract in OpenAPI. - Scope this fix to the canonical native dynamic-tool call/result pair used by Codex. The retained Claude sample proves compatibility with that captured shape. It does not qualify every current ACPX Claude producer. Provider-activity-only paths and OpenCode keep the explicit fallback. ## Verification - Real PostgreSQL red: the new boundary test fails because the first selected event is assistant text rather than the saved write. The six existing tests pass. - Real PostgreSQL/API green: 36 tests pass across task cold-open and OpenAPI routes on an isolated Linux current-master checkout. - UI: 104 transcript tests pass, including completed native placement from the body-free receipt. - Review regression red: a later notes write incorrectly enters conversation readback (one failure, six passes). After the fix, the PostgreSQL/API suite passes all 36 tests. - Fixed the new UI fixture's string-to-Date type mismatch found by hosted CI. Original failed CI remains visible; fresh exact-head checks are pending. - Existing token/style gates pass. - Retained native Codex and Claude fixture shapes both pass. Each fixture includes the canonical item pair and parallel provider `tool.execution.*` activity. All identifiers and document bodies in the fixtures are synthetic. This captured-shape test is historical readback compatibility, not fresh current-source Claude acceptance. The Codex canonical pair is emitted by `packages/paperclip-runner/src/drivers/codex/codex-session-server-requests.ts` in the admitted dynamic-tool handler. - Projection-collision regression red: a non-Plan result selected by conversation identity was relabeled as a Plan. The SQL projection now uses the same full Plan predicate as selection. The regression and both native fixtures pass. - The database test checks cursor paging, a Plan followed by notes, failed/read/control-plane/unrelated-turn exclusion, schema and ID bounds, omission of a large tool body, unchanged full-history reads, and ordinary conversation payloads containing NUL. - Local server typecheck fails on 80 installed workspace export mismatches outside changed paths. No changed-path type error or local full build pass is claimed. Clean hosted checks must pass before merge. - Current-head hosted CI and legacy status pass. One failed-check retry followed recorded runner shutdowns and an unrelated native-distribution timeout; that test passed all 33 cases locally. Original failed logs remain retained. - Zero provider calls or paid attempt retries. Original failures and accounting journals remain unchanged. This is not a new live qualification result. ## Risks - Conversation pages include matching Plan results within the existing 1,000-row page limit. The query adds a run-scoped match against canonical tool calls and projects results inside SQL. PostgreSQL still reads the stored JSON to select receipts; private bodies are not transferred to the application. It adds no separate database round trip or background work. - Matching requires the existing canonical call/result identities. Missing or malformed historical receipts keep the explicit fallback. This fixes compact conversation readback; it does not redesign placement in an unrelated full-history multi-document UI flow. - This restores the existing UI write-boundary mechanism. It does not change Plan acceptance, permissions, or completion rules. ## Model Used - OpenAI Codex, GPT-6, with reasoning and code execution tools. The exact serving variant and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
DottaandPaperclip
|
a1f77821b9 |
perf(tasks): show recent conversation before historical activity (#17593)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - A task page must show the instruction and current conversation
quickly.
> - The current page waits for related task data and historical run
transcripts.
> - These reads grow with task history, even when the reader only needs
the latest answer.
> - This pull request gives cold opens a small initial view and loads
older history on demand.
> - Messages and runs observed while the page stays open remain visible.
> - The benefit is a readable task before historical diagnostics finish
loading.
## Linked Issues or Issue Description
Related: #14667 and #16229. The first changes the reveal gate and scroll
anchors. The second adds general history pagination. This PR adds a core
task read and a folded recent-conversation view. It preserves the
default full-read API contracts. The shared files will need coordination
if either related PR lands first.
**What existing behavior does this improve?**
Cold opening an ordinary task with a long conversation or large run
logs.
**Current behavior**
Task enrichment and historical transcripts delay the readable
conversation. A cold open can fetch several megabytes of tool activity
before revealing saved messages.
**Proposed behavior**
Show the task instruction, recent saved replies, latest human input, and
the latest run's conversation data first. Fetch older messages and full
run activity when the reader chooses **More messages**. Retain new
messages while the page stays open.
## What Changed
- Add an authorized core issue view that skips related enrichment. Keep
enrichment on the existing detail read. Share task mutation, rollback
and invalidation updates with the core cache.
- Add a bounded recent-comment read. Preserve the first human
instruction when no description exists, including legacy rows with a
NULL author type. Keep company and task access checks and comment
redaction.
- Add optional recent activity and run-summary views. Fetch original
input receipts for visible comments separately so delivered messages
stay out of the queue. Reconcile watched unfinished runs after a
reconnect beyond the preview window, removing deleted runs after
reconciliation.
- Filter the latest native run's initial events to assistant messages,
request lifecycles and terminal results. Full event reads remain
available on expansion. Legacy fallback reads use a small tail extended
to the first complete record. HTTP cancellation aborts the storage read.
- Retain observed messages across cursor catch-up and new runs. Apply
local cancellation and deletion to both caches and their task aliases.
Refetch locally seeded history pages on expansion. Preserve expansion
across identifier and UUID links. Keep deep links, classic tasks and
Agent Chat on their existing full-history path.
- Update API documentation, design guidance, capability maps and
regression coverage.
## Verification
- Final acceptance on `9ea0b15ae44d9f6d2c9a9f824b4835f299f92979`: all CI
gates passed, including server/UI tests, all eight browser shards,
runner checks, full typecheck and build. [Final CI
run](https://github.com/paperclipai/paperclip/actions/runs/40326990328).
GitHub reports 55 successful checks, two intentionally skipped checks,
and one neutral Paperclip Review check described below. No checks are
pending or failed; Greptile is 5/5 and no review threads remain open.
- October 26 merge update: merged current `master`. The only conflict
was in task-page imports; both the compact-history hook and the new
live-update hooks remain. All CI tests and build jobs passed on
`044fd6733d`. Fresh review found canceled-comment cache drift, legacy
human-comment filtering, send-then-expand truncation, and partial legacy
log records. These defects were fixed in `af3d66a5bd`.
- Review-fix verification: 470 tests pass across ten affected suites,
including all six PostgreSQL cold-open tests without a helper override.
Cancellation/delete and send-then-expand regression tests failed before
the fixes and pass afterward. Coverage includes task identifier/UUID
reopen, stale in-flight reads, legacy NULL author types, local-file and
object-storage tails cut inside UTF-8 text, route forwarding, and
request cancellation. Full workspace typecheck and production build,
token gates, map validation and diff checks pass.
- Follow-up review found that a failed expanded legacy read lost its
record-alignment state on retry. `9ea0b15ae4` commits the expanded
window only after a successful read. The new regression fails before the
fix and passes afterward; all 19 transcript tests, UI typecheck, UI
build and token gates pass. Greptile reviewed this commit at 5/5 with no
actionable findings, and all review threads are resolved.
- The final Paperclip Review check ended neutral because its execution
did not complete; no passing Paperclip assessment is claimed. Its
earlier four concrete findings were fixed and regression-tested.
Greptile and the security scans completed successfully on the final
commit.
- The preceding `af3d66a5bd` CI run was canceled when superseded by the
retry fix. Its server shard 11 check briefly reported failure, but the
job API records cancellation and its test step passed all 1,750 tests.
This superseded run is not used as final CI acceptance.
- 549 focused tests pass on the final implementation across nine task
UI, activity-route, OpenAPI and PostgreSQL suites. These cover pending
history, desktop/mobile visibility, expansion, retained runs, message
bursts, cancellation recovery and queued-comment removal in both views.
- Legacy expansion keeps the latest answer visible while fetching and
deduplicating older output.
- Five PostgreSQL tests pass for core access denial, recent comments,
missing-description instructions, event paging and bounded activity/run
reads. macOS could not start another PostgreSQL cluster because it
exhausted its semaphore limit. These tests used a separate disposable
database on an existing local server through a temporary test setup
override. The helper was restored afterward.
- Full workspace typecheck and build pass with the final state fixes, as
do token gates and capability-map validation.
- Token gates, capability-map validation and `git diff --check` pass.
- Browser proof used a disposable task with 80 comments and six large
run histories. The cold view showed the instruction and recent exchange.
**More messages** revealed older comments. Navigation away and back
preserved expansion.
- Local HTTP sample: core 50 ms, recent comments 24 ms, recent run
summaries 17 ms. The latest native conversation response was 839 JSON
characters / 8 ms; its full event response was 1,099,005 / 86 ms.
Browser content paint was 1.34 seconds with development assets cached.
These are local observations, not staging latency claims.
- The first CI run passed every job except four board-send browser cases
and the review threshold. Those browser fixtures only mocked the old
comments endpoint; they now serve both recent and full reads, retaining
all message and attachment assertions. Both initial review findings were
fixed with regression tests. The next CI run passed every job. The
deeper review identified core-cache mutation, older delivery receipt and
retained run-status gaps; those are now fixed with regression coverage.
Final review is 5/5 with zero unresolved findings. Final-head CI
initially failed one unchanged runner shutdown test
(`runnerd-codex-transport.test.ts`, timeout-attachment case) with
`NativeSessionCloseUnrecoverableError`; all other CI jobs passed. The
same runner code passed on
|