Skip to content

[WIP] Enable Node.js Type Stripping - #2

Closed
Ethan-Arrowood wants to merge 1 commit into
mainfrom
enable-type-stripping
Closed

[WIP] Enable Node.js Type Stripping#2
Ethan-Arrowood wants to merge 1 commit into
mainfrom
enable-type-stripping

Conversation

@Ethan-Arrowood

@Ethan-Arrowood Ethan-Arrowood commented Oct 13, 2025

Copy link
Copy Markdown
Member

Work in progress to enable Node.js type stripping by correctly configuring code.

High level list of to dos:

  • Remove type: 'commonjs' from package.json for less-strict module resolution
  • Fix relative file paths to utilize PACKAGE_ROOT value
    • Consider some alternative for static files resolution such as moving values to JS strings
  • Fix imports and requires to include file endings
  • Fix file endings to match intended source module type
    • Pretty much all of our TS code is written using ESM syntax, but we've been transforming it to CJS anyways. Some early tests demonstrate that if we switch to ESM most of our code will continue to work fine. Maybe the easiest solution is to rename TS to .mts for now until we can get all the CJS converted over time 🤷‍♂️
  • Ensure TypeScript build configuration is outputting appropriately

@Ethan-Arrowood

Copy link
Copy Markdown
Member Author

this is quite out of date now. going to close

heskew added a commit that referenced this pull request Apr 24, 2026
Stacked onto #402 in response to the deep external review. Changes:

Accepted and fixed:

- #3 (ai-review-log log step): Ported verbatim from oauth. Harper's
  reviews now feed the central calibration tracker that the weekly
  sweep runs against. Adds AI_REVIEW_LOG_TOKEN to the secrets
  prerequisite list (flagged in PR body update).

- #4 (dead `documentation/**` glob): Harper has no `documentation/`
  dir — the docs site is a separate repo. Replaced with realistic
  Harper doc-file names (README.md, CLAUDE.md, AGENTS.md,
  dependencies.md) + package.json keyword edits. Same fix in both
  mention and issue-to-pr prompts.

- #5 (prefix-match label too permissive): `startsWith('claude-fix:')`
  matched typoed variants (`claude-fix:typos`, etc). Tightened to
  explicit whitelist of the four supported labels.

- #6 (fixed heredoc marker collision risk): Replaced
  `CLAUDE_SCOPE_EOF` with a random `EOF_$(openssl rand -hex 16)`
  delimiter. Collision-proof against any content a future
  ai-review-prompts layer might include.

- #7a (eager `npm ci` on mention): Removed. Most mentions (explain,
  review, small edits) don't need deps — install is ~35-60s × every
  mention. Prompt now tells the agent to run `npm ci` itself before
  any script that requires dependencies. issue-to-pr keeps its eager
  install since that workflow almost always builds/tests.

- #8a (Opus cost on every mention): Shifted to Sonnet default with
  Opus opt-in via case-insensitive word-boundary `deep` in the
  comment. "Needs deep review of the whole migration" escalates;
  "fix this typo" stays on Sonnet. Cost gets spent deliberately, not
  by default.

- #9 (no scope-to-diff guidance): Review prompt now tells the agent
  to start from `git diff --name-only <base>...HEAD` and only
  expand scope when a specific finding demands it. On a ~1000-file
  repo this matters.

Plus a mention-parsing step that enforces:
- `@claude` must be the first non-whitespace token (word-boundary
  after) — rules out `@claudette`, inline prose mentions, and
  quoted replies (`> @claude ...`) where the reply addresses a
  human. The existing `contains('@claude')` job-level `if:` stays
  as a cheap pre-filter; the new shell step is the real precision
  gate.
- Subsequent steps guard on `steps.mention.outputs.proceed == 'true'`.

Comment sharpening (accept the tradeoff, tighten the rationale):

- #1 (postinstall RCE via package.json edit): The allowlist comment
  on both agent workflows previously implied `Bash(npm install)`
  (no-arg) was a real mitigation. It blocks `npm install @attacker/x`
  but NOT the `postinstall` path — an injection can edit package.json
  and then bare `npm install` executes the hostile lifecycle script
  with GITHUB_TOKEN + the claude[bot] installation token in env.
  Comment now names this path explicitly. The actual guardrails are
  branch protection + the author_association gate; a structural fix
  (`.npmrc ignore-scripts=true`, or dropping `Bash(npm install)`
  entirely in favor of a separate CI install job) deserves its own
  PR.

- #2 (`Bash(git:*)` contradicts review.yml's stated principle):
  review.yml's comment previously read as universal guidance. It's
  actually specific to the read-only review workflow. Comment now
  explicitly notes that the authoring workflows deliberately grant
  broader git access and rely on branch protection as the guardrail.

Not addressed here:

- Splitting issue-to-pr into read-only research + narrow-write
  commit steps (post-v0.1.0 follow-up).
- Tightening mention/issue-to-pr to specific read-only + commit/push
  git subcommands (same structural PR).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kriszyp pushed a commit that referenced this pull request May 5, 2026
Stacked onto #402 in response to the deep external review. Changes:

Accepted and fixed:

- #3 (ai-review-log log step): Ported verbatim from oauth. Harper's
  reviews now feed the central calibration tracker that the weekly
  sweep runs against. Adds AI_REVIEW_LOG_TOKEN to the secrets
  prerequisite list (flagged in PR body update).

- #4 (dead `documentation/**` glob): Harper has no `documentation/`
  dir — the docs site is a separate repo. Replaced with realistic
  Harper doc-file names (README.md, CLAUDE.md, AGENTS.md,
  dependencies.md) + package.json keyword edits. Same fix in both
  mention and issue-to-pr prompts.

- #5 (prefix-match label too permissive): `startsWith('claude-fix:')`
  matched typoed variants (`claude-fix:typos`, etc). Tightened to
  explicit whitelist of the four supported labels.

- #6 (fixed heredoc marker collision risk): Replaced
  `CLAUDE_SCOPE_EOF` with a random `EOF_$(openssl rand -hex 16)`
  delimiter. Collision-proof against any content a future
  ai-review-prompts layer might include.

- #7a (eager `npm ci` on mention): Removed. Most mentions (explain,
  review, small edits) don't need deps — install is ~35-60s × every
  mention. Prompt now tells the agent to run `npm ci` itself before
  any script that requires dependencies. issue-to-pr keeps its eager
  install since that workflow almost always builds/tests.

- #8a (Opus cost on every mention): Shifted to Sonnet default with
  Opus opt-in via case-insensitive word-boundary `deep` in the
  comment. "Needs deep review of the whole migration" escalates;
  "fix this typo" stays on Sonnet. Cost gets spent deliberately, not
  by default.

- #9 (no scope-to-diff guidance): Review prompt now tells the agent
  to start from `git diff --name-only <base>...HEAD` and only
  expand scope when a specific finding demands it. On a ~1000-file
  repo this matters.

Plus a mention-parsing step that enforces:
- `@claude` must be the first non-whitespace token (word-boundary
  after) — rules out `@claudette`, inline prose mentions, and
  quoted replies (`> @claude ...`) where the reply addresses a
  human. The existing `contains('@claude')` job-level `if:` stays
  as a cheap pre-filter; the new shell step is the real precision
  gate.
- Subsequent steps guard on `steps.mention.outputs.proceed == 'true'`.

Comment sharpening (accept the tradeoff, tighten the rationale):

- #1 (postinstall RCE via package.json edit): The allowlist comment
  on both agent workflows previously implied `Bash(npm install)`
  (no-arg) was a real mitigation. It blocks `npm install @attacker/x`
  but NOT the `postinstall` path — an injection can edit package.json
  and then bare `npm install` executes the hostile lifecycle script
  with GITHUB_TOKEN + the claude[bot] installation token in env.
  Comment now names this path explicitly. The actual guardrails are
  branch protection + the author_association gate; a structural fix
  (`.npmrc ignore-scripts=true`, or dropping `Bash(npm install)`
  entirely in favor of a separate CI install job) deserves its own
  PR.

- #2 (`Bash(git:*)` contradicts review.yml's stated principle):
  review.yml's comment previously read as universal guidance. It's
  actually specific to the read-only review workflow. Comment now
  explicitly notes that the authoring workflows deliberately grant
  broader git access and rely on branch protection as the guardrail.

Not addressed here:

- Splitting issue-to-pr into read-only research + narrow-write
  commit steps (post-v0.1.0 follow-up).
- Tightening mention/issue-to-pr to specific read-only + commit/push
  git subcommands (same structural PR).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
heskew added a commit that referenced this pull request May 26, 2026
claude-bot's re-review of 47bd103 flagged missing test coverage for the
source-changing PATCH path. Writing those tests surfaced the actual bug —
the `when(...)` wrap 47bd103 added to `TableResource.update()` is
unreachable in production for REST PATCH and PUT.

ROOT CAUSE — legacy URLSearchParams branch drops the promise:

  Table.patch(target, recordUpdate) and Table.put(target, record) both
  detect the back-compat `URLSearchParams` argument shape and route
  through:

    (this as any).update(target, false);   // promise DISCARDED
    return this.save() as any;

  REST dispatch goes through the transactional wrapper which calls
  `resource.put(data, query)` / `resource.patch(data, query)` — `query`
  is a URLSearchParams, so the URLSearchParams branch is the CANONICAL
  REST path, not the "standard path" branch that 47bd103 fixed. The
  embedder's promise on `update()` was settled-and-discarded; `save()`
  ran on a write that hadn't yet been staged on the txn, so on a busy
  worker the embedder mutation arrived after the in-memory commit had
  already taken the un-embedded record.

  Fixed both put() and patch() with the same shape used in the standard
  path: `when(this.update(target, ...), () => this.save())`.

TESTS — three new integration tests in embed-directive.test.ts:

  - PATCH-with-source — PATCH `{content: '...'}` on a seeded row;
    verifies embed fires exactly once and stored vector matches
    `deterministicVector(updatedContent)`, not the seed.
  - PUT-with-source — same shape via REST PUT; locks in the parallel
    `Table.put()` fix.
  - Caching-table @embed — installs a `resources.js` that exposes
    `CachedEmbedSource extends Resource` and calls
    `CachedEmbedDoc.sourcedFrom(CachedEmbedSource)`. GET on the cache
    triggers `getFromSource` → cache write → embed wiring at
    `Table.ts:~4520`. Verifies the cached row's vector matches the
    source-returned content, and a cache-hit GET does not re-fire the
    embedder. Closes claude-bot blocker #2 directly.

Local: 17/17 embedHook unit + 13/13 caching unit + 7/7 integration green.

Refs #632 #510 #747

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jun 8, 2026
… collision)

When typed random-access structures are off, records use classic shared
structures whose first byte can be 66 (0x42 = structure-id #2). RecordEncoder's
rocksdb metadata heuristic treats a leading 66 as a local-timestamp prefix and
strips 8 bytes, corrupting the record (decoded as null). The audit store's
getValue decodes a value that carries no on-disk timestamp prefix, so it now
passes { noMetadata: true } to skip the heuristic entirely. Surfaced by the
MQTT "can publish non-JSON" path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jun 9, 2026
… collision)

When typed random-access structures are off, records use classic shared
structures whose first byte can be 66 (0x42 = structure-id #2). RecordEncoder's
rocksdb metadata heuristic treats a leading 66 as a local-timestamp prefix and
strips 8 bytes, corrupting the record (decoded as null). The audit store's
getValue decodes a value that carries no on-disk timestamp prefix, so it now
passes { noMetadata: true } to skip the heuristic entirely. Surfaced by the
MQTT "can publish non-JSON" path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jun 10, 2026
- waitForJob: fix falsy-status early-exit — `status !== 'IN_PROGRESS'`
  exits immediately when job is not found / response is malformed,
  rather than polling until timeout
- northwind: replace redundant post-waitForCondition assertions with a
  .catch() re-throw that appends lastResponse.text to the timeout error,
  making failure messages reachable and useful (#2 and #3)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
heskew added a commit that referenced this pull request Jun 10, 2026
…, thresholds, reindex

Four tests through the full stack (real RocksDB, real Table.search(), schema-driven
HNSW deployment via component API) guard each of the six data-integrity fixes in
commit 251e5b7:

  1. delete-entry-point: 50 records, bulk-delete 40 including EP, all survivors
     reachable.  Guards fix #2 (EP replacement scan + transaction + skip-deleted).

  2. update-churn: 30 records × 5 re-embed rounds, all records still findable
     by their final vector.  Guards fix #3 (UPDATE sweeps only level l).

  3. threshold queries: 2-D vectors at known exact cosine distances verify that
     le(boundary) is inclusive and lt(boundary) is exclusive.  Guards fix #6b
     (le comparator uses <= not <).

  4. reindex backfill: populate table without HNSW, add index, poll until search
     works, assert all 40 pre-existing records reachable; post-backfill update and
     delete must behave correctly.  Guards fixes #4 and #5.

Interrupted-backfill-then-restart is explicitly deferred: 40-record backfill
completes in milliseconds so a SIGKILL race would be non-deterministic.  That
scenario is covered by the unit tests in unitTests/resources/vectorIndex.test.js.

Vector search is exercised via the HTTP QUERY method so the body reaches
Table.search() without the mapCondition stripping done by search_by_conditions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dawsontoth added a commit that referenced this pull request Jun 23, 2026
- resolveResourcePath: restore the historical `${prefix}/${name}` join so a
  bare name under an empty prefix keeps its leading slash (plain-Map
  consumers like globalIsolation.test.js rely on the exact `/Name` key;
  real Resources.set strips it). Fixes the unit-test regression.
- resolveResourcePath + setParamRoute: normalize trailing slashes so a
  route declared/registered as `/widget/:id/` still matches normalized
  request URLs (review #3).
- setParamRoute: reject a wildcard that is not the final segment (review #1).
- setParamRoute: order routes by segment-by-segment specificity
  (static > param > wildcard at each index, longer pattern wins on a tie)
  to prevent shadowing of mixed static/param patterns (review #2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dawsontoth added a commit that referenced this pull request Jun 23, 2026
- resolveResourcePath: restore the historical `${prefix}/${name}` join so a
  bare name under an empty prefix keeps its leading slash (plain-Map
  consumers like globalIsolation.test.js rely on the exact `/Name` key;
  real Resources.set strips it). Fixes the unit-test regression.
- resolveResourcePath + setParamRoute: normalize trailing slashes so a
  route declared/registered as `/widget/:id/` still matches normalized
  request URLs (review #3).
- setParamRoute: reject a wildcard that is not the final segment (review #1).
- setParamRoute: order routes by segment-by-segment specificity
  (static > param > wildcard at each index, longer pattern wins on a tie)
  to prevent shadowing of mixed static/param patterns (review #2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dawsontoth added a commit that referenced this pull request Jun 24, 2026
- resolveResourcePath: restore the historical `${prefix}/${name}` join so a
  bare name under an empty prefix keeps its leading slash (plain-Map
  consumers like globalIsolation.test.js rely on the exact `/Name` key;
  real Resources.set strips it). Fixes the unit-test regression.
- resolveResourcePath + setParamRoute: normalize trailing slashes so a
  route declared/registered as `/widget/:id/` still matches normalized
  request URLs (review #3).
- setParamRoute: reject a wildcard that is not the final segment (review #1).
- setParamRoute: order routes by segment-by-segment specificity
  (static > param > wildcard at each index, longer pattern wins on a tie)
  to prevent shadowing of mixed static/param patterns (review #2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dawsontoth added a commit that referenced this pull request Jun 24, 2026
- resolveResourcePath: restore the historical `${prefix}/${name}` join so a
  bare name under an empty prefix keeps its leading slash (plain-Map
  consumers like globalIsolation.test.js rely on the exact `/Name` key;
  real Resources.set strips it). Fixes the unit-test regression.
- resolveResourcePath + setParamRoute: normalize trailing slashes so a
  route declared/registered as `/widget/:id/` still matches normalized
  request URLs (review #3).
- setParamRoute: reject a wildcard that is not the final segment (review #1).
- setParamRoute: order routes by segment-by-segment specificity
  (static > param > wildcard at each index, longer pattern wins on a tie)
  to prevent shadowing of mixed static/param patterns (review #2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp pushed a commit that referenced this pull request Jun 24, 2026
… (#1460)

* feat(resources): support parameterised paths in Resource exports (#602)

Add `:param` and `*wildcard` route segments to Harper's resource path
mechanism. Routes can be declared via a `static path` field on the
resource class (leading `/` = root-relative top-level, `./`/bare =
component-relative) or via the existing `export { X as '/path' }` form,
both now honoring parameterised and leading-slash root paths.

- Resources.ts: compile parameterised paths into a separate `paramRoutes`
  list (kept out of the base Map so the exact/prefix fast path is
  untouched); getMatch falls back to them only when no static resource
  matches ("static wins") and only when param routes exist. Matched,
  URL-decoded segments are stored on `entry.params`.
- jsResource.ts: honor `static path` during registration; resolve
  leading-slash declared paths/export names as root-relative.
- REST.ts (HTTP + WS) and DurableSubscriptionsSession.ts (MQTT
  publish/subscribe): bind `entry.params` onto the RequestTarget so
  `:id` -> target.id, `*rest` -> target.rest, etc.
- openApi.ts: emit parameterised routes as templated paths with path
  parameters; mcp/resources.ts: list them via resources/templates/list
  as `{param}` URI templates. Shared converter `routePatternToTemplate`.

Docs in resources/DESIGN.md. Unit tests in
unitTests/resources/paramRoutes.test.js plus OpenAPI/MCP enumeration
coverage, and an end-to-end integrationTests/apiTests/param-routes.test.mjs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(resources): address review feedback on parameterised routes

- resolveResourcePath: restore the historical `${prefix}/${name}` join so a
  bare name under an empty prefix keeps its leading slash (plain-Map
  consumers like globalIsolation.test.js rely on the exact `/Name` key;
  real Resources.set strips it). Fixes the unit-test regression.
- resolveResourcePath + setParamRoute: normalize trailing slashes so a
  route declared/registered as `/widget/:id/` still matches normalized
  request URLs (review #3).
- setParamRoute: reject a wildcard that is not the final segment (review #1).
- setParamRoute: order routes by segment-by-segment specificity
  (static > param > wildcard at each index, longer pattern wins on a tie)
  to prevent shadowing of mixed static/param patterns (review #2).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(server): bind MQTT subscribe route params before framework fields

In the MQTT subscribe path, `...entry.params` was spread last in the
Object.assign literal, so a route param could silently override
framework-controlled fields — most importantly `checkPermission`, which
would break authorization. Spread params first so framework fields always
win, matching the order used in REST.ts and publishMessage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(resources): name a bare wildcard `wildcard` for valid templates

A bare `*` segment was stored under the key `'*'`, which `routePatternToTemplate`
then emitted as `{*}` in the OpenAPI path — invalid per RFC 6570 / OpenAPI 3.0.3
— while the MCP generator independently used `{wildcard}`. Normalize a nameless
wildcard to `wildcard` at compile time so the bound key, OpenAPI path template,
and MCP URI template are all consistent and valid. Named wildcards (`*rest`)
are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(mcp,resources): repair rebase build-break and sync param-route teardown

The rebase onto main's MCP v1 surface (#1349) renamed the template
accumulator to `all`, but the parameterised-route loop still pushed to the
old `templates` identifier — an undefined reference that failed the linter
(no-undef) and every unit suite. Point it at `all`.

Also, per review: `Resources extends Map` keeps parameterised routes in a
side array, so override `delete()`/`clear()` to prune that array too —
otherwise a removed/cleared route could keep matching an unloaded Resource
class. (Today the registry is always rebuilt via resetResources(), so this
is defensive, but it removes the latent footgun.)

Tests: update the MCP template tests for the new paginated
ListResourceTemplatesResult shape; add delete()/clear() coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
heskew added a commit that referenced this pull request Jun 30, 2026
…ck (#1326)

Resolve every scope.models call through a ModelRouter that returns the ordered
candidate backends; the facade uses the first that satisfies the call's required
capabilities and falls through to the next on failure — capability-aware selection
and fallback are one mechanism.

- ModelRouter / RouteRequest / Capability types; opts.requires on embed/generate.
- routing.ts: a default router (logical name -> [name, ...config fallback group],
  capability-filtered, ordered). Synchronous, so the facade keeps its up-front
  resolution errors (the generateStream guard).
- models.registerRouter(router) installs a custom policy — a method on the models
  scope (reachable from components via scope.models), not a free global, since the
  surface is model-specific. ModelRouter / RouteRequest / Capability are re-exported
  for custom-router authors.
- Models.embed/generate loop candidates with per-attempt hdb_model_calls analytics
  and fallback; generateStream resolves synchronously to the first candidate. Tools
  in the input auto-require the tools capability, so a tools call routes to a
  tools-capable backend in the group instead of hard-erroring.
- Config: `fallback: [names]` on a model entry defines the ordered group (validator
  + bootstrap). Stale groups are cleared on each reload. Resolves #510 open-decision #2.
- Existing behavior preserved (NotFound / ModelCapabilityError / analytics /
  sync-throw); 272 model unit tests pass, incl. new routing + fallback coverage.

Closes #1326. Part of #510.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
heskew added a commit that referenced this pull request Jun 30, 2026
…ck (#1326)

Resolve every scope.models call through a ModelRouter that returns the ordered
candidate backends; the facade uses the first that satisfies the call's required
capabilities and falls through to the next on failure — capability-aware selection
and fallback are one mechanism.

- ModelRouter / RouteRequest / Capability types; opts.requires on embed/generate.
- routing.ts: a default router (logical name -> [name, ...config fallback group],
  capability-filtered, ordered). Synchronous, so the facade keeps its up-front
  resolution errors (the generateStream guard).
- models.registerRouter(router) installs a custom policy — a method on the models
  scope (reachable from components via scope.models), not a free global, since the
  surface is model-specific. ModelRouter / RouteRequest / Capability are re-exported
  for custom-router authors.
- Models.embed/generate loop candidates with per-attempt hdb_model_calls analytics
  and fallback; generateStream resolves synchronously to the first candidate. Tools
  in the input auto-require the tools capability, so a tools call routes to a
  tools-capable backend in the group instead of hard-erroring.
- Config: `fallback: [names]` on a model entry defines the ordered group (validator
  + bootstrap). Stale groups are cleared on each reload. Resolves #510 open-decision #2.
- Existing behavior preserved (NotFound / ModelCapabilityError / analytics /
  sync-throw); 272 model unit tests pass, incl. new routing + fallback coverage.

Closes #1326. Part of #510.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
heskew added a commit that referenced this pull request Jun 30, 2026
…ck (#1326)

Resolve every scope.models call through a ModelRouter that returns the ordered
candidate backends; the facade uses the first that satisfies the call's required
capabilities and falls through to the next on failure — capability-aware selection
and fallback are one mechanism.

- ModelRouter / RouteRequest / Capability types; opts.requires on embed/generate.
- routing.ts: a default router (logical name -> [name, ...config fallback group],
  capability-filtered, ordered). Synchronous, so the facade keeps its up-front
  resolution errors (the generateStream guard).
- models.registerRouter(router) installs a custom policy — a method on the models
  scope (reachable from components via scope.models), not a free global, since the
  surface is model-specific. ModelRouter / RouteRequest / Capability are re-exported
  for custom-router authors.
- Models.embed/generate loop candidates with per-attempt hdb_model_calls analytics
  and fallback; generateStream resolves synchronously to the first candidate. Tools
  in the input auto-require the tools capability, so a tools call routes to a
  tools-capable backend in the group instead of hard-erroring.
- Config: `fallback: [names]` on a model entry defines the ordered group (validator
  + bootstrap). Stale groups are cleared on each reload. Resolves #510 open-decision #2.
- Existing behavior preserved (NotFound / ModelCapabilityError / analytics /
  sync-throw); 272 model unit tests pass, incl. new routing + fallback coverage.

Closes #1326. Part of #510.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
heskew added a commit that referenced this pull request Jun 30, 2026
…1534)

Resolve every scope.models call through a ModelRouter that returns the ordered
candidate backends; the facade uses the first that satisfies the call's required
capabilities and falls through to the next on failure — capability-aware selection
and fallback are one mechanism. Also finishes namespacing the model-extension API
under scope.models: registerBackend / defineBackend / registerRouter are methods on
the models singleton, and the generic free globals are removed (breaking, #1534).

Routing (#1326):
- ModelRouter / RouteRequest / Capability types; opts.requires on embed/generate.
- routing.ts: a default router (logical name -> [name, ...config fallback group],
  capability-filtered, ordered). Synchronous, so the facade keeps its up-front
  resolution errors (the generateStream guard).
- Models.embed/generate loop candidates with per-attempt hdb_model_calls analytics
  and fallback (any error falls through; an aborted caller short-circuits — checked
  before each attempt and after each failure). generateStream resolves synchronously
  to the first candidate. Tools in the input auto-require the tools capability.
- A custom router returning no candidates for a satisfying backend surfaces as a
  plain "no candidates" error, not a misleading ModelCapabilityError.
- Config: `fallback: [names]` on a model entry defines the ordered group (validator +
  bootstrap); stale groups cleared on each reload. Resolves #510 open-decision #2.

Namespaced extension API (#1534, breaking):
- models.defineBackend(spec) added (method); models.registerBackend already existed.
- Removed the generic free globals + top-level harperdb exports registerBackend and
  defineBackend (globals.js, index.ts, getHarperExports, _assignPackageExport).
  Authors switch to models.registerBackend(...) / models.defineBackend(...) /
  models.registerRouter(...). The DefineBackendSpec / ModelBackend types stay exported.

Existing behavior preserved (NotFound / ModelCapabilityError / analytics / sync-throw);
282 model unit tests pass, incl. new routing, fallback, abort, and method-only coverage.

BREAKING CHANGE: the `registerBackend` and `defineBackend` free globals / top-level
`harperdb` exports are removed. Use `models.registerBackend(...)` and
`models.defineBackend(...)` (methods on `scope.models` / the `models` global) instead.
scope.models is still pre-feature-complete, so the break is taken now rather than
carrying a deprecated generic global.

Closes #1326. Closes #1534. Part of #510.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jul 6, 2026
…durable-session test

`subscribe with QoS=1 and reconnect with non-clean session` sequenced connect/
disconnect/publish steps with hard-coded `delay(10)`/`delay(50)` calls instead
of waiting for the actual server-side state to settle. `client.endAsync()`
only resolves once the local socket closes — it does not wait for the broker
to finish tearing down the durable session — so reconnecting with the same
clientId immediately after could race the broker (issue #1138 flagged this
file as the #2-worst offender for this pattern). Under CI load the delays
were occasionally too short, causing the intermittent "got 0 messages"/mocha
timeout flakes seen on #410 and #1476.

Replace the disconnect-then-reconnect delays with a wait on the broker's own
`disconnected` event (already exposed via `server.mqtt.events`) for the
matching clientId, and replace the pre-disconnect delay with a wait for the
client's outgoing PUBACK. Also await the first offline publish (it was
fire-and-forget while the other two were awaited), so all three publishes are
durably committed before the reconnecting client asserts on redelivery.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jul 16, 2026
…ryRun (#1832)

An orphaned blob file is invisible today: nothing surfaces the condition, and
the only way to learn of it is to run the destructive cleanup_orphan_blobs
sweep and read the count out of the logs. That makes measuring the condition
and acting on it the same operation, so an operator cannot alert on stranded
bytes or size a reclaim before committing to it.

cleanupOrphans now returns the orphan count and the bytes those files hold,
and takes a dryRun flag that runs the identical scan without unlinking
anything. cleanup_orphan_blobs passes dryRun through. Both modes stay
on-demand only, so this adds no steady-state cost.

This is ask #2 of #1832 (observability). The primary ask — making the inline
commit-time unlink race-safe — is not included; see the issue for the
root-cause analysis and the open design question.

Co-Authored-By: Claude Opus <noreply@anthropic.com>
kriszyp added a commit that referenced this pull request Jul 22, 2026
…ryRun (#1832)

An orphaned blob file is invisible today: nothing surfaces the condition, and
the only way to learn of it is to run the destructive cleanup_orphan_blobs
sweep and read the count out of the logs. That makes measuring the condition
and acting on it the same operation, so an operator cannot alert on stranded
bytes or size a reclaim before committing to it.

cleanupOrphans now returns the orphan count and the bytes those files hold,
and takes a dryRun flag that runs the identical scan without unlinking
anything. cleanup_orphan_blobs passes dryRun through. Both modes stay
on-demand only, so this adds no steady-state cost.

This is ask #2 of #1832 (observability). The primary ask — making the inline
commit-time unlink race-safe — is not included; see the issue for the
root-cause analysis and the open design question.

Co-Authored-By: Claude Opus <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant