fix(drift): match path segments that mix literal text with parameters - #2994
fix(drift): match path segments that mix literal text with parameters#2994ariesclark wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: ecf7b3b The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Performance Benchmark (Lower is Faster)
|
There was a problem hiding this comment.
🟡 Changes recommended
The new time-based performance assertion is likely to be flaky on slower or contended CI runners and should be made more robust.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes drift/coverage path-template matching so segments that mix literal text with one or more {params} (e.g. /instances/{worldId}:{instanceId} or {name}.json) compile into a working regex rather than being treated as a literal segment.
Changes:
- Update
compileOpenApiPathto scan each segment for multiple{param}occurrences, escape literal runs between them, and compile safe capture groups that split on literal separators. - Adjust route specificity scoring to add a middle tier for “mixed literal + params” segments.
- Add focused unit tests (including a guard against catastrophic backtracking) and a changeset documenting the CLI behavior fix.
File summaries
| File | Description |
|---|---|
| packages/cli/src/commands/drift/utils/http.ts | Reworks OpenAPI path-template compilation to support mixed segments and improves scoring for route selection. |
| packages/cli/src/commands/drift/tests/compile-openapi-path.test.ts | Adds unit coverage for mixed segments, split behavior, scoring, and a performance regression guard. |
| .changeset/drift-multi-parameter-path-segments.md | Documents the patch-level CLI fix for drift/coverage matching. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const longPath = '/v1/' + 'a:'.repeat(2500) + 'a'; | ||
|
|
||
| const startedAt = performance.now(); | ||
| expect(regex.exec(longPath)).toBeNull(); | ||
| expect(performance.now() - startedAt).toBeLessThan(100); |
What/Why/How?
Note
This was heavily assisted by Anthropic's Opus 5. I've reviewed the code to the best of my ability, but if there's any obvious issues I didn't catch, let me know.
A path template whose segment mixes literal text with parameters never matched a request.
compileOpenApiPathtested each segment with/^\{([^}]+)\}$/, which recognizes only a segment that is entirely one parameter. Anything else, such as two parameters or a parameter beside literal text, fell through toescapeRegexand became a literal. So/instances/{worldId}:{instanceId}matched only a URL containing that raw text, which no request carries.driftreported those requests as undocumented andcoverageleft them out of its figures.The compiler now scans each segment for
\{([^}]+)\}, escapes the literal runs between matches, and emits([^/]+?)per parameter. Non-greedy is deliberate: with two parameters around a separator, the first must stop at the first occurrence rather than swallow to the last.Specificity scoring gains a middle tier. A pure literal still scores
+2and a bare parameter0, with+1for a segment holding both. The counter reads a per-segmentparamsBeforesnapshot, becauseparamsaccumulates across the whole template.Reference
None.
Testing
compile-openapi-path.test.tscovers single-parameter segments, the/boundary, two parameters in one segment, a multi-parameter segment with a following suffix, the first-separator split when the trailing value holds another separator, score ordering, and pure literals.Against a 140-exchange capture,
drift's false "undocumented endpoint" reports went from 3 to 0 andcoveragerose from 87 to 90 operations.driftnow validates the three newly matched responses.Check yourself
Security
The change only widens which documented paths a recorded request can match, and adds no new input handling.
Note
Low Risk
Matching logic is widened for documented paths only, with no new external input surfaces; regression risk is limited to path-template resolution in drift/coverage.
Overview
compileOpenApiPathno longer treats a path segment as either a single{param}or a full literal. It now finds every{param}in a segment, escapes literal runs, and builds regexes that split on separators between parameters (e.g./instances/{worldId}:{instanceId}).driftandcoveragecan match captured requests against those templates again instead of flagging them as undocumented or excluding them from coverage. Specificity scoring adds a middle tier (+1) for mixed literal/parameter segments so they rank above bare-parameter paths.New unit tests in
compile-openapi-path.test.tscover multi-parameter segments, trailing literals, adjacent params, performance on long non-matches, and score ordering. A changeset documents the patch for@redocly/cli.Reviewed by Cursor Bugbot for commit ecf7b3b. Bugbot is set up for automated code reviews on this repo. Configure here.