diff --git a/.github/workflows/weekly-use-effect-review.lock.yml b/.github/workflows/weekly-use-effect-review.lock.yml index b0dc9c3e..29779c78 100644 --- a/.github/workflows/weekly-use-effect-review.lock.yml +++ b/.github/workflows/weekly-use-effect-review.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"71a4478acc0f9a335f5aad09be0a56a3dc033b9140dee7c22612f7c123f3d415","body_hash":"a92bbc71bd4a08103817df7740bac3999a7dcee60f202f50eeed9501066edbc9","compiler_version":"v0.86.2","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.79"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"71a4478acc0f9a335f5aad09be0a56a3dc033b9140dee7c22612f7c123f3d415","body_hash":"bd1d56de5e3ff2f12864ebb0025aa53859f16742faaedf5ea7be03fe3ac4573c","compiler_version":"v0.86.2","strict":true,"agent_id":"copilot","engine_versions":{"copilot":"1.0.79"}} # gh-aw-manifest: {"version":1,"secrets":["GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"3d3c42e5aac5ba805825da76410c181273ba90b1","version":"v7.0.1"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"github/gh-aw-actions/setup","sha":"6aab9e5b5c91c615506061f09bedd81a23babe3c","version":"v0.86.2"},{"repo":"oven-sh/setup-bun","sha":"0c5077e51419868618aeaa5fe8019c62421857d6","version":"v2.2.0"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.44","digest":"sha256:0d727725c737b58c7bdf51f640cffb928385ec46517e0917c7f1a02f1bada8b4","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.44@sha256:0d727725c737b58c7bdf51f640cffb928385ec46517e0917c7f1a02f1bada8b4"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.44","digest":"sha256:b50fbadba138f6e9aba94aca09711335c489bb3b15861220cb66f6092e042dc7","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.44@sha256:b50fbadba138f6e9aba94aca09711335c489bb3b15861220cb66f6092e042dc7"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.44","digest":"sha256:83e48bbe12c634be8c228a576832fe45f66c529ac3659db92bddbcf2eeb6d627","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.44@sha256:83e48bbe12c634be8c228a576832fe45f66c529ac3659db92bddbcf2eeb6d627"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.9","digest":"sha256:e5a1569aeaf41820fa7bdee3e94468cae448133cdbf00119ad24f5b74db1ab9f","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.9@sha256:e5a1569aeaf41820fa7bdee3e94468cae448133cdbf00119ad24f5b74db1ab9f"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:0d9f1fb5fd6610c0ac1f5194a38e45a8a1e81f8a390d5142d8e4e6f26a4b3196","pinned_image":"ghcr.io/github/gh-aw-node@sha256:0d9f1fb5fd6610c0ac1f5194a38e45a8a1e81f8a390d5142d8e4e6f26a4b3196"},{"image":"ghcr.io/github/github-mcp-server:v1.9.0","digest":"sha256:881b53d6f75f69bdbc1b5b10fc2f1361717c19054143b3a8529fb5c32061a50e","pinned_image":"ghcr.io/github/github-mcp-server:v1.9.0@sha256:881b53d6f75f69bdbc1b5b10fc2f1361717c19054143b3a8529fb5c32061a50e"}]} # This file was automatically generated by gh-aw (v0.86.2). DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # diff --git a/.github/workflows/weekly-use-effect-review.md b/.github/workflows/weekly-use-effect-review.md index af9f2f43..1fc54da3 100644 --- a/.github/workflows/weekly-use-effect-review.md +++ b/.github/workflows/weekly-use-effect-review.md @@ -57,13 +57,26 @@ Classify the intent as `derive-render`, `handle-event`, `reset-or-adjust-state`, - Prefer `useSyncExternalStore` for external stores. - For data fetching kept in an Effect, handle cancellation or stale responses. - For a genuine external synchronization Effect, keep it only when dependencies - include every reactive value read by setup or cleanup, cleanup mirrors setup, - it tolerates Strict Mode, and it has no dependency suppression. + reflect the values that require resynchronization, cleanup mirrors setup, and + it tolerates Strict Mode. Treat dependency suppressions as investigation leads, + not sufficient evidence of incorrect behavior. + +Before changing an Effect, trace its callers and lifecycle. Establish which +values can actually change while the component remains mounted, including plugin +initialization, update hooks, and keyed remounts. Identify a concrete failure or +measurable unnecessary work, its reachable trigger, and how the change resolves +it. A dependency suppression alone does not justify a PR. + +Preserve setup/cleanup ownership: cleanup must release the resources and notify +the consumers associated with that setup. Explain any intentional callback +changes. Follow React's ref rules: do not introduce render-time ref assignments +or hide dependencies behind refs solely to satisfy lint. Only fix high-confidence violations. Each pull request must contain exactly one independent violation, start from the default branch, and use a branch named `automation/use-effect/`. Do not stack or combine pull requests. -Stop after five pull requests; leave remaining findings for a later run. +Zero findings is a successful run. Five pull requests is a maximum, not a target; +leave remaining findings for a later run. Do not change dependency manifests, lockfiles, workflow files, agent instructions, or other protected files. Do not create an issue or pull request @@ -74,7 +87,10 @@ Before proposing each fix, run the narrowest relevant check. Use `bun test` for logic covered by unit tests, `bun run types` for TypeScript changes, `bun run ci` for formatting and lint-sensitive changes, and `bun run e2e` only when the changed Effect alters browser behavior that cannot be covered without a browser. +Run verification that exercises the claimed failure or measures the unnecessary +work. Confirm any cited lint rule is enabled. If required tools or checks are +unavailable, report the limitation and create no PR. In each draft PR, explain the Effect's location and classification, why the old -Effect was incorrect, the selected refactor, and the verification command and -result. +Effect was incorrect, its reachable trigger, the relevant caller/lifecycle +evidence, the selected refactor, and the verification command and result. diff --git a/apps/server/src/storage/postgres/adapter.test.ts b/apps/server/src/storage/postgres/adapter.test.ts index e2adc311..2c502b5f 100644 --- a/apps/server/src/storage/postgres/adapter.test.ts +++ b/apps/server/src/storage/postgres/adapter.test.ts @@ -295,7 +295,7 @@ if (url) { let storage = new PostgresStorage(url); let sql = new SQL(url); let suffix = crypto.randomUUID(); - let locked = Promise.withResolvers(); + let locked = Promise.withResolvers(); let release = Promise.withResolvers(); try { await storage.migrate(); @@ -312,13 +312,15 @@ if (url) { createdBy: userId, now, }); - let lease = await storage.leases.acquire(`job-lock-${suffix}`, "old-writer", 50); + let lease = await storage.leases.acquire(`job-lock-${suffix}`, "old-writer", 1_000); let blocker = sql.begin(async transaction => { - await transaction`SELECT pg_advisory_xact_lock(2043237432)`; - locked.resolve(); + let [row] = await transaction<{ pid: number }[]>` + SELECT pg_backend_pid() AS pid, pg_advisory_xact_lock(2043237432) + `; + locked.resolve(row!.pid); await release.promise; }); - await locked.promise; + let blockerPid = await locked.promise; let enqueue = storage.jobs.enqueue({ id: `job-lock-job-${suffix}`, channelId, @@ -333,10 +335,29 @@ if (url) { now, lease: lease!, }); - await Bun.sleep(75); + let result = enqueue.then(() => undefined, error => error); + let waiting = false; + for (let attempt = 0; attempt < 100; attempt++) { + let [row] = await sql<{ waiting: boolean }[]>` + SELECT EXISTS ( + SELECT 1 FROM pg_stat_activity + WHERE ${blockerPid} = ANY(pg_blocking_pids(pid)) + ) AS waiting + `; + if (row!.waiting) { + waiting = true; + break; + } + await Bun.sleep(10); + } + expect(waiting).toBe(true); + await sql` + SELECT pg_sleep(GREATEST(0, EXTRACT(EPOCH FROM (expires_at - clock_timestamp())))) + FROM storage_leases WHERE name = ${lease!.name} + `; release.resolve(); await blocker; - await expect(enqueue).rejects.toMatchObject({ failure: "conflict" }); + expect(await result).toMatchObject({ failure: "conflict" }); expect((await storage.jobs.list(channelId, 10))!.jobs).toEqual([]); } finally { release.resolve();