Add issue templates - #1
Merged
Merged
Conversation
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>
7 tasks
This was referenced May 19, 2026
This was referenced May 23, 2026
2 tasks
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>
This was referenced Jul 10, 2026
This was referenced Jul 18, 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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add some basic Issue templates to get us started.