Conversation
pgbus's hard parts are invariants, not APIs: a PG connection has one owner, admission control may only ever under-admit, a batch's counters and its execution rows have to agree at every commit point. Those facts lived in method comments, in a 92-entry cubic learnings export and in the reply threads of a dozen merged PRs — none of which a fresh session reads. This puts them where the next change has to walk past them. lode/ holds a summary of the system and its three non-negotiables, the repo's terminology, the practices .claude/rules does not state, a workflow profile for the shared /lode: skills, and thirteen subsystem summaries: client, active-job, concurrency, uniqueness, batch, event-bus, process, streams, web, configuration, schema, testing-and-ci and docs-site. Every line range, file count and line count in them was computed with a script, and every cited method and spec example name was verified to exist in the file it is attributed to (104 method citations, 143 file-plus-example-name pairs). lode/review/ holds 91 rules across eight files, split by sub-area: batch (18), concurrency (13), testing (13, including two "Not a bug" entries), uniqueness (11), web-and-i18n (11), client (10), process-and-streams (9) and event-bus-and-maintenance (6). They come from two sources — the 92 cubic learnings and the review threads of merged PRs #420, #456, #457, #458, #460, #463, #466 and #467 — and most rules carry both origins, because the learnings were distilled from those threads. Fourteen learnings collapsed into seven rules as exact duplicates (single-owner connections, shutdown join budgets, shutdown-contract tests, restoring shared overrides, const_source_location, persist-failed-dispatches, locks-page title parity). Three learnings did not survive verification and are recorded in the rollout's discrepancies file instead: 38c9758b is inverted by the current code (the uniqueness reaper now skips DLQ queues, PR #450), 2cb6a24e/d797d796 overstate it (only the ActiveJob executor writes pgbus_failed_events, never the event-bus consumer), and 39de03bb/f2659872 have no subject left in the tree — kept, but marked as a forward-looking constraint with no current caller. Nine doc-versus-code disagreements were found and deliberately not fixed here: the "Tailwind CDN" line in CLAUDE.md, the 800-line file limit four files exceed, the coverage floors that sit below the documented targets, and six smaller ones. The plugin supersedes seven local commands, now deleted: lfg, github-review-pr, github-review-failures, github-review-comments, finish-prs, tdd and plan. Everything those files knew — the CI job table, the conflict runbook, the lockfile rules, the PR-body shape — moved into lode/workflow.md, which the shared skills read. architect, perf, review-pr and security stay; debug-flaky never existed locally. CLAUDE.md's command table now names the /lode: commands and carries the Memory pointer, .claude/settings.json enables the plugin, and lode/tmp/ is gitignored. Claude-Session: https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF
…s, one uncited learning The retired github-review-pr's rule for vendor/turbo.js and apexcharts.js (never hand-merge, take one side whole) had vanished; the changelog heading list named a Removed section the file has never had and omitted Security; and cubic learning 84bcd3c2 (a retried archive is ambiguous, not a duplicate) lived only in a summary, so it now has a review rule of its own. Claude-Session: https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF
… DataSource writes Twenty generators, not twenty-one (migration_path.rb is a shared module). A bare rubocop run no longer crashes; .rubocop.yml has excluded docs at AllCops since PR #269, and the Rakefile and CI comments that say otherwise are stale. Web::DataSource's four write methods do not rescue; only the reads return neutral values. Claude-Session: https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 38 files
Confidence score: 4/5
lode/configuration/summary.mdandlode/review/event-bus-and-maintenance.mdcontain conflicting descriptions of shared connections andTRANSIENT_DROPversusClient::STALE_CONNECTION_PATTERNS, which could mislead maintenance of pooling and retry behavior—align the documentation withClient#initializeandstale_connection_retry.rb.lode/workflow.mdandlode/review/web-and-i18n.mdreference the wrong ApexCharts path and claim locale title corrections that are not present, making asset checks and translation follow-up unreliable—update the paths and verify all checked-in locale files.lode/review/batch.md,lode/active-job/summary.md, andlode/schema/summary.mdoverstate or misstate execution cleanup, uniqueness metadata, and the owned table inventory, which could lead to incorrect operational assumptions—reconcile these summaries withsweep_stale_executions,classify_stale,resolve_stale,Uniqueness.inject_metadata, and runtime-created tables.lode/web/summary.mdandlode/testing-and-ci/summary.mdhave incorrect view and test-suite counts, weakening confidence in the repository inventory and coverage reporting—recalculate counts using the documented directory and filename boundaries.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lode/configuration/summary.md">
<violation number="1" location="lode/configuration/summary.md:19">
P2: The third `connection_options` path is described as 'a Proc returning ActiveRecord's `raw_connection` — the shared path' that forces `pool_size: 1` and the serializing mutex in `Client#initialize`, but the code no longer does that by default. `#connection_options` calls `extract_ar_connection_hash`, which derives a Hash of connection params from ActiveRecord's `connection_db_config` so pgmq opens its own dedicated connections; the shared `raw_connection` Proc is only the rescue fallback when that extraction fails (it logs a 'NOT thread-safe' warning). Rewrite the bullet so the shared-Proc behavior is attributed to the fallback, not the default path — otherwise an agent or operator reading this will wrongly believe the standard Rails install runs on the serialized, non-thread-safe connection.</violation>
</file>
<file name="lode/review/batch.md">
<violation number="1" location="lode/review/batch.md:10">
P3: The rule title 'An execution row is removed only when its msg_id is still nil' is overbroad: the stale-execution sweep removes rows that do have a msg_id (sweep_stale_executions → classify_stale → resolve_stale → job_completed/job_discarded → resolve_execution → delete_all). Scope the claim to the orphan/un-count path, e.g. 'A row is un-counted (removed from the batch without resolution) only when its msg_id is still nil', so it does not contradict the stale-sweep behavior described below.</violation>
</file>
<file name="lode/web/summary.md">
<violation number="1" location="lode/web/summary.md:4">
P3: The view count is off by one: there are 33 `.erb` files under `app/views/pgbus/`, not 34. The only candidate for a 34th is `app/views/layouts/pgbus/application.html.erb`, which lives outside the counted directory. Since this memory file advertises that every count was computed and all other counts in this file verify exactly, an agent trusting this count to detect a missing view will chase a phantom. Correct it to 33, or state that the layout is included.</violation>
</file>
<file name="lode/review/event-bus-and-maintenance.md">
<violation number="1" location="lode/review/event-bus-and-maintenance.md:13">
P2: This rule claims TRANSIENT_DROP "is wider than Client::STALE_CONNECTION_PATTERNS", but the cited source comment in stale_connection_retry.rb says the opposite ("Deliberately narrower ... and for the opposite reason"). The two lists are actually incomparable: they share only "ssl syscall error", the client's six pre-checkout patterns are not in TRANSIENT_DROP, and TRANSIENT_DROP's mid-flight shapes are not in the client list. A future reader of this rule can't reconcile the two, and the gate may act on a wrong relative claim. Reword to state the two lists are scoped to different failure phases and are not a subset of one another, matching the source comment.</violation>
</file>
<file name="lode/active-job/summary.md">
<violation number="1" location="lode/active-job/summary.md:13">
P3: The claim that "Each writes one `pgbus_*` key" is inaccurate: `Uniqueness.inject_metadata` (lib/pgbus/uniqueness.rb) merges two keys into the payload — the uniqueness key and the strategy — not one. Since these lode summaries are used as authoritative rules by the pre-PR gate, correct the wording so a future agent doesn't assume only a single key is written per step.</violation>
</file>
<file name="lode/schema/summary.md">
<violation number="1" location="lode/schema/summary.md:8">
P3: The schema inventory says pgbus owns its `pgbus_*` tables as "installed by Rails migrations the generators write," and counts 16. There is a 17th `pgbus_*` table, `pgbus_pgmq_schema_versions`, created at runtime by `Pgbus::PgmqSchema`'s version-tracking SQL (and by the `upgrade_pgmq` migration). Since this file is durable agent memory meant to be a complete count, note that table separately so a future reader doesn't undercount the pgbus-owned schema.</violation>
</file>
<file name="lode/workflow.md">
<violation number="1" location="lode/workflow.md:53">
P2: Both tables point at `vendor/apexcharts.js`, but the vendored file lives at `app/frontend/pgbus/vendor/apexcharts.js` — there is no top-level `vendor/` copy, and `spec/pgbus/web/vendored_assets_spec.rb` verifies the `app/frontend/pgbus/vendor/apexcharts.js` path. An agent resolving a merge conflict on this vendored file will look in the wrong place. Fix the path in both the Layers row and the Conflicts row (`app/frontend/pgbus/vendor/turbo.js` is already correct).</violation>
</file>
<file name="lode/review/web-and-i18n.md">
<violation number="1" location="lode/review/web-and-i18n.md:52">
P2: This rule's "Holds because" claims `pgbus.locks.index.title` equals `pgbus.layout.nav.locks` in each locale and cites nl/it as already corrected, but the checked-in locale files contradict it: in all twelve locales the index title still translates "Concurrency" while the nav label translates "Locks" (nl nav "Vergrendelingen" vs title "Gelijktijdigheid"; it nav "Blocchi" vs title "Concorrenza"; en "Locks" vs "Concurrency"). If this is intended as a prescriptive rule, phrase it as a requirement for future edits rather than a state that holds; otherwise the rule asserts an invariant the current code violates, which will mislead agents running `/lode:gate`.</violation>
</file>
<file name="lode/testing-and-ci/summary.md">
<violation number="1" location="lode/testing-and-ci/summary.md:12">
P3: The unit suite has 210 spec files, not 211: `find spec/pgbus -name '*_spec.rb'` returns 210. The stated total of 323 only adds up with 210 (210+21+17+60+12+3 root specs), so the table row is both wrong and internally inconsistent with the summary's own total.</violation>
<violation number="2" location="lode/testing-and-ci/summary.md:16">
P3: The system suite has 12 `_spec.rb` files, not 16. The 16 counts the four non-spec support files under `spec/system/support/` as suite files, which is inconsistent with the other rows and with the summary's own total of 323 `_spec.rb` files (210+21+17+60+12+3 = 323).</violation>
<violation number="3" location="lode/testing-and-ci/summary.md:64">
P3: The RuboCop paragraph is mangled: 'A bare run discovers `docs/.rubocop.' is cut off, the next sentence restates the opening line ('RuboCop is given explicit paths by the Rakefile and CI.'), and only then does the paragraph continue with 'A bare run no longer crashes'. Rewrite it as one coherent paragraph — drop the dangling fragment and the repeated sentence.</violation>
</file>
Heads up: you’ve reached your flex budget. Increase your flex budget or wait for usage to reset.
Re-trigger cubic
Comment on lines
+19
to
+21
| 3. Otherwise a **Proc** returning ActiveRecord's `raw_connection` — the shared | ||
| path, which is what forces `pool_size: 1` and the serializing mutex in | ||
| `Client#initialize`. |
There was a problem hiding this comment.
P2: The third connection_options path is described as 'a Proc returning ActiveRecord's raw_connection — the shared path' that forces pool_size: 1 and the serializing mutex in Client#initialize, but the code no longer does that by default. #connection_options calls extract_ar_connection_hash, which derives a Hash of connection params from ActiveRecord's connection_db_config so pgmq opens its own dedicated connections; the shared raw_connection Proc is only the rescue fallback when that extraction fails (it logs a 'NOT thread-safe' warning). Rewrite the bullet so the shared-Proc behavior is attributed to the fallback, not the default path — otherwise an agent or operator reading this will wrongly believe the standard Rails install runs on the serialized, non-thread-safe connection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/configuration/summary.md, line 19:
<comment>The third `connection_options` path is described as 'a Proc returning ActiveRecord's `raw_connection` — the shared path' that forces `pool_size: 1` and the serializing mutex in `Client#initialize`, but the code no longer does that by default. `#connection_options` calls `extract_ar_connection_hash`, which derives a Hash of connection params from ActiveRecord's `connection_db_config` so pgmq opens its own dedicated connections; the shared `raw_connection` Proc is only the rescue fallback when that extraction fails (it logs a 'NOT thread-safe' warning). Rewrite the bullet so the shared-Proc behavior is attributed to the fallback, not the default path — otherwise an agent or operator reading this will wrongly believe the standard Rails install runs on the serialized, non-thread-safe connection.</comment>
<file context>
@@ -0,0 +1,108 @@
+ database.yml-style `:variables` block so `client_min_messages` and friends
+ reach pgmq's connections while a non-libpq key never reaches `PG.connect`
+ (issue #332).
+3. Otherwise a **Proc** returning ActiveRecord's `raw_connection` — the shared
+ path, which is what forces `pool_size: 1` and the serializing mutex in
+ `Client#initialize`.
</file context>
Suggested change
| 3. Otherwise a **Proc** returning ActiveRecord's `raw_connection` — the shared | |
| path, which is what forces `pool_size: 1` and the serializing mutex in | |
| `Client#initialize`. | |
| 3. Otherwise `extract_ar_connection_hash` derives a Hash of connection params | |
| from ActiveRecord's `connection_db_config` (host/port/dbname/user/password | |
| plus the `:variables` block), so pgmq opens its own dedicated connections. | |
| Only if that extraction fails does it fall back to a shared | |
| `-> { ...raw_connection }` Proc, logged as not thread-safe — that fallback | |
| alone forces `pool_size: 1` and the serializing mutex in | |
| `Client#initialize`. |
| - **Origin:** cubic learning f8cba24b; PR #467 | ||
|
|
||
| ### `TRANSIENT_DROP` is wider than the client's list, and includes `ssl syscall error` | ||
| - **Holds because:** libpq reports the same peer disappearance two ways — `unexpected eof while reading` from the TLS layer and `SSL SYSCALL error: EOF detected` from the syscall layer — so omitting the second re-raises on a drop the first would have retried. The list is `PQconsumeInput`, `server closed the connection unexpectedly`, `unexpected eof while reading`, `ssl syscall error`, matched against the error **and its cause**. It is wider than `Client::STALE_CONNECTION_PATTERNS` for the opposite reason that list is narrow: the client omits mid-flight shapes because a half-committed *enqueue* would duplicate a message. Same vocabulary, different safety argument — never widen one by citing the other. A refused or timed-out *connection* is an outage, not a drop, and is not retried. |
There was a problem hiding this comment.
P2: This rule claims TRANSIENT_DROP "is wider than Client::STALE_CONNECTION_PATTERNS", but the cited source comment in stale_connection_retry.rb says the opposite ("Deliberately narrower ... and for the opposite reason"). The two lists are actually incomparable: they share only "ssl syscall error", the client's six pre-checkout patterns are not in TRANSIENT_DROP, and TRANSIENT_DROP's mid-flight shapes are not in the client list. A future reader of this rule can't reconcile the two, and the gate may act on a wrong relative claim. Reword to state the two lists are scoped to different failure phases and are not a subset of one another, matching the source comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/event-bus-and-maintenance.md, line 13:
<comment>This rule claims TRANSIENT_DROP "is wider than Client::STALE_CONNECTION_PATTERNS", but the cited source comment in stale_connection_retry.rb says the opposite ("Deliberately narrower ... and for the opposite reason"). The two lists are actually incomparable: they share only "ssl syscall error", the client's six pre-checkout patterns are not in TRANSIENT_DROP, and TRANSIENT_DROP's mid-flight shapes are not in the client list. A future reader of this rule can't reconcile the two, and the gate may act on a wrong relative claim. Reword to state the two lists are scoped to different failure phases and are not a subset of one another, matching the source comment.</comment>
<file context>
@@ -0,0 +1,40 @@
+- **Origin:** cubic learning f8cba24b; PR #467
+
+### `TRANSIENT_DROP` is wider than the client's list, and includes `ssl syscall error`
+- **Holds because:** libpq reports the same peer disappearance two ways — `unexpected eof while reading` from the TLS layer and `SSL SYSCALL error: EOF detected` from the syscall layer — so omitting the second re-raises on a drop the first would have retried. The list is `PQconsumeInput`, `server closed the connection unexpectedly`, `unexpected eof while reading`, `ssl syscall error`, matched against the error **and its cause**. It is wider than `Client::STALE_CONNECTION_PATTERNS` for the opposite reason that list is narrow: the client omits mid-flight shapes because a half-committed *enqueue* would duplicate a message. Same vocabulary, different safety argument — never widen one by citing the other. A refused or timed-out *connection* is an outage, not a drop, and is not retried.
+- **Where:** `lib/pgbus/event_bus/stale_connection_retry.rb` (`TRANSIENT_DROP`, `.transient_drop?`); `lib/pgbus/client.rb` (`STALE_CONNECTION_PATTERNS`)
+- **Proven by:** `spec/pgbus/event_bus/stale_connection_retry_spec.rb`
</file context>
Suggested change
| - **Holds because:** libpq reports the same peer disappearance two ways — `unexpected eof while reading` from the TLS layer and `SSL SYSCALL error: EOF detected` from the syscall layer — so omitting the second re-raises on a drop the first would have retried. The list is `PQconsumeInput`, `server closed the connection unexpectedly`, `unexpected eof while reading`, `ssl syscall error`, matched against the error **and its cause**. It is wider than `Client::STALE_CONNECTION_PATTERNS` for the opposite reason that list is narrow: the client omits mid-flight shapes because a half-committed *enqueue* would duplicate a message. Same vocabulary, different safety argument — never widen one by citing the other. A refused or timed-out *connection* is an outage, not a drop, and is not retried. | |
| It is not comparable to `Client::STALE_CONNECTION_PATTERNS` — the two lists are scoped to different failure phases and neither is a subset of the other (they share only `ssl syscall error`). The client covers pre-checkout shapes and omits mid-flight ones because a half-committed *enqueue* would duplicate a message; this list covers the socket dying under the idempotent stamp. |
| | Migrations | `lib/generators/pgbus/templates/*.erb` | owned here; a released migration template is append-only — add a new `add_*` generator rather than editing one apps have run | | ||
| | Vendored PGMQ SQL | `lib/pgbus/pgmq_schema/pgmq_v*.sql` | **vendored — never edit**; add a new version file copied verbatim from upstream | | ||
| | Built asset | `app/frontend/pgbus/style.css` | **generated** — edit the views, then `rake frontend:css` | | ||
| | Vendored JS | `app/frontend/pgbus/vendor/turbo.js`, `vendor/apexcharts.js` | **vendored** — never edit; re-vendor from upstream | |
There was a problem hiding this comment.
P2: Both tables point at vendor/apexcharts.js, but the vendored file lives at app/frontend/pgbus/vendor/apexcharts.js — there is no top-level vendor/ copy, and spec/pgbus/web/vendored_assets_spec.rb verifies the app/frontend/pgbus/vendor/apexcharts.js path. An agent resolving a merge conflict on this vendored file will look in the wrong place. Fix the path in both the Layers row and the Conflicts row (app/frontend/pgbus/vendor/turbo.js is already correct).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/workflow.md, line 53:
<comment>Both tables point at `vendor/apexcharts.js`, but the vendored file lives at `app/frontend/pgbus/vendor/apexcharts.js` — there is no top-level `vendor/` copy, and `spec/pgbus/web/vendored_assets_spec.rb` verifies the `app/frontend/pgbus/vendor/apexcharts.js` path. An agent resolving a merge conflict on this vendored file will look in the wrong place. Fix the path in both the Layers row and the Conflicts row (`app/frontend/pgbus/vendor/turbo.js` is already correct).</comment>
<file context>
@@ -0,0 +1,142 @@
+| Migrations | `lib/generators/pgbus/templates/*.erb` | owned here; a released migration template is append-only — add a new `add_*` generator rather than editing one apps have run |
+| Vendored PGMQ SQL | `lib/pgbus/pgmq_schema/pgmq_v*.sql` | **vendored — never edit**; add a new version file copied verbatim from upstream |
+| Built asset | `app/frontend/pgbus/style.css` | **generated** — edit the views, then `rake frontend:css` |
+| Vendored JS | `app/frontend/pgbus/vendor/turbo.js`, `vendor/apexcharts.js` | **vendored** — never edit; re-vendor from upstream |
+| Lockfiles | `Gemfile.lock`, `gemfiles/rails_7_1.gemfile.lock`, `docs/Gemfile.lock`, `bun.lock`, `docs/bun.lock` | **generated** — never hand-merge; see Conflicts |
+| Docs site | `docs/**` | separate app, separate bundle, separate lint; see `lode/docs-site/summary.md` |
</file context>
| - **Proven by:** `spec/system/batches_spec.rb` — the "with a data source that omits pending_jobs" context | ||
| - **Origin:** cubic learning 2f07b6f5; PR #458 | ||
|
|
||
| ### A page heading uses that locale's own nav term, and a new key is defined in all twelve locales |
There was a problem hiding this comment.
P2: This rule's "Holds because" claims pgbus.locks.index.title equals pgbus.layout.nav.locks in each locale and cites nl/it as already corrected, but the checked-in locale files contradict it: in all twelve locales the index title still translates "Concurrency" while the nav label translates "Locks" (nl nav "Vergrendelingen" vs title "Gelijktijdigheid"; it nav "Blocchi" vs title "Concorrenza"; en "Locks" vs "Concurrency"). If this is intended as a prescriptive rule, phrase it as a requirement for future edits rather than a state that holds; otherwise the rule asserts an invariant the current code violates, which will mislead agents running /lode:gate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/web-and-i18n.md, line 52:
<comment>This rule's "Holds because" claims `pgbus.locks.index.title` equals `pgbus.layout.nav.locks` in each locale and cites nl/it as already corrected, but the checked-in locale files contradict it: in all twelve locales the index title still translates "Concurrency" while the nav label translates "Locks" (nl nav "Vergrendelingen" vs title "Gelijktijdigheid"; it nav "Blocchi" vs title "Concorrenza"; en "Locks" vs "Concurrency"). If this is intended as a prescriptive rule, phrase it as a requirement for future edits rather than a state that holds; otherwise the rule asserts an invariant the current code violates, which will mislead agents running `/lode:gate`.</comment>
<file context>
@@ -0,0 +1,68 @@
+- **Proven by:** `spec/system/batches_spec.rb` — the "with a data source that omits pending_jobs" context
+- **Origin:** cubic learning 2f07b6f5; PR #458
+
+### A page heading uses that locale's own nav term, and a new key is defined in all twelve locales
+- **Holds because:** a heading that reads as a different concept than the tab that led to it makes the page look like the wrong page. `pgbus.locks.index.title` must equal `pgbus.layout.nav.locks` in each locale (`nl` said "Vergrendelingen" in the nav and something else in the heading; `it` said "Blocchi"). The same completeness rule applies to every new key: the batch view uses the canonical `pgbus.batches.show.failed` and `pgbus.batches.show.on_failure` keys, and both are defined in `da de en es fi fr it ja nb nl pt sv`.
+- **Where:** `config/locales/*.yml`; `app/views/pgbus/locks/index.html.erb`; `app/views/pgbus/batches/show.html.erb`
</file context>
| - **Proven by:** `spec/pgbus/active_job/adapter_spec.rb:"counts the job into its batch before the message is sent"`, `:"raises AlreadyFinished before sending when the batch has finished"`, `:"tags every bulk payload with the batch id and counts them once, before the send"`; `spec/integration/batch_open_spec.rb:"raises at perform_later and sends nothing when the batch finished under a stale handle"` | ||
| - **Origin:** issue #423; PR #420 | ||
|
|
||
| ### An execution row is removed only when its `msg_id` is still nil |
There was a problem hiding this comment.
P3: The rule title 'An execution row is removed only when its msg_id is still nil' is overbroad: the stale-execution sweep removes rows that do have a msg_id (sweep_stale_executions → classify_stale → resolve_stale → job_completed/job_discarded → resolve_execution → delete_all). Scope the claim to the orphan/un-count path, e.g. 'A row is un-counted (removed from the batch without resolution) only when its msg_id is still nil', so it does not contradict the stale-sweep behavior described below.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/review/batch.md, line 10:
<comment>The rule title 'An execution row is removed only when its msg_id is still nil' is overbroad: the stale-execution sweep removes rows that do have a msg_id (sweep_stale_executions → classify_stale → resolve_stale → job_completed/job_discarded → resolve_execution → delete_all). Scope the claim to the orphan/un-count path, e.g. 'A row is un-counted (removed from the batch without resolution) only when its msg_id is still nil', so it does not contradict the stale-sweep behavior described below.</comment>
<file context>
@@ -0,0 +1,110 @@
+- **Proven by:** `spec/pgbus/active_job/adapter_spec.rb:"counts the job into its batch before the message is sent"`, `:"raises AlreadyFinished before sending when the batch has finished"`, `:"tags every bulk payload with the batch id and counts them once, before the send"`; `spec/integration/batch_open_spec.rb:"raises at perform_later and sends nothing when the batch finished under a stale handle"`
+- **Origin:** issue #423; PR #420
+
+### An execution row is removed only when its `msg_id` is still nil
+- **Holds because:** a row stays `msg_id`-less both when the enqueue died before the send **and** when the send landed and only the backfill failed — and in the second case the message is live and must not be un-counted. The sweep's removal is a CAS (`where(id: row.id, msg_id: nil).delete_all`) and the `total_jobs` decrement plus finish check run only when it deleted something. The bulk rescue untracks only the payloads whose id is actually nil (`msg_ids.nil? || msg_ids[index].nil?`), so a short `send_batch` response keeps the rows for messages that did land.
+- **Where:** `lib/pgbus/batch/sweep.rb#uncount_orphan!`, `#classify_orphan`; `lib/pgbus/active_job/adapter.rb#enqueue_immediate` (rescue `Pgbus::EnqueueError`)
</file context>
Suggested change
| ### An execution row is removed only when its `msg_id` is still nil | |
| ### A row is un-counted from the batch only when its `msg_id` is still nil |
| `Adapter#enqueue` and `#enqueue_at` build the payload the same way, in a fixed | ||
| order: `Serializer.serialize_job_hash`, then `Concurrency.inject_metadata`, | ||
| `Uniqueness.inject_metadata`, `FairShare.inject_metadata`, and finally | ||
| `#inject_batch_metadata`. Each writes one `pgbus_*` key into the payload hash; |
There was a problem hiding this comment.
P3: The claim that "Each writes one pgbus_* key" is inaccurate: Uniqueness.inject_metadata (lib/pgbus/uniqueness.rb) merges two keys into the payload — the uniqueness key and the strategy — not one. Since these lode summaries are used as authoritative rules by the pre-PR gate, correct the wording so a future agent doesn't assume only a single key is written per step.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/active-job/summary.md, line 13:
<comment>The claim that "Each writes one `pgbus_*` key" is inaccurate: `Uniqueness.inject_metadata` (lib/pgbus/uniqueness.rb) merges two keys into the payload — the uniqueness key and the strategy — not one. Since these lode summaries are used as authoritative rules by the pre-PR gate, correct the wording so a future agent doesn't assume only a single key is written per step.</comment>
<file context>
@@ -0,0 +1,90 @@
+`Adapter#enqueue` and `#enqueue_at` build the payload the same way, in a fixed
+order: `Serializer.serialize_job_hash`, then `Concurrency.inject_metadata`,
+`Uniqueness.inject_metadata`, `FairShare.inject_metadata`, and finally
+`#inject_batch_metadata`. Each writes one `pgbus_*` key into the payload hash;
+nothing else may.
+
</file context>
Suggested change
| `#inject_batch_metadata`. Each writes one `pgbus_*` key into the payload hash; | |
| `#inject_batch_metadata`. Each writes `pgbus_*` metadata into the payload hash (Concurrency, FairShare and batch each add one key; Uniqueness adds the key plus the strategy); |
| first is installed by the gem at runtime; the second by Rails migrations the | ||
| generators write. | ||
|
|
||
| ## The 16 `pgbus_*` tables |
There was a problem hiding this comment.
P3: The schema inventory says pgbus owns its pgbus_* tables as "installed by Rails migrations the generators write," and counts 16. There is a 17th pgbus_* table, pgbus_pgmq_schema_versions, created at runtime by Pgbus::PgmqSchema's version-tracking SQL (and by the upgrade_pgmq migration). Since this file is durable agent memory meant to be a complete count, note that table separately so a future reader doesn't undercount the pgbus-owned schema.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/schema/summary.md, line 8:
<comment>The schema inventory says pgbus owns its `pgbus_*` tables as "installed by Rails migrations the generators write," and counts 16. There is a 17th `pgbus_*` table, `pgbus_pgmq_schema_versions`, created at runtime by `Pgbus::PgmqSchema`'s version-tracking SQL (and by the `upgrade_pgmq` migration). Since this file is durable agent memory meant to be a complete count, note that table separately so a future reader doesn't undercount the pgbus-owned schema.</comment>
<file context>
@@ -0,0 +1,85 @@
+first is installed by the gem at runtime; the second by Rails migrations the
+generators write.
+
+## The 16 `pgbus_*` tables
+
+`lib/generators/pgbus/templates/` holds 22 templates (`migration.rb.erb` for a
</file context>
Suggested change
| ## The 16 `pgbus_*` tables | |
| ## The 16 `pgbus_*` tables (plus `pgbus_pgmq_schema_versions`, created by the PGMQ install's version-tracking SQL) |
| | generators | `spec/generators/` (21) | `rails_helper.rb` | nothing | | ||
| | requests | `spec/requests/` (17) | `rails_helper.rb` | the dummy app | | ||
| | integration | `spec/integration/` (60) | `integration_helper.rb` | `PGBUS_DATABASE_URL` — a real PostgreSQL with PGMQ | | ||
| | system | `spec/system/` (16) | `system_helper.rb` | the dummy app + Playwright/Chromium | |
There was a problem hiding this comment.
P3: The system suite has 12 _spec.rb files, not 16. The 16 counts the four non-spec support files under spec/system/support/ as suite files, which is inconsistent with the other rows and with the summary's own total of 323 _spec.rb files (210+21+17+60+12+3 = 323).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/testing-and-ci/summary.md, line 16:
<comment>The system suite has 12 `_spec.rb` files, not 16. The 16 counts the four non-spec support files under `spec/system/support/` as suite files, which is inconsistent with the other rows and with the summary's own total of 323 `_spec.rb` files (210+21+17+60+12+3 = 323).</comment>
<file context>
@@ -0,0 +1,98 @@
+| generators | `spec/generators/` (21) | `rails_helper.rb` | nothing |
+| requests | `spec/requests/` (17) | `rails_helper.rb` | the dummy app |
+| integration | `spec/integration/` (60) | `integration_helper.rb` | `PGBUS_DATABASE_URL` — a real PostgreSQL with PGMQ |
+| system | `spec/system/` (16) | `system_helper.rb` | the dummy app + Playwright/Chromium |
+
+`spec/support/` (11 files) holds the shared doubles — `PgmqDoubles`,
</file context>
Suggested change
| | system | `spec/system/` (16) | `system_helper.rb` | the dummy app + Playwright/Chromium | | |
| | system | `spec/system/` (12) | `system_helper.rb` | the dummy app + Playwright/Chromium | |
|
|
||
| | Suite | Files | Entry | Needs | | ||
| |---|---|---|---| | ||
| | unit | `spec/pgbus/` (211) | `spec_helper.rb` | nothing; PGMQ is doubled | |
There was a problem hiding this comment.
P3: The unit suite has 210 spec files, not 211: find spec/pgbus -name '*_spec.rb' returns 210. The stated total of 323 only adds up with 210 (210+21+17+60+12+3 root specs), so the table row is both wrong and internally inconsistent with the summary's own total.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/testing-and-ci/summary.md, line 12:
<comment>The unit suite has 210 spec files, not 211: `find spec/pgbus -name '*_spec.rb'` returns 210. The stated total of 323 only adds up with 210 (210+21+17+60+12+3 root specs), so the table row is both wrong and internally inconsistent with the summary's own total.</comment>
<file context>
@@ -0,0 +1,98 @@
+
+| Suite | Files | Entry | Needs |
+|---|---|---|---|
+| unit | `spec/pgbus/` (211) | `spec_helper.rb` | nothing; PGMQ is doubled |
+| generators | `spec/generators/` (21) | `rails_helper.rb` | nothing |
+| requests | `spec/requests/` (17) | `rails_helper.rb` | the dummy app |
</file context>
Suggested change
| | unit | `spec/pgbus/` (211) | `spec_helper.rb` | nothing; PGMQ is doubled | | |
| | unit | `spec/pgbus/` (210) | `spec_helper.rb` | nothing; PGMQ is doubled | |
Comment on lines
+64
to
+65
| `docs/.rubocop.RuboCop is given explicit paths by the Rakefile and CI. A bare run no longer crashes: `.rubocop.yml` excludes `docs` and `docs/**/*` at `AllCops` (PR #269) so `docs/.rubocop.yml` is never read; the comments in the Rakefile and `main.yml` that still describe a crash predate that exclusion. `docs/` lints itself in | ||
| `docs-ci.yml`. |
There was a problem hiding this comment.
P3: The RuboCop paragraph is mangled: 'A bare run discovers `docs/.rubocop.' is cut off, the next sentence restates the opening line ('RuboCop is given explicit paths by the Rakefile and CI.'), and only then does the paragraph continue with 'A bare run no longer crashes'. Rewrite it as one coherent paragraph — drop the dangling fragment and the repeated sentence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lode/testing-and-ci/summary.md, line 64:
<comment>The RuboCop paragraph is mangled: 'A bare run discovers `docs/.rubocop.' is cut off, the next sentence restates the opening line ('RuboCop is given explicit paths by the Rakefile and CI.'), and only then does the paragraph continue with 'A bare run no longer crashes'. Rewrite it as one coherent paragraph — drop the dangling fragment and the repeated sentence.</comment>
<file context>
@@ -0,0 +1,98 @@
+| `system_test` | Playwright Chromium (cached on `bun.lock`), `rspec spec/system/`, uploads `tmp/capybara/**/*.png` on failure |
+
+RuboCop is always given **explicit paths**. A bare run discovers
+`docs/.rubocop.RuboCop is given explicit paths by the Rakefile and CI. A bare run no longer crashes: `.rubocop.yml` excludes `docs` and `docs/**/*` at `AllCops` (PR #269) so `docs/.rubocop.yml` is never read; the comments in the Rakefile and `main.yml` that still describe a crash predate that exclusion. `docs/` lints itself in
+`docs-ci.yml`.
+
</file context>
Suggested change
| `docs/.rubocop.RuboCop is given explicit paths by the Rakefile and CI. A bare run no longer crashes: `.rubocop.yml` excludes `docs` and `docs/**/*` at `AllCops` (PR #269) so `docs/.rubocop.yml` is never read; the comments in the Rakefile and `main.yml` that still describe a crash predate that exclusion. `docs/` lints itself in | |
| `docs-ci.yml`. | |
| RuboCop is always given **explicit paths**. A bare run no longer crashes: `.rubocop.yml` excludes `docs` and `docs/**/*` at `AllCops` (PR #269) so `docs/.rubocop.yml` is never read; the comments in the Rakefile and `main.yml` that still describe a crash predate that exclusion. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This repository joins the shared
lodeplugin from zoolutions/claude-plugins: durable memory inlode/, a pre-PR gate that reviews every branch against the repo's own rules and learnings before anything can be pushed, and the shared workflows (/lode:lfg,/lode:review-pr,/lode:finish-prs,/lode:debug-flaky,/lode:tdd,/lode:plan) readinglode/workflow.md.lode/— summary, terminology, practices, map, and thirteen subsystem summaries (client, ActiveJob, concurrency, uniqueness, batch, event bus, process, streams, web, configuration, schema, testing and CI, docs site). Every claim cites a file and method; every count was computed (185 files inlib/, 17 controllers, 13 error classes, 7 stale-connection patterns, the 7,999-byte NOTIFY cap, 20 generators).lode/review/— 92 rules in nine files: all 92 cubic learnings for this repo (14 merged into 7 where they were the same finding) plus the review threads of merged PRs feat(batches): execution-row completion tracking, stalled-batch sweep, on_failure rename #420, fix(web): bound the health server's per-connection reads #456, fix(client): retry notify-insert deadlocks on stream-queue setup #457, fix(web): rebuild the drifted Tailwind artifact, auto-refresh batches #458, fix(concurrency): make on_conflict: :block durable and never over-admit #460, feat(web): surface concurrency keys on the Locks page, in metrics and MCP #463, feat(streams): forward coalesce: on the Turbo::StreamsChannel broadcast path #466 and fix(event_bus): retry the claim stamp when the AR socket drops under it #467. Three learnings the code now contradicts were rewritten rather than copied and say so: DLQ queues skip the uniqueness-key check (PR fix(uniqueness): reaper ignores dead-letter queues when probing unbound locks #450),FailedEventRecorder.record!has exactly one caller, andconst_source_locationhas no caller yet. 143 spec citations were checked against the file they name; twelve that pointed atbatch_spec.rbactually live inbatch_execution_spec.rband were corrected.lode/workflow.md(140 lines) — the profile.lfg,github-review-pr,github-review-failures,github-review-comments,finish-prs,tdd,plan. Kept:architect,perf,review-pr,security. New.claude/settings.jsonenablinglode@zoolutions;CLAUDE.mdgains a Memory section and the new command rows;.gitignoregainslode/tmp/.No Ruby changes. Nine doc-versus-code gaps are recorded in the lode where they apply; the ones worth a follow-up:
CLAUDE.md's "Tailwind CDN, zero npm" claim (two specs enforce the opposite), the 800-line file limit that five files exceed (client.rb2,134,configuration.rb1,794,data_source.rb1,786,supervisor.rb983,worker.rb883), the coverage floors (89/75 enforced against 80/100 documented), and the Rakefile andmain.ymlcomments that still say a barerubocopcrashes ondocs/when the exclusion that prevents it landed in #269.Test plan
rake -T,rspec --version,rubocop --version,bin/release list,bundle exec exe/pgbus help; the suite needs Postgres and was not run; the gate ran a barebundle exec rubocopto disprove the crash claim/lode:lfgon the stale-comment cleanup reads the profile and reaches the gateDeviations & judgment calls
/lode:seed, resumed once after a rate limit (every draft re-verified;summary.mdandterminology.mdcorrected, including an ephemeral-NOTIFY claim that had the fallback backwards), then gated here.LODE_SKIP_GATE=1after the gate passed: the orchestrating session runs the cached 0.1.0 push hook keyed on a different checkout. A session opened here will not need it.Gate
Rounds: 1. Findings: 1 P1 fixed, 5 P2 fixed, 1 P3. Rejected 0, deferred 0.
https://claude.ai/code/session_01Hga3idYBp7fWdj7GmmfofF
Summary by cubic
Seeds durable project memory in
lode/and enables the sharedlode@zoolutionspre-PR gate, so every push is reviewed against the repo's own rules and learnings.New memory
lode/holds a system summary, terminology, practices, a workflow profile, and 13 subsystem summaries, with all line counts and method citations verified against the code.lode/review/holds 92 rules distilled from prior learnings and the review threads of merged PRs; 14 duplicates were merged into 7, and 3 rules the code now contradicts were rewritten.Gate and commands
.claude/settings.jsonenables the shared plugin, and the pre-PR gate now runs every branch againstlode/before anything can be pushed.lfg,github-review-pr,github-review-failures,github-review-comments,finish-prs,tdd,plan);lode/workflow.mdcarries what they knew, whilearchitect,perf,review-pr, andsecuritystay.CLAUDE.mdgains a Memory section and the new command rows, and.gitignoregainslode/tmp/.Written for commit 97698e0. Summary will update on new commits.