Repository navigation
Fix production capability vector reindex resilience - #625
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThis PR introduces a shared ChangesVector Reindex Refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
🔎 Preview deployed: https://kody-pr-625.kentcdodds.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: 2
🧹 Nitpick comments (6)
.github/workflows/deploy.yml (2)
271-283: 🩺 Stability & Availability | 🔵 TrivialConfirm 120s
--max-timestill fits the new resilience path.The recursive per-item split/retry and AI Gateway fallback added elsewhere in this reindex stack can meaningfully lengthen total reindex duration (extra round trips per failed batch/vector plus a direct-Workers-AI retry on gateway failure). A hardcoded 120s timeout on this health-check curl could start failing production deploys with false negatives once real vector volume/failure rates trigger the new fallback paths, even though the underlying reindex work is progressing correctly.
🤖 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 @.github/workflows/deploy.yml around lines 271 - 283, The reindex deploy check in the workflow still uses a hardcoded 120s curl timeout, which may be too short now that the reindex path can recurse per item and fall back through AI Gateway retries. Review the `status="$(curl ... --max-time 120 ... __maintenance/reindex-capabilities)"` step and adjust the timeout strategy to accommodate the longer worst-case duration, or make it configurable so the `reindex-capabilities` health check does not fail deploys prematurely.
278-279: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider trimming the response body before printing to CI logs.
cat reindex-response.jsondumps the full JSON body (including per-phasefailedPhasesand any nested per-vectorfailureswithid/error) into the workflow log. Even with capped failure lists upstream, this puts internal record identifiers for capabilities/jobs/memories/packages into CI logs. Consider printing only a summary (e.g.,jq '{ok, failure}') unless full detail is needed for debugging.♻️ Example
- cat reindex-response.json + jq '{ok, error, failure}' reindex-response.json 2>/dev/null || cat reindex-response.json🤖 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 @.github/workflows/deploy.yml around lines 278 - 279, The workflow step that prints reindex results is exposing the full reindex-response JSON to CI logs. Update the deploy workflow’s reindex logging to avoid using the raw cat reindex-response.json output and instead print only a minimal summary from the response, keeping detailed failure fields out of the logs unless explicitly needed for debugging. Use the existing reindex response handling in the deploy job to locate the log statement and replace it with a summary-focused print.packages/worker/src/mcp/capabilities/reindex-batches.node.test.ts (1)
14-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for upsert failure isolation.
The helper’s core behavior includes recursively splitting failed upsert batches, but these tests only cover embedding failures. Add a case where the initial
index.upsert()rejects for one vector and assert successful vectors are upserted without extra embedding calls.🤖 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/mcp/capabilities/reindex-batches.node.test.ts` around lines 14 - 98, Add a test in reindex-batches.node.test.ts to cover upsert failure isolation in reindexVectorCandidates, since the current cases only verify embedding failures. Mock index.upsert to reject for a batch containing one bad vector so the helper recursively splits the batch, then assert the successful vectors are still upserted and embedTextsForVectorize is not called again during the retry path. Use the existing reindexVectorCandidates, embedTextsForVectorize, and index.upsert symbols to locate the behavior and keep the assertions aligned with the current failure-shaping pattern.packages/worker/src/mcp/capabilities/capability-search.ts (2)
260-289: 🩺 Stability & Availability | 🔵 TrivialPersistent gateway failures silently double AI cost/latency with only console-level visibility.
When
AI_GATEWAY_IDis stale/misconfigured, every batch now incurs two Workers AI calls (failing gateway attempt + direct retry) indefinitely, with only aconsole.warnas signal. Given this is exactly the scenario the new "stale-gateway" test simulates, consider surfacing repeated fallback occurrences via a metric or Sentry breadcrumb/alert so a persistently broken gateway config doesn't go unnoticed for an extended period.🤖 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/mcp/capabilities/capability-search.ts` around lines 260 - 289, The fallback in embedTextBatchForVectorize only logs a console warning when Workers AI Gateway keeps failing, so repeated stale-gateway retries can go unnoticed. Add a persistent visibility signal for this path by emitting a metric or Sentry breadcrumb/alert inside the gateway-failure catch before retrying direct Workers AI, and make sure it includes the gatewayId and error details from getErrorMessage(error) so the repeated fallback is detectable beyond console output.
245-293: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the gateway + direct failure path. The new combined error message at
capability-search.ts:284-286is still untested; the current tests cover gateway fallback success and direct-only failure, but not the case where both calls reject. A small test that makes bothAI.run(...)calls fail would lock this path down.🤖 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/mcp/capabilities/capability-search.ts` around lines 245 - 293, Add a test for the combined failure path in embedTextBatchForVectorize: mock runtime.AI.run to fail first with an AI Gateway gateway.id call and then fail again on the direct retry, and assert the thrown Error includes both the gateway and direct error messages. Use the existing runWorkersAiEmbeddingRequest and embedTextBatchForVectorize symbols to target the same fallback flow the current tests already cover for success and single-failure cases.packages/worker/src/mcp/capabilities/capability-search.workers.test.ts (1)
122-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood fallback test; add coverage for combined gateway+direct failure.
Correctly verifies gateway→direct fallback ordering and the warning message. However, per the PR's stated "gateway-to-direct fallback with combined error messages" behavior, there's no test where both the gateway call and the direct fallback call fail — that's the actual failure-surfacing path that matters most for observability.
🤖 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/mcp/capabilities/capability-search.workers.test.ts` around lines 122 - 157, Add a test in capability-search.workers.test.ts for the combined failure path in embedTextsForVectorize: make the first AI Gateway attempt fail and the direct Workers AI retry fail too, then assert the returned error surfaces both failures in the combined message. Reuse the existing aiEnv, embedTextsForVectorize, and CAPABILITY_EMBEDDING_MODEL symbols, and verify the warning/throw behavior reflects gateway-to-direct fallback followed by final failure.
🤖 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/src/mcp/capabilities/reindex-batches.ts`:
- Around line 62-99: The retry path in splitOrRecord/processBatch is re-running
embedding work after an upsert failure, which can both waste calls and
misclassify the error phase. Update the re-split logic in reindex-batches.ts so
that when embedTextsForVectorize succeeds, the computed embeddings are passed
through recursive retries and reused for the split upsert attempts instead of
calling processBatch again. Keep the phase handling in splitOrRecord consistent
by preserving the original 'upsert' failure context while retrying smaller
batches with the same embeddings.
In `@packages/worker/src/package-registry/package-reindex.ts`:
- Around line 39-90: The saved-package reindex flow is accumulating every
manifest load failure in failures, which can make the returned maintenance
payload huge even though failed already tracks the full count. Update the
package reindex logic around loadPackageManifestBySourceId and
reindexVectorCandidates to cap or truncate the stored failure details before
returning them, while still preserving the total failed count and summary.
---
Nitpick comments:
In @.github/workflows/deploy.yml:
- Around line 271-283: The reindex deploy check in the workflow still uses a
hardcoded 120s curl timeout, which may be too short now that the reindex path
can recurse per item and fall back through AI Gateway retries. Review the
`status="$(curl ... --max-time 120 ... __maintenance/reindex-capabilities)"`
step and adjust the timeout strategy to accommodate the longer worst-case
duration, or make it configurable so the `reindex-capabilities` health check
does not fail deploys prematurely.
- Around line 278-279: The workflow step that prints reindex results is exposing
the full reindex-response JSON to CI logs. Update the deploy workflow’s reindex
logging to avoid using the raw cat reindex-response.json output and instead
print only a minimal summary from the response, keeping detailed failure fields
out of the logs unless explicitly needed for debugging. Use the existing reindex
response handling in the deploy job to locate the log statement and replace it
with a summary-focused print.
In `@packages/worker/src/mcp/capabilities/capability-search.ts`:
- Around line 260-289: The fallback in embedTextBatchForVectorize only logs a
console warning when Workers AI Gateway keeps failing, so repeated stale-gateway
retries can go unnoticed. Add a persistent visibility signal for this path by
emitting a metric or Sentry breadcrumb/alert inside the gateway-failure catch
before retrying direct Workers AI, and make sure it includes the gatewayId and
error details from getErrorMessage(error) so the repeated fallback is detectable
beyond console output.
- Around line 245-293: Add a test for the combined failure path in
embedTextBatchForVectorize: mock runtime.AI.run to fail first with an AI Gateway
gateway.id call and then fail again on the direct retry, and assert the thrown
Error includes both the gateway and direct error messages. Use the existing
runWorkersAiEmbeddingRequest and embedTextBatchForVectorize symbols to target
the same fallback flow the current tests already cover for success and
single-failure cases.
In `@packages/worker/src/mcp/capabilities/capability-search.workers.test.ts`:
- Around line 122-157: Add a test in capability-search.workers.test.ts for the
combined failure path in embedTextsForVectorize: make the first AI Gateway
attempt fail and the direct Workers AI retry fail too, then assert the returned
error surfaces both failures in the combined message. Reuse the existing aiEnv,
embedTextsForVectorize, and CAPABILITY_EMBEDDING_MODEL symbols, and verify the
warning/throw behavior reflects gateway-to-direct fallback followed by final
failure.
In `@packages/worker/src/mcp/capabilities/reindex-batches.node.test.ts`:
- Around line 14-98: Add a test in reindex-batches.node.test.ts to cover upsert
failure isolation in reindexVectorCandidates, since the current cases only
verify embedding failures. Mock index.upsert to reject for a batch containing
one bad vector so the helper recursively splits the batch, then assert the
successful vectors are still upserted and embedTextsForVectorize is not called
again during the retry path. Use the existing reindexVectorCandidates,
embedTextsForVectorize, and index.upsert symbols to locate the behavior and keep
the assertions aligned with the current failure-shaping pattern.
🪄 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: f8acd8a3-f17b-4bb6-801d-73646e1cfccf
📒 Files selected for processing (12)
.github/workflows/deploy.ymlpackages/worker/src/capability-maintenance.node.test.tspackages/worker/src/capability-maintenance.tspackages/worker/src/jobs/job-reindex.tspackages/worker/src/mcp/capabilities/capability-reindex.tspackages/worker/src/mcp/capabilities/capability-search.tspackages/worker/src/mcp/capabilities/capability-search.workers.test.tspackages/worker/src/mcp/capabilities/reindex-batches.node.test.tspackages/worker/src/mcp/capabilities/reindex-batches.tspackages/worker/src/mcp/memory/memory-reindex.tspackages/worker/src/package-registry/package-reindex.node.test.tspackages/worker/src/package-registry/package-reindex.ts
Comment on lines
+62
to
+99
| async function splitOrRecord( | ||
| batch: ReadonlyArray<VectorReindexCandidate>, | ||
| phase: VectorReindexFailurePhase, | ||
| error: unknown, | ||
| ) { | ||
| if (batch.length === 1) { | ||
| recordFailure({ candidate: batch[0]!, phase, error }) | ||
| return | ||
| } | ||
| const midpoint = Math.ceil(batch.length / 2) | ||
| await processBatch(batch.slice(0, midpoint)) | ||
| await processBatch(batch.slice(midpoint)) | ||
| } | ||
|
|
||
| async function processBatch(batch: ReadonlyArray<VectorReindexCandidate>) { | ||
| let embeddings: Array<Array<number>> | ||
| try { | ||
| embeddings = await embedTextsForVectorize( | ||
| input.env, | ||
| batch.map((candidate) => candidate.text), | ||
| ) | ||
| } catch (error) { | ||
| await splitOrRecord(batch, 'embed', error) | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| await input.index.upsert( | ||
| batch.map((candidate, index_) => ({ | ||
| id: candidate.id, | ||
| values: embeddings[index_]!, | ||
| metadata: candidate.metadata, | ||
| })), | ||
| ) | ||
| upserted += batch.length | ||
| } catch (error) { | ||
| await splitOrRecord(batch, 'upsert', error) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reuse computed embeddings when splitting upsert failures.
After embedding succeeds, the upsert catch path calls splitOrRecord(), which re-enters processBatch() and embeds each split again. A transient AI/Gateway failure during that retry can misclassify an upsert problem as an embed failure and adds unnecessary embedding calls.
Suggested direction
+type EmbeddedVectorCandidate = {
+ candidate: VectorReindexCandidate
+ values: Array<number>
+}
+
-async function splitOrRecord(
+async function splitEmbedOrRecord(
batch: ReadonlyArray<VectorReindexCandidate>,
- phase: VectorReindexFailurePhase,
error: unknown,
) {
if (batch.length === 1) {
- recordFailure({ candidate: batch[0]!, phase, error })
+ recordFailure({ candidate: batch[0]!, phase: 'embed', error })
return
}
const midpoint = Math.ceil(batch.length / 2)
await processBatch(batch.slice(0, midpoint))
await processBatch(batch.slice(midpoint))
}
+
+async function upsertEmbeddedBatch(batch: ReadonlyArray<EmbeddedVectorCandidate>) {
+ try {
+ await input.index.upsert(
+ batch.map(({ candidate, values }) => ({
+ id: candidate.id,
+ values,
+ metadata: candidate.metadata,
+ })),
+ )
+ upserted += batch.length
+ } catch (error) {
+ if (batch.length === 1) {
+ recordFailure({ candidate: batch[0]!.candidate, phase: 'upsert', error })
+ return
+ }
+ const midpoint = Math.ceil(batch.length / 2)
+ await upsertEmbeddedBatch(batch.slice(0, midpoint))
+ await upsertEmbeddedBatch(batch.slice(midpoint))
+ }
+}🤖 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/mcp/capabilities/reindex-batches.ts` around lines 62 -
99, The retry path in splitOrRecord/processBatch is re-running embedding work
after an upsert failure, which can both waste calls and misclassify the error
phase. Update the re-split logic in reindex-batches.ts so that when
embedTextsForVectorize succeeds, the computed embeddings are passed through
recursive retries and reused for the split upsert attempts instead of calling
processBatch again. Keep the phase handling in splitOrRecord consistent by
preserving the original 'upsert' failure context while retrying smaller batches
with the same embeddings.
Comment on lines
+39
to
+90
| const failures: Array<VectorReindexFailure> = [] | ||
| const candidates: Array<VectorReindexCandidate> = [] | ||
| for (const row of rows) { | ||
| try { | ||
| const { manifest } = await loadPackageManifestBySourceId({ | ||
| env, | ||
| baseUrl: input.baseUrl, | ||
| userId: row.userId, | ||
| sourceId: row.sourceId, | ||
| }) | ||
| candidates.push({ | ||
| id: savedPackageVectorId(row.id), | ||
| values: vectors[index_]!, | ||
| text: buildSavedPackageEmbedText(manifest), | ||
| metadata: { | ||
| kind: 'package', | ||
| userId: row.userId, | ||
| }, | ||
| })), | ||
| ) | ||
| upserted += loaded.length | ||
| }) | ||
| } catch (error) { | ||
| failed += 1 | ||
| const failure = { | ||
| id: savedPackageVectorId(row.id), | ||
| phase: 'load' as const, | ||
| error: getErrorMessage(error), | ||
| } | ||
| failures.push(failure) | ||
| console.error( | ||
| JSON.stringify({ | ||
| message: 'saved package vector reindex skipped package', | ||
| packageId: row.id, | ||
| sourceId: row.sourceId, | ||
| error: failure.error, | ||
| }), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| return failed > 0 | ||
| ? { | ||
| upserted, | ||
| failed, | ||
| error: `${failed} saved package vector(s) failed to reindex`, | ||
| } | ||
| : { upserted } | ||
| const result = await reindexVectorCandidates({ | ||
| env, | ||
| index, | ||
| kind: 'saved package', | ||
| candidates, | ||
| }) | ||
| const totalFailed = failed + (result.failed ?? 0) | ||
| const allFailures = [...failures, ...(result.failures ?? [])] | ||
| if (totalFailed === 0) return { upserted: result.upserted } | ||
|
|
||
| const summary = `${totalFailed} saved package vector(s) failed to reindex` | ||
| return { | ||
| upserted: result.upserted, | ||
| failed: totalFailed, | ||
| failures: allFailures, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cap load-phase failure details before returning them.
reindexVectorCandidates() caps reported failures, but manifest load failures are accumulated unbounded here. If many saved packages fail to load, the maintenance response can become very large even though failed already carries the full count.
Suggested fix
const failures: Array<VectorReindexFailure> = []
+const maxReportedFailures = 20
const candidates: Array<VectorReindexCandidate> = []
@@
- failures.push(failure)
+ if (failures.length < maxReportedFailures) failures.push(failure)
@@
- const allFailures = [...failures, ...(result.failures ?? [])]
+ const allFailures = [...failures, ...(result.failures ?? [])].slice(
+ 0,
+ maxReportedFailures,
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const failures: Array<VectorReindexFailure> = [] | |
| const candidates: Array<VectorReindexCandidate> = [] | |
| for (const row of rows) { | |
| try { | |
| const { manifest } = await loadPackageManifestBySourceId({ | |
| env, | |
| baseUrl: input.baseUrl, | |
| userId: row.userId, | |
| sourceId: row.sourceId, | |
| }) | |
| candidates.push({ | |
| id: savedPackageVectorId(row.id), | |
| values: vectors[index_]!, | |
| text: buildSavedPackageEmbedText(manifest), | |
| metadata: { | |
| kind: 'package', | |
| userId: row.userId, | |
| }, | |
| })), | |
| ) | |
| upserted += loaded.length | |
| }) | |
| } catch (error) { | |
| failed += 1 | |
| const failure = { | |
| id: savedPackageVectorId(row.id), | |
| phase: 'load' as const, | |
| error: getErrorMessage(error), | |
| } | |
| failures.push(failure) | |
| console.error( | |
| JSON.stringify({ | |
| message: 'saved package vector reindex skipped package', | |
| packageId: row.id, | |
| sourceId: row.sourceId, | |
| error: failure.error, | |
| }), | |
| ) | |
| } | |
| } | |
| return failed > 0 | |
| ? { | |
| upserted, | |
| failed, | |
| error: `${failed} saved package vector(s) failed to reindex`, | |
| } | |
| : { upserted } | |
| const result = await reindexVectorCandidates({ | |
| env, | |
| index, | |
| kind: 'saved package', | |
| candidates, | |
| }) | |
| const totalFailed = failed + (result.failed ?? 0) | |
| const allFailures = [...failures, ...(result.failures ?? [])] | |
| if (totalFailed === 0) return { upserted: result.upserted } | |
| const summary = `${totalFailed} saved package vector(s) failed to reindex` | |
| return { | |
| upserted: result.upserted, | |
| failed: totalFailed, | |
| failures: allFailures, | |
| const failures: Array<VectorReindexFailure> = [] | |
| const maxReportedFailures = 20 | |
| const candidates: Array<VectorReindexCandidate> = [] | |
| for (const row of rows) { | |
| try { | |
| const { manifest } = await loadPackageManifestBySourceId({ | |
| env, | |
| baseUrl: input.baseUrl, | |
| userId: row.userId, | |
| sourceId: row.sourceId, | |
| }) | |
| candidates.push({ | |
| id: savedPackageVectorId(row.id), | |
| text: buildSavedPackageEmbedText(manifest), | |
| metadata: { | |
| kind: 'package', | |
| userId: row.userId, | |
| }, | |
| }) | |
| } catch (error) { | |
| failed += 1 | |
| const failure = { | |
| id: savedPackageVectorId(row.id), | |
| phase: 'load' as const, | |
| error: getErrorMessage(error), | |
| } | |
| if (failures.length < maxReportedFailures) failures.push(failure) | |
| console.error( | |
| JSON.stringify({ | |
| message: 'saved package vector reindex skipped package', | |
| packageId: row.id, | |
| sourceId: row.sourceId, | |
| error: failure.error, | |
| }), | |
| ) | |
| } | |
| } | |
| const result = await reindexVectorCandidates({ | |
| env, | |
| index, | |
| kind: 'saved package', | |
| candidates, | |
| }) | |
| const totalFailed = failed + (result.failed ?? 0) | |
| const allFailures = [...failures, ...(result.failures ?? [])].slice( | |
| 0, | |
| maxReportedFailures, | |
| ) | |
| if (totalFailed === 0) return { upserted: result.upserted } | |
| const summary = `${totalFailed} saved package vector(s) failed to reindex` | |
| return { | |
| upserted: result.upserted, | |
| failed: totalFailed, | |
| failures: allFailures, |
🤖 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/package-registry/package-reindex.ts` around lines 39 -
90, The saved-package reindex flow is accumulating every manifest load failure
in failures, which can make the returned maintenance payload huge even though
failed already tracks the full count. Update the package reindex logic around
loadPackageManifestBySourceId and reindexVectorCandidates to cap or truncate the
stored failure details before returning them, while still preserving the total
failed count and summary.
This was referenced Jul 5, 2026
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
3 participants
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
AI_GATEWAY_IDrouting fail-open: if Workers AI via AI Gateway throws, embeddings retry direct Workers AI and log a structured warning.Root-cause analysis
Most plausible root cause is AI Gateway misconfiguration exposed by #613.
AI_GATEWAY_IDhad been synced as an optional secret but unused; after #613, every embedding call passed that id intoenv.AI.run. If the gateway id is stale/nonexistent in the Cloudflare account, the first embedding request throws and the production maintenance endpoint returns 500. The code also had two secondary production risks: it did not enforce Workers AI embedding batch/input limits inembedTextsForVectorize, and reindex batches were all-or-nothing on embed/upsert failures.I cannot verify account-level Cloudflare Gateway configuration from the repo. Kent should check the production
AI_GATEWAY_IDsecret: either create/fix the matching AI Gateway in the Cloudflare account, or unset the secret. With this PR, an invalid gateway no longer blocks embeddings because direct Workers AI is retried automatically.Validation
npx vitest run --project node-unit packages/worker/src/capability-maintenance.node.test.ts packages/worker/src/package-registry/package-reindex.node.test.ts packages/worker/src/mcp/capabilities/reindex-batches.node.test.tsnpx vitest run --project workers-unit packages/worker/src/mcp/capabilities/capability-search.workers.test.tsnpm run typechecknpm run test:e2e:runnpm run validate(pass; first validate attempt hit a transient Playwrightssr-hydrationsocket hang-up, then E2E and full validate passed on rerun)System recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@7fb5290· Head:dcec519Classification: extends — this changes vector embedding/reindex failure behavior and maintenance response shape, without adding a new primitive.
Primitives touched
vectorize-searchcapability-registrymemoriesuserIdmetadatajobsuserIdmetadatasaved-packagesscheduled-cronSystem map
Change flow
Invariants
per-user-isolation: memory, job, and package vector metadata continue to carryuserId; builtin vectors remain global withkind: builtin.Summary by CodeRabbit
New Features
Bug Fixes