Commit changes to package-lock.json from running npm install by cap10morgan · Pull Request #3 · HarperFast/harper

cap10morgan · 2025-10-17T18:31:14Z

No description provided.

@claude

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>

@claude

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>

- 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>

…, 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>

- 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>

… (#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>

…egistrations (#1809) /code-review #3: the deploy pre-flight snapshot/restore only isolated the quota handler; a candidate's server.registerOperation during the throwaway validation load still mutated the process-wide operation map AND announced to the main thread, leaking onto the live worker on a failed deploy. Replace the quota-specific withMcpQuotaHandlerPreserved with a general guard (deployValidationState.ts): server.registerOperation and server.setMcpQuotaHandler both no-op while a validation load is in flight (validation only needs to surface load-time errors, not register anything). operations.js wraps the validation load in runWithDeployValidationGuard. Skipping is cleaner than snapshot/restore here — it also suppresses the cross-thread operation announce, which a local restore can't undo. Depth-counted; the narrow interleaving caveat is documented. Tests: registerOperation + setMcpQuotaHandler are skipped during validation and resume after (incl. after a thrown load), in serverUtilities.test.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy

@export

…erenced Resource (#1809) (#1821) * Register the MCP durable quota policy as a function, not a config-referenced Resource (#1809) The quota hook was configured by `mcp.<profile>.quota.resource` pointing at an exported Resource, whose inherited CRUD then surfaced on every transport (update_/delete_ MCP tools + REST/SSE/WS/GraphQL/MQTT) — a permitted client could reset its own counter. The docs example worked around it by turning six exportTypes flags off, which made the safe path the easy-to-forget one. Replace it with a registration function: `server.setMcpQuotaHandler(fn)`. The policy is a plain function (never an exposed Resource), enabled by registering it (no config). checkDurableQuota invokes the registered handler; no handler => allowed (opt-in), throw => fail-closed deny (unchanged). The handler receives `profile` so one handler can gate operations vs application. - Remove the `mcp.<profile>.quota.resource`/`.method` config params. - Wire `server.setMcpQuotaHandler` next to `server.registerOperation`. - Migrate the mcp-quota fixture off `tables.QuotaCounter`: an exported tool (Answerer) + an internal (non-@export) counter table + a registered handler, so nothing exposes the counter. Integration test asserts the counter is not REST-reachable. Supersedes the docs#576 six-`false` example; docs follow-up separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy * Isolate the quota handler from deploy pre-flight validation; allow clearing (review) Codex review: - P1: the quota handler is a process-wide singleton, so a candidate component's top-level server.setMcpQuotaHandler(...) during deploy pre-flight validation would outlive the throwaway load and alter live enforcement on a failed deploy. Snapshot the handler before the validation load and restore it in the finally (added getMcpQuotaHandler()). - P2: the Server interface rejected undefined though the setter supports clearing; widen the public type to McpQuotaHandler | undefined. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy * Test: assert the internal counter exposes no REST route and no MCP CRUD tools (#1809) Codify the security property the redesign delivers (and that /verify checked by hand): the counter table is internal, so GET /QuotaCounter 404s and tools/list carries no QuotaCounter update_/delete_ tools a client could call to reset its quota. Tightened the counter-not-exposed assertion from !=200 to ==404. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy * Extract withMcpQuotaHandlerPreserved and unit-test the deploy-validation isolation (#1809) The P1 snapshot/restore was inline in the deploy op and only its get/set primitive was covered. Extract it into withMcpQuotaHandlerPreserved(fn) — operations.js wraps the throwaway validation load in it — and unit-test the isolation directly: a candidate that registers a different handler, one that clears it, and a load that throws all leave the live worker's handler intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy * Simplify quota-handler wiring; note the snapshot-restore tradeoff (review) /code-review: assign server.setMcpQuotaHandler directly instead of a redundant wrapper (also keeps the impl param type in sync with the Server interface's McpQuotaHandler | undefined). Document that withMcpQuotaHandlerPreserved restores unconditionally, so a legitimate interleaving registration would be reverted — a narrow window, and the lesser evil vs leaking a candidate policy live. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy * Generalize deploy-validation isolation to all process-wide server.* registrations (#1809) /code-review #3: the deploy pre-flight snapshot/restore only isolated the quota handler; a candidate's server.registerOperation during the throwaway validation load still mutated the process-wide operation map AND announced to the main thread, leaking onto the live worker on a failed deploy. Replace the quota-specific withMcpQuotaHandlerPreserved with a general guard (deployValidationState.ts): server.registerOperation and server.setMcpQuotaHandler both no-op while a validation load is in flight (validation only needs to surface load-time errors, not register anything). operations.js wraps the validation load in runWithDeployValidationGuard. Skipping is cleaner than snapshot/restore here — it also suppresses the cross-thread operation announce, which a local restore can't undo. Depth-counted; the narrow interleaving caveat is documented. Tests: registerOperation + setMcpQuotaHandler are skipped during validation and resume after (incl. after a thrown load), in serverUtilities.test.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --------- Co-authored-by: Kyle Bernhardy <kyle.bernhardy@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Commit changes to package-lock.json from running npm install

dbedff1

cap10morgan requested review from a team and Ethan-Arrowood October 17, 2025 18:32

Ethan-Arrowood approved these changes Oct 17, 2025

View reviewed changes

cap10morgan merged commit ef789ca into main Oct 17, 2025
3 checks passed

cap10morgan deleted the fix/sync-package-lock.json branch October 17, 2025 19:03

cap10morgan mentioned this pull request Nov 4, 2025

Decide on a test file naming convention #21

Closed

heskew mentioned this pull request Mar 26, 2026

Flaky test: 'invalidate and get from cache and check headers with loadAsInstance' in basicREST-test #280

Closed

claude Bot mentioned this pull request May 2, 2026

Eliminate TypeScript Errors and Convert foundational .js files to .ts #413

Merged

heskew mentioned this pull request Jun 4, 2026

Harden atomic config writes: createFileSync zero-byte window, symlink/bind-mount, mode preservation, and castConfigValue key mapping #1143

Open

claude Bot mentioned this pull request Jun 12, 2026

test(integration): add scale correctness suite (#1192) #1229

Merged

2 tasks

This was referenced Jun 26, 2026

fix(itc): await cross-worker propagation for role/schema mutations (#1497) #1499

Merged

fix(encoder): heal a diverged shared-structures dictionary on decode (durable-wins reload) #1506

Closed

kriszyp mentioned this pull request Jul 10, 2026

uWS backend (#1096): silent onUpgrade auth bypass, unconditional loopback IP on delegated routes, unbounded static-file buffering #1752

Open

kriszyp mentioned this pull request Jul 20, 2026

feat(deploy): two-phase stage/activate for deploy_component (draft/RFC) #1849

Open

kylebernhardy mentioned this pull request Jul 31, 2026

fix(components): preserve entry events across deploy scans #1936

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit changes to package-lock.json from running npm install - #3

Commit changes to package-lock.json from running npm install#3
cap10morgan merged 1 commit into
mainfrom
fix/sync-package-lock.json

cap10morgan commented Oct 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cap10morgan commented Oct 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants