Add account list-detail UIs for values, jobs, memories, and email - #937
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (26)
📝 WalkthroughWalkthroughAdds account Values, Jobs, Memories, and Email pages with typed loaders, authenticated APIs, interactive client routes, navigation and document metadata wiring, unit tests, and end-to-end coverage. ChangesAccount management surfaces
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant ClientRoute
participant AccountApi
participant AccountData
participant Storage
Browser->>ClientRoute: Open account section
ClientRoute->>AccountApi: GET account JSON endpoint
AccountApi->>AccountData: Load authenticated data
AccountData->>Storage: Query account records
Storage-->>AccountData: Return list and selected detail
AccountData-->>AccountApi: Return loader payload
AccountApi-->>ClientRoute: Return JSON data
ClientRoute-->>Browser: Render list and detail views
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-937.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
packages/worker/src/app/loader-data.ts (1)
727-752: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThese shapes are declared twice.
AccountValueListItem,AccountValueDetail, andAccountValuesLoaderDataare identical to the exports inpackages/worker/src/app/account-values-data.ts(Lines 13-38), andaccount-memories-data.ts/account-email-data.tsshow the same pattern per graph context. Because the client type-checks against this file while the handlers produce the other, drift here goes unnoticed until runtime. Prefer re-exporting the data-module types (or the reverse) so there is one definition per shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/loader-data.ts` around lines 727 - 752, Remove the duplicate AccountValueListItem, AccountValueDetail, and AccountValuesLoaderData declarations from loader-data.ts and reuse the corresponding exports from account-values-data.ts. Update loader-data.ts imports or exports so consumers retain the same public types while establishing a single source of truth.packages/worker/client/routes/account-values.tsx (1)
90-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReserved-name rule is duplicated client/server.
isReservedAccountValueNameandreservedAccountValueWriteErroralready exist server-side (packages/worker/src/app/account-values-data.tsLines 40-42 andpackages/worker/src/app/handlers/account-values.ts). Two copies of the prefix list and the message will drift. Consider extracting them into a shared module both sides import.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/client/routes/account-values.tsx` around lines 90 - 95, Extract the reserved account-value prefixes and write-error message used by isReservedAccountValueName into a shared module, then update both the client route and server account-values handlers to import and reuse those shared symbols. Remove the duplicated local definitions while preserving the existing reserved-name checks and error text.packages/worker/src/app/handlers/account-jobs.node.test.ts (1)
134-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
beforeEachmock reset so assertions don't depend on test order.
not.toHaveBeenCalled()assertions here only hold because prior tests happened not to call those mocks. AbeforeEach(() => vi.clearAllMocks())(re-seedingreadAuthenticatedAppUserandresetInspection()) makes each test independent.Also applies to: 229-229, 247-247, 264-264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/account-jobs.node.test.ts` around lines 134 - 139, Make the tests in account-jobs.node.test.ts independent of execution order by adding a beforeEach that clears all Vitest mocks, then re-seeds readAuthenticatedAppUser and invokes resetInspection(). Ensure this setup covers the affected tests and preserves their expected default mock data.packages/worker/src/app/handlers/account-jobs.ts (1)
58-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer aligning the two
AccountJobsLoaderDatatypes over anascast.The cast between
#app/account-jobs-data.ts'sAccountJobsLoaderDataand#app/loader-data.ts's version silences structural drift; if either shape changes the mismatch surfaces only at runtime in the client. Consider havingloader-data.tsre-export/reference the loader module's type so the assignment type-checks without a cast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/account-jobs.ts` around lines 58 - 61, Align the AccountJobsLoaderData definitions used by `#app/account-jobs-data.ts` and `#app/loader-data.ts` so they share a single source of truth, preferably by re-exporting or referencing the loader module’s type. Then remove the `as AppAccountJobsLoaderData` cast from the `loaderData.accountJobs` assignment in the account jobs handler and ensure the assignment type-checks directly.packages/worker/src/app/account-jobs-data.ts (1)
85-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared selected-id URL decoder. Both loaders reimplement identical logic (path-prefix match, single-segment guard,
decodeURIComponentwith raw fallback,selectedquery fallback), differing only in base path and param name — so hardening or bug fixes must be applied in every copy.
packages/worker/src/app/account-jobs-data.ts#L85-L105: replace the body ofreadAccountJobsSelectedJobIdwith a call to a sharedreadSelectedIdFromUrl({ requestUrl, basePath, pathId })helper.packages/worker/src/app/account-memories-data.ts#L42-L62: replacereadAccountMemoriesSelectedMemoryId's body with the same shared helper, passing/account/memories.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/account-jobs-data.ts` around lines 85 - 105, Extract the duplicated URL selected-ID parsing into a shared readSelectedIdFromUrl({ requestUrl, basePath, pathId }) helper, preserving path-prefix matching, single-segment validation, decoded-ID handling with raw fallback, and the selected query fallback. In packages/worker/src/app/account-jobs-data.ts lines 85-105, replace readAccountJobsSelectedJobId’s body with the helper using its jobs base path and parameter name; in packages/worker/src/app/account-memories-data.ts lines 42-62, replace readAccountMemoriesSelectedMemoryId’s body with the same helper using /account/memories.packages/worker/src/app/handlers/account-memories.node.test.ts (1)
253-265: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSame untyped
nullmock override.
deleteMemoryis inferred to resolve to a memory row, somockResolvedValueOnce(null)may not type-check. Considernull as never(or widen the hoisted mock's return type) for consistency with the email test file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/account-memories.node.test.ts` around lines 253 - 265, Update the deleteMemory mock setup in the not-found test to use the established type-safe null override, such as casting null to never, or widen the hoisted mock return type consistently with the email test. Preserve the existing 404 assertion and handler behavior.packages/worker/src/app/handlers/account-email.ts (1)
41-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the cast by having one source of truth for
AccountEmailLoaderData.
account-email-data.tsdeclares its ownAccountEmailLoaderData(with a comment saying it lives there "until the integrator addsaccountEmailtoAppLoaderData"), and this file imports a second definition from#app/loader-data.ts. Theascast silences any future divergence between the two. Prefer havingloader-data.tsre-export the loader's type and remove the cast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/account-email.ts` around lines 41 - 44, The account email loader uses duplicate AccountEmailLoaderData definitions, with a cast masking divergence. Remove the local duplicate type in account-email-data.ts, have `#app/loader-data.ts` re-export the loader’s AccountEmailLoaderData, and update the account-email handler’s loaderData assignment to use the shared type without an `as` cast.packages/worker/src/app/handlers/account-email.node.test.ts (1)
235-250: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDispatch the
preparemock on SQL text instead of call parity.The
prepareCall % 2heuristic silently breaks if the loader ever adds/reorders a query, and it produces wrong results rather than a clear failure. MatchingCOUNT(*)in the SQL argument is both self-documenting and order-independent.♻️ Suggested change
- let prepareCall = 0 - mockModule.prepare.mockImplementation(() => { - prepareCall += 1 - // countAndListMessages issues COUNT then SELECT in Promise.all order. - if (prepareCall % 2 === 1) return createCountResult(1) - return createListResult([messageRow]) - }) + mockModule.prepare.mockImplementation((sql: unknown) => + String(sql).includes('COUNT(*)') + ? createCountResult(1) + : createListResult([messageRow]), + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/handlers/account-email.node.test.ts` around lines 235 - 250, The prepare mock in createEnv should dispatch based on the SQL text passed to mockModule.prepare rather than the prepareCall parity counter. Return createCountResult for statements containing COUNT(*) and createListResult for the message-list query, with an explicit failure for unexpected SQL so query changes are detected clearly.packages/worker/src/app/account-email-data.ts (1)
253-303: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffLeading-wildcard LIKE plus COUNT(*) on every page load will full-scan
email_messages.Both the search and unfiltered paths run a
COUNT(*)alongside the page query;LIKE '%term%'onsubject/from_addresscannot use an index. Worth confirming a(user_id, created_at DESC, id DESC)index exists for the list path, and consider FTS or dropping the exact total (e.g. fetchpageSize + 1to infer "has more") if message volumes grow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/app/account-email-data.ts` around lines 253 - 303, The countAndListMessages function performs an expensive COUNT(*) on every page and uses leading-wildcard LIKE searches that cannot use ordinary indexes. Confirm or add the (user_id, created_at DESC, id DESC) index for list ordering, and replace exact totals with pageSize + 1 fetching to expose whether more messages exist; for scalable search, use the project’s existing FTS mechanism instead of leading-wildcard LIKE where available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/client/routes/account-memories.tsx`:
- Around line 313-316: Update the showMemoryNotFound calculation near
detailMemory to also require that the detail payload is not pending, using the
same pending-load flag and guard pattern as the jobs and values routes. Preserve
the existing selectedId, detailMemory, and ready-status checks.
In `@packages/worker/src/app/account-memories-data.ts`:
- Around line 135-157: Update the memory search flow around listMemoriesByUserId
and memoryMatchesQuery so query filtering occurs before limiting results, or
otherwise expose that only the newest 100 rows were searched in the returned
payload. Ensure older matching memories are discoverable when the query is
applied server-side.
In `@packages/worker/src/app/handlers/account-values.node.test.ts`:
- Around line 345-349: Update the readAuthenticatedAppUser mock override in
packages/worker/src/app/handlers/account-values.node.test.ts:345-349 to use null
as never, or widen the hoisted mock type to allow null. Apply the same treatment
to the deleteMemory override in
packages/worker/src/app/handlers/account-memories.node.test.ts:253-265,
preserving the existing unauthorized behavior.
---
Nitpick comments:
In `@packages/worker/client/routes/account-values.tsx`:
- Around line 90-95: Extract the reserved account-value prefixes and write-error
message used by isReservedAccountValueName into a shared module, then update
both the client route and server account-values handlers to import and reuse
those shared symbols. Remove the duplicated local definitions while preserving
the existing reserved-name checks and error text.
In `@packages/worker/src/app/account-email-data.ts`:
- Around line 253-303: The countAndListMessages function performs an expensive
COUNT(*) on every page and uses leading-wildcard LIKE searches that cannot use
ordinary indexes. Confirm or add the (user_id, created_at DESC, id DESC) index
for list ordering, and replace exact totals with pageSize + 1 fetching to expose
whether more messages exist; for scalable search, use the project’s existing FTS
mechanism instead of leading-wildcard LIKE where available.
In `@packages/worker/src/app/account-jobs-data.ts`:
- Around line 85-105: Extract the duplicated URL selected-ID parsing into a
shared readSelectedIdFromUrl({ requestUrl, basePath, pathId }) helper,
preserving path-prefix matching, single-segment validation, decoded-ID handling
with raw fallback, and the selected query fallback. In
packages/worker/src/app/account-jobs-data.ts lines 85-105, replace
readAccountJobsSelectedJobId’s body with the helper using its jobs base path and
parameter name; in packages/worker/src/app/account-memories-data.ts lines 42-62,
replace readAccountMemoriesSelectedMemoryId’s body with the same helper using
/account/memories.
In `@packages/worker/src/app/handlers/account-email.node.test.ts`:
- Around line 235-250: The prepare mock in createEnv should dispatch based on
the SQL text passed to mockModule.prepare rather than the prepareCall parity
counter. Return createCountResult for statements containing COUNT(*) and
createListResult for the message-list query, with an explicit failure for
unexpected SQL so query changes are detected clearly.
In `@packages/worker/src/app/handlers/account-email.ts`:
- Around line 41-44: The account email loader uses duplicate
AccountEmailLoaderData definitions, with a cast masking divergence. Remove the
local duplicate type in account-email-data.ts, have `#app/loader-data.ts`
re-export the loader’s AccountEmailLoaderData, and update the account-email
handler’s loaderData assignment to use the shared type without an `as` cast.
In `@packages/worker/src/app/handlers/account-jobs.node.test.ts`:
- Around line 134-139: Make the tests in account-jobs.node.test.ts independent
of execution order by adding a beforeEach that clears all Vitest mocks, then
re-seeds readAuthenticatedAppUser and invokes resetInspection(). Ensure this
setup covers the affected tests and preserves their expected default mock data.
In `@packages/worker/src/app/handlers/account-jobs.ts`:
- Around line 58-61: Align the AccountJobsLoaderData definitions used by
`#app/account-jobs-data.ts` and `#app/loader-data.ts` so they share a single source
of truth, preferably by re-exporting or referencing the loader module’s type.
Then remove the `as AppAccountJobsLoaderData` cast from the
`loaderData.accountJobs` assignment in the account jobs handler and ensure the
assignment type-checks directly.
In `@packages/worker/src/app/handlers/account-memories.node.test.ts`:
- Around line 253-265: Update the deleteMemory mock setup in the not-found test
to use the established type-safe null override, such as casting null to never,
or widen the hoisted mock return type consistently with the email test. Preserve
the existing 404 assertion and handler behavior.
In `@packages/worker/src/app/loader-data.ts`:
- Around line 727-752: Remove the duplicate AccountValueListItem,
AccountValueDetail, and AccountValuesLoaderData declarations from loader-data.ts
and reuse the corresponding exports from account-values-data.ts. Update
loader-data.ts imports or exports so consumers retain the same public types
while establishing a single source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f9ca377-3e80-4e80-8ff0-715daa42c50b
📒 Files selected for processing (26)
e2e/account-email.spec.tse2e/account-jobs.spec.tse2e/account-memories.spec.tse2e/account-values.spec.tspackages/worker/client/routes/account-email.tsxpackages/worker/client/routes/account-jobs.tsxpackages/worker/client/routes/account-management-components.tsxpackages/worker/client/routes/account-memories.tsxpackages/worker/client/routes/account-values.tsxpackages/worker/client/routes/index.tsxpackages/worker/src/app/account-email-data.tspackages/worker/src/app/account-jobs-data.tspackages/worker/src/app/account-memories-data.tspackages/worker/src/app/account-values-data.tspackages/worker/src/app/document-head.tspackages/worker/src/app/handlers/account-email.node.test.tspackages/worker/src/app/handlers/account-email.tspackages/worker/src/app/handlers/account-jobs.node.test.tspackages/worker/src/app/handlers/account-jobs.tspackages/worker/src/app/handlers/account-memories.node.test.tspackages/worker/src/app/handlers/account-memories.tspackages/worker/src/app/handlers/account-values.node.test.tspackages/worker/src/app/handlers/account-values.tspackages/worker/src/app/loader-data.tspackages/worker/src/app/router.tspackages/worker/src/app/routes.ts
| const rows = await listMemoriesByUserId(input.env.APP_DB, userId, { | ||
| statuses, | ||
| limit: 100, | ||
| }) | ||
| const memories = rows | ||
| .map((row) => | ||
| toListItem({ | ||
| id: row.id, | ||
| category: row.category, | ||
| status: row.status, | ||
| subject: row.subject, | ||
| summary: row.summary, | ||
| details: row.details, | ||
| tags: parseJsonStringArray(row.tags_json), | ||
| sourceUris: parseJsonStringArray(row.source_uris_json), | ||
| dedupeKey: row.dedupe_key, | ||
| createdAt: row.created_at, | ||
| updatedAt: row.updated_at, | ||
| lastAccessedAt: row.last_accessed_at, | ||
| deletedAt: row.deleted_at, | ||
| }), | ||
| ) | ||
| .filter((memory) => memoryMatchesQuery(memory, query)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Search filters only within the newest 100 rows.
The limit: 100 fetch happens before memoryMatchesQuery, so a query matching an older memory returns nothing even though the record exists. Either push the query into listMemoriesByUserId (or a search path) or make the cap explicit in the returned payload so the UI can say results are limited to the newest 100.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/app/account-memories-data.ts` around lines 135 - 157,
Update the memory search flow around listMemoriesByUserId and memoryMatchesQuery
so query filtering occurs before limiting results, or otherwise expose that only
the newest 100 rows were searched in the returned payload. Ensure older matching
memories are discoverable when the query is applied server-side.
| mockModule.readAuthenticatedAppUser.mockResolvedValueOnce(null) | ||
| const unauthorized = await handler.handler({ | ||
| request: new Request('https://example.com/account/values.json'), | ||
| }) | ||
| expect(unauthorized.status).toBe(401) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hoisted mocks infer non-nullable return types, so null overrides may not type-check. Both files declare vi.fn(async () => ({ ... })) and later override with mockResolvedValueOnce(null); the email test avoids this with null as never.
packages/worker/src/app/handlers/account-values.node.test.ts#L345-L349: usenull as neverfor thereadAuthenticatedAppUseroverride (or type the hoisted mock to includenull).packages/worker/src/app/handlers/account-memories.node.test.ts#L253-L265: apply the same treatment to thedeleteMemoryoverride.
📍 Affects 2 files
packages/worker/src/app/handlers/account-values.node.test.ts#L345-L349(this comment)packages/worker/src/app/handlers/account-memories.node.test.ts#L253-L265
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/app/handlers/account-values.node.test.ts` around lines
345 - 349, Update the readAuthenticatedAppUser mock override in
packages/worker/src/app/handlers/account-values.node.test.ts:345-349 to use null
as never, or widen the hoisted mock type to allow null. Apply the same treatment
to the deleteMemory override in
packages/worker/src/app/handlers/account-memories.node.test.ts:253-265,
preserving the existing unauthorized behavior.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| Pick a memory from the list to inspect its details, sources, | ||
| and timestamps. | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
Memories detail loading state missing
Medium Severity
While a memory detail fetch is in flight, the main panel falls through to the empty “Select a memory” state even though the URL already selects a memory. waitingForDetail is computed but never used in the detail ternary, unlike the jobs page.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 378955d. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 49c131e. Configure here.
| env: input.env, | ||
| request: input.request, | ||
| user: input.user, | ||
| }) |
There was a problem hiding this comment.
Delete reload drops memory filters
Medium Severity
After a successful memory delete, the API reloads list data using the POST request URL (/account/memories.json), so q and includeDeleted from the user’s inbox view are ignored. The client applies that payload before navigating, briefly showing the wrong list versus the URL filters.
Reviewed by Cursor Bugbot for commit 49c131e. Configure here.
Give users the same URL-backed list/detail management pattern already used for secrets, packages, and connectors for the four user-owned entities that previously had MCP/capability access only. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Reject reserved value names and package-job deletes from the account UI, lock value names while editing, harden selected-id decoding, and make Playwright heading assertions exact so format/validate gates pass. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Match the jobs/values pending-load guard so selecting a memory no longer briefly shows "Memory not found" before the detail payload arrives. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
The previous commit ran prettier instead of the repo oxfmt formatter, which failed CI format:check. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
49c131e to
b20d6f6
Compare
Fold post-#937/#939/#940 account verification tests into fewer workflow journeys and drop low-signal Playwright coverage. - account-values handler: list/select/save/delete + rejection matrix - account-memories handler: list/filter/select + soft/force delete - activity-data helpers + load/cursor; billing refresh workflow without instructional error-copy pins - auth-session password-change matrix; auth-denial and email-delivery alert cron/threshold/cooldown journeys - trim activity e2e instructional empty-state copy; drop memories e2e (values e2e + memories handler cover the pattern); drop jobs schedule text pin Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
…#958) * test: consolidate low-signal workers tests into workflow journeys Collapses 32 tests across four files down to 15, following the 'fewer longer workflow tests' principle from testing-principles.md. run-records.workers.test.ts (20 → 9): - Merge eager-begin/finish, finish-only upsert, one-shot recordRunRecord, waitUntil non-blocking, and execute on-failure policy into a single 'write surfaces journey' test. - Delete 'sandbox level markers become structured log levels' — covered by the existing sandbox e2e test which already asserts structured log levels on each entry. - Merge retention-priority, cap-protect-running, stale-reconcile, stale-cap-evict, and age-cap scenarios into 'cap and stale retention journey' (each with its own userId for DO isolation). - Extend 'write after idle re-arms alarm and age-prunes' into a full 'alarm lifecycle' narrative that also covers the fresh-arm far-future deadline assertion and the self-termination assertion from the two standalone alarm tests. runs.workers.test.ts (5 → 2): - Keep the auth guard test unchanged. - Merge run_list filter smoke, run_get with logs/foreign/missing rejection, run_summary counts, and run_list/run_summary tenant isolation into a single 'run capabilities smoke' test. Filter/ pagination matrix remains in run-records.workers.test.ts. execute-console-capture.workers.test.ts (4 → 2): - Merge success-levels, throw-capture, and unshimmed-methods into 'console capture contract'; keep 'reused dynamic workers' separate. stripe-webhooks.workers.test.ts (3 → 2): - Fold the 503-when-secret-unset guard into the main journey test as an upfront check before any seeding. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * test: fold Sentry, D1 retry, and small unit microtests into journeys Collapse post-#934 one-string-per-filter and sibling unit cases toward fewer longer workflow tests. - sentry-options: fold UserCodeError hint + platform keep into the main filter journey; delete standalone user-code-error.node.test.ts - d1-retry: keep representative matchers and one retry/fail path - observability: fold conversationId/storageId/detail context into the platform-bug reporting journey - activation: merge twice-same-package, HTTP ignore, sticky timestamp, fast path, and never-throw degradation - fold stripe signature fixture, package-workflow sentry tags, invocation surface/name helpers, and package_service_states count + entitlement boundary Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * test: consolidate account list/detail and alert suites Fold post-#937/#939/#940 account verification tests into fewer workflow journeys and drop low-signal Playwright coverage. - account-values handler: list/select/save/delete + rejection matrix - account-memories handler: list/filter/select + soft/force delete - activity-data helpers + load/cursor; billing refresh workflow without instructional error-copy pins - auth-session password-change matrix; auth-denial and email-delivery alert cron/threshold/cooldown journeys - trim activity e2e instructional empty-state copy; drop memories e2e (values e2e + memories handler cover the pattern); drop jobs schedule text pin Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * style: apply oxfmt after mistaken prettier pass CI format:check uses oxfmt (tabs/single quotes). A Prettier pass had rewritten touched test files to spaces/double quotes. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
3 participants
Summary
Adds URL-backed account management list/detail pages for the four user-owned entities that previously had MCP/capability access only:
/account/values_integration:/_openapi:; name locked when editing/account/jobs/account/memories/account/emailFollows the existing secrets/MCP-servers/connectors pattern (
createListDetailRoute, account management shell, JSON APIs, nav entries).Test plan
npm run validate(format, lint, typecheck, 1303 unit tests, 24 e2e, MCP e2e, primitives, migrations)qfilterSystem recap — extends existing primitives (medium risk)
Mode: recap · Base:
main· Head:cursor/account-list-detail-entities-2b17Classification: extends — browser app gains four account list/detail surfaces and JSON APIs; values/jobs/memories/email services are composed as-is behind session-auth handlers. No new primitives; per-user isolation unchanged (every read/write binds
userId).Primitives touched
app-ui/account/{values,jobs,memories,email}routes, loaders, nav, document titlesvaluesjobsmemoriesemailSystem map
Before / after
Summary by CodeRabbit
New Features
Tests