Skip to content

fix: shared advisory-lock session and seven open bugs (#2169) - #2240

Merged
cjimti merged 1 commit into
mainfrom
fix/2169-open-bugs
Oct 21, 2026
Merged

cjimti merged 1 commit into
mainfrom
fix/2169-open-bugs

Conversation

@cjimti

@cjimti cjimti commented Oct 21, 2026

Copy link
Copy Markdown
Member

Description

Changes the advisory locks every background loop takes (new internal/advisorylock), /readyz, the database pool floor, the storage forecast, manage_asset update and patch, the call catalog's outcome filter (GET /api/v1/admin/calls, GET /api/v1/portal/calls), full-text matching on seven search surfaces and memory (migration 000209), API and GraphQL operation ranking (internal/opranking), the Google API refusal hint on api_invoke_endpoint, the api and graphql connection editors, and the deployment guide's image pins. Eight tickets:

  1. v1.147.0: advisory-locked loops pin pooled sessions; with a small max_open_conns the lock-holding replica starves (every DB call times out) #2169 a replica starved its own database pool. Each advisory-locked loop held a pooled connection for its whole pass while the pass drew its queries from the same pool; with max_open_conns: 5, four loops held four connections and nothing finished, while /readyz stayed 200.
    • Every try-lock held across work now goes on one database session per process (internal/advisorylock): the catalog scan, maps, storage and webhook alerts, retention, the capacity listing, the call catalog and audit sweeps, thumbnail sweeps, catalog import, resource reclassification, connlimit slots and the connoauth refresher. However many locks a replica holds, they pin one connection, which goes back to the pool with the last lock. The locked passes run on the pool.
    • PostgreSQL grants a session a key it already holds, so the session refuses a key its own process holds, keeping two goroutines of one process exclusive as two replicas are. A lock or unlock statement runs on a context detached from its caller, so a deadline reached mid-statement cannot end the session. A session whose connection broke, or whose unlock failed, is ended, which releases its locks, and the next take opens another.
    • .semgrep/go-advisory-lock.yml refuses a pg_try_advisory_lock taken anywhere else. connoauth's blocking per-call Store.Lock stays on a connection of its own, since a lock that waits cannot share a session.
    • database.max_open_conns below 4 is refused at startup, naming the minimum (dbobs.CheckPoolSize).
    • /readyz answers 503 database_pool_starved when every connection of the pool has been in use, with none handed out within 500ms, for 30 seconds. A database that is down opens no connections and does not trip it.
  2. Storage forecast on a fresh install: a first-boot burst becomes a fill date and can raise exhaustion alerts; both replicas sample every tick #2203 storage forecast on a fresh install. A store's first day (from its first kept sample, when none was pruned) is left out of every growth fit; the 7-day rate then waits for a day of history and the 30-day rate for two weeks, so the 30-day rate never repeats the 7-day one over the same days. A replica that finds a sample taken within nine tenths of the interval takes none, so each tick is sampled once. Items 1 and 3 of the ticket were largely in place from Storage page on a fresh install: a first-boot burst is projected as daily growth, a flat store shows a negative slope, and sub-byte rates render as "undefined" #2201; the 30-day minimum moves from 7 to 14 days.
  3. fix(portal): a tile can be drawn before an update's new references are granted #2234 a tile drawn before an update's references were granted. manage_asset update and patch record the newly declared references beside the asset's existing ones before the new version exists (Declarer.Widen), apply the exact declared set once the content is stored, and put the references back when the content write fails. A kept reference keeps its token.
  4. admin calls: the outcome filter takes 36s on a 95k-record catalog #2231 the outcome filter took 36 seconds. The outcome is derived per record, and the satisfied rule probed two indexes for every record. Now:
    • a count states the outcome as the conditions the CASE reaches it by, asks the satisfied rule only of calls a memory record or asset capture names (citedSet, joined by hash), and reads supersession from one pass over the reads in (created_at, event_id) order;
    • a page of satisfied calls joins the cited set; every other page stays newest first and stops at a page of matches;
    • list and count run in a read-only transaction with JIT off (the compile alone took 1.5s);
    • migration 000208 adds idx_call_records_supersession.
      On the dev catalog (111,919 records) every outcome answers in 0.32s or less, from 3.6 to 4.7s.
  5. search: an unembedded exact match ranks below similar-looking files #2230 a file was not found by the words of its name. The ticket's stated cause was not what failed: the resource ranker already scored an unembedded match 0.7, above any non-matching file (0.6 at most). PostgreSQL's parser read acceptance-1627-1792487495133911000 as accept, -1627 and -1792487495133911000, so a search for acceptance 1627 1792487495133911000 never matched the file lexically, and q3-2024-report.csv was one token.
    • Migration 000209 reads -, _, ., / and : as word breaks, beside the text as written, in the names, titles, paths, file names, tags and categories of the resource, asset, collection, prompt, knowledge page, script and catalog dataset search functions, in the memory content index and in the folder search, and rebuilds those indexes.
    • The ranking defect the ticket described does exist in internal/opranking (API and GraphQL operations), where an unembedded match scored 0.4 and lost to any near neighbour above 0.67; it is now blended at a cosine of zero, as every SQL ranker blends a row the vector arm cannot see.
  6. docs: deployment guide pins three images to digests that do not exist #2224 three image digests in the deployment guide did not exist. The compose example now pins pgvector/pgvector:pg16 (the platform's migrations create the vector extension, which postgres:16-alpine lacks), Keycloak 26.7.5 with its KC_BOOTSTRAP_ADMIN_* variables, and Trino 453, each to the digest its registry serves. make docs-images-check (scripts/docs-image-digests.sh) resolves every image the docs pin to a digest and runs in the E2E Nightly, where network is available; it refuses all three of the old digests.
  7. fix(upstreamauth): read Google API refusals by status, not product text #2237 Google API refusals read by status, not product text. googlekey.APIHint chooses the hint from error.status and error.details[].reason: SERVICE_DISABLED enables the API, ACCESS_TOKEN_SCOPE_INSUFFICIENT adds the scope to oauth_scope, any other 401 or 403 in Google's error shape grants the service account access inside the product. No code, test or doc names a product's message text; the ticket's existing test sample was one, so the 401 samples carry other messages.
  8. feat(portal): token lifetime field on api and graphql connection forms #2236 Token lifetime on the connection editors. The api and graphql editors show Token lifetime ("Used when the token endpoint states no expiry") under the OAuth block, writing oauth_token_lifetime; an empty field stores nothing and the 15-minute default applies; a negative value is refused with the server's message.

Also: certifi 2026.10.19 in docs/requirements.txt (the osv gate flagged 2026.6.17); the import ratchet gains the internal/advisorylock edges and pkg/audit/postgres -> internal/logsan.

Upgrading

  • Migration 000209 rebuilds seven full-text indexes and the memory content index, and 000208 builds one index on call_records. Each runs once, in the migration, and takes time in proportion to those tables.
  • A configuration with database.max_open_conns of 1 to 3 is refused at startup.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Test improvement (new or updated tests)
  • Documentation update
  • Stability/performance improvement

Related Issues

Closes #2169
Closes #2203
Closes #2234
Closes #2231
Closes #2230
Closes #2224
Closes #2237
Closes #2236

Filed from this work: #2239 (make preverify runs migrate-check when a migration changes).

Readers of this state

  • /readyz gains the 503 status database_pool_starved. Read by readiness probes; no portal page reads it.
  • GET /api/v1/admin/storage: rate_7d_min_history_hours and rate_30d_min_history_hours now count a store's first day (48 and 360 when its whole history is kept, 24 and 336 otherwise), and a 30-day rate appears after two weeks. The Storage page reads them unchanged ("Not enough history, x of y").
  • GET /api/v1/admin/calls and /api/v1/portal/calls return the same shape; only their speed changes.
  • api_invoke_endpoint's hint on a Google service-account connection gains the scope step and now answers every 401/403 in Google's shape.
  • Connection config oauth_token_lifetime is now set from the api and graphql editors (ApiGatewayAuthFields); the MSW mock's acme-billing-api carries it.
  • internal/apidocs/swagger.json is unchanged.

Testing

  • Acceptance through the real surface on the local dev stack (two replicas behind the proxy), per ticket after the last change to each; every criterion passed: 2169 (2/2), 2203 (2/2), 2231 (1/1), 2230 (2/2), 2234 (2/2), 2236 (1/1, api and graphql), 2237 (1/1, against the real Google Sheets API). Transcripts are in build/<n>/acceptance.md; make acceptance-check confirms every changed acceptance file's criteria passed in their recorded run.

  • Real-Postgres:

    • advisorylock: six locked passes finish on a pool of five; the storage and webhook alert locks, two connlimit slots and a refresh lock share one connection on a pool of four.
    • storagecap: two replicas sample one tick once.
    • callrecord: on a 100,000-record catalog where nearly every call is a read in a session, every outcome's page and count answer within a second (slowest 0.47s, best of three), and each outcome counts exactly what the original rule asked of every record counts.
    • resource search: the unembedded acceptance-1627-… file is the first hit among 30 embedded look-alikes, hybrid and lexical.
    • The RealDB suites of every surface whose full-text function changed, make migrate-check on pg16 and pg17, and the SQL statement gate.
  • UI: ApiGatewayAuthFields tests, the connections-oauth Playwright spec against MSW, and light and dark screenshots (admin-admin-connection-token-lifetime).

  • make verify passed: the full Go race and coverage run (total 94.0%, patch 96.1%), the schedule lane, the real-database lane, the full vitest suite (3,236) and Playwright (296).

  • One early run of TestRealDB_LockedPassesFinishOnASmallPool failed and its output was not captured; every run since, including 100 runs of the advisorylock RealDB tests at -cpu=1,2, passed.

  • Ran make test locally

  • Tested manually

  • Added new tests for changes (if applicable)

Checklist

  • My code follows the project's style guidelines (make lint)
  • I have read CONTRIBUTING.md
  • I have updated documentation if needed
  • This PR is focused and does not include unrelated changes

Changes the advisory locks every background loop takes (new internal/advisorylock), /readyz, the database pool floor, the storage forecast, manage_asset update and patch, the call catalog's outcome filter, full-text matching on seven search surfaces (migration 000209), the api/graphql operation ranking, the Google API refusal hint, the api and graphql connection editors, and the deployment guide's image pins; eight tickets:

1. #2169 a replica starved its own pool: each advisory-locked loop held a pooled connection for its whole pass while the pass drew from the same pool. Every try-lock held across work (catalog scan, maps, storage and webhook alerts, retention, capacity, call catalog and audit sweeps, thumbnail sweeps, catalog import, resource reclassification, connlimit slots, the connoauth refresher) now goes on one session per process; a key the process holds is refused, a session whose connection broke is ended, and .semgrep/go-advisory-lock.yml refuses a try-lock taken elsewhere. database.max_open_conns below 4 is refused at startup (dbobs.CheckPoolSize), and /readyz answers 503 database_pool_starved after 30s with no connection handed out.
2. #2203 storage forecast: a store's first day is left out of every fit, the 30-day rate waits for two weeks of history, and a replica that finds a sample within the interval takes none.
3. #2234 manage_asset update and patch grant newly declared references before the new version exists (Declarer.Widen), drop the ones no longer declared once the content is stored, and put them back when the content write fails.
4. #2231 the outcome filter on /api/v1/admin/calls and /api/v1/portal/calls: a count states the outcome as conditions, asks the satisfied rule only of calls an artifact names (citedSet) and reads supersession from one ordered pass over reads; a satisfied page joins the cited set; both run with JIT off. Migration 000208 adds idx_call_records_supersession. On the dev catalog (111,919 records) every outcome answers in 0.32s or less, from 3.6 to 4.7s.
5. #2230 a name is matched by its words: PostgreSQL read "acceptance-1627-1792487495133911000" as "acceptance" and the signed numbers -1627 and -1792487495133911000, so a search by its words never matched. Migration 000209 reads -, _, ., / and : as word breaks beside the text as written in the resource, asset, collection, prompt, knowledge page, script and catalog dataset search functions, the memory index and the folder search. An api or graphql operation not yet embedded is blended at a cosine of zero, as the SQL rankers blend a row the vector arm cannot see.
6. #2224 the deployment guide's compose example pins pgvector/pgvector:pg16, Keycloak 26.7.5 and Trino 453 to digests their registries serve, and make docs-images-check (scripts/docs-image-digests.sh, run in the E2E Nightly) resolves every digest the docs pin.
7. #2237 googlekey.APIHint reads a Google API refusal by its status and details[].reason: SERVICE_DISABLED, ACCESS_TOKEN_SCOPE_INSUFFICIENT, and any other 401 or 403 in Google's shape; no product's message text.
8. #2236 the api and graphql connection editors set oauth_token_lifetime as "Token lifetime" under the OAuth block.

Also: certifi 2026.10.19 in docs/requirements.txt (osv); the import ratchet gains the internal/advisorylock edges; the asset references built-in page, docs/server storage, configuration, observability, deployment and api-gateway, docs/memory/overview, docs/portal/admin-connections, CLAUDE.md and llms-full.txt.

Closes #2169
Closes #2203
Closes #2234
Closes #2231
Closes #2230
Closes #2224
Closes #2237
Closes #2236
@codecov

codecov Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.39884% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.82%. Comparing base (7790188) to head (9fae576).

Files with missing lines Patch % Lines
internal/advisorylock/advisorylock.go 96.77% 3 Missing ⚠️
internal/portal/assetrefs/declare.go 94.28% 2 Missing ⚠️
internal/httpserver/server.go 50.00% 1 Missing ⚠️
internal/platform/callrecord/postgres.go 98.55% 1 Missing ⚠️
internal/platform/callrecord/retention.go 66.66% 1 Missing ⚠️
pkg/toolkits/portal/patch_handlers.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2240      +/-   ##
==========================================
+ Coverage   94.80%   94.82%   +0.02%     
==========================================
  Files        1376     1378       +2     
  Lines      111500   111643     +143     
==========================================
+ Hits       105705   105871     +166     
+ Misses       5778     5755      -23     
  Partials       17       17              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cjimti
cjimti merged commit 61d7938 into main Oct 21, 2026
12 checks passed
@cjimti
cjimti deleted the fix/2169-open-bugs branch October 21, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment