chore: Full codebase review for v1.2.0

[adrozdenko]

CodeRabbit Full Codebase Review

This PR contains the entire pactwork codebase for comprehensive review.

What's included

• Core CLI (src/cli/) — Commands: generate, validate, breaking, types, scenarios, etc.
• Core Modules (src/core/) — Parser, generator, validator, reporter, contracts, scenarios
• Runtime Utilities (src/runtime/) — applyScenario, withLatency, withSequence, etc.
• Storybook Addon (packages/storybook-addon/) — Toolbar controls + observability panel

Version

• pactwork: v1.2.0
• @pactwork/storybook-addon: v1.0.0

Test Coverage

• 180 tests in main package
• 63 tests in storybook-addon
• Total: 243 tests passing

---

@coderabbitai full review

Summary by CodeRabbit

• New Features

  • Storybook addon, a public GitHub Action, new PR/issue templates, and six CLI commands: types, breaking, record, verify, scenarios, coverage.

• Runtime

  • Composable runtime transforms: scenario application, latency, sequencing, rate-limiting, network-error simulation, deterministic seeding, and pipeline utilities.

• Coverage

  • Scenario coverage reporting with console/JSON/Markdown/GitHub outputs and optional minimum-threshold enforcement.

• Documentation

  • Major docs rewrite (README, ROADMAP, CONTRIBUTING, SECURITY, CODE_OF_CONDUCT, CHANGELOG) and many guidance pages.

• CI & Chores

  • CI workflow, package version bump, tooling and build configuration updates.

• Tests

  • Extensive new test suites across addon, runtime, coverage, typegen, validator, reporter, and CLI.

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/generator/index.ts (1)

163-187: 🧹 Nitpick | 🔵 Trivial

Minor: Regex replacement could be more precise.

The regex Spec hash: .+ on line 183 will match to the end of the line, which is the intended behavior. However, if the file contains multiple occurrences of "Spec hash:", only the first will be replaced due to not using the g flag. This is likely fine since generated files should only have one header, but worth noting.

♻️ Optional: Use more precise replacement

-      content = content.replace(/Spec hash: .+/, `Spec hash: ${specHash}`)
+      content = content.replace(/^\/\/ Spec hash: .+$/m, `// Spec hash: ${specHash}`)

🤖 Fix all issues with AI agents

In `@docs/vault/anti-patterns/typescript/case-block-lexical-declarations.md`:
- Line 43: Replace the imprecise word "hoisted" in the sentence about
declarations in switch statements with a clearer explanation: state that
`const`/`let` are scoped to the switch statement’s single block (not hoisted
per-case), which causes Temporal Dead Zone (TDZ) behavior and
duplicate-declaration errors across `case` clauses; update the sentence so it
reads something like “Without braces, `const`/`let` are scoped to the switch
statement (same block), causing TDZ and duplicate declaration errors across
cases.” Ensure the terms `const`, `let`, `switch`, `case`, and TDZ are used to
make the scope behavior explicit.

In `@docs/vault/patterns/typescript/cycle-detection-in-schema-comparison.md`:
- Around line 22-38: Update the example to demonstrate recursive resolution and
how the visited Set is propagated: keep the existing resolveRef function but add
a resolveAllRefs wrapper that calls resolveRef(schema, schemas, visited), then
recursively iterates resolved.properties and calls resolveAllRefs(prop, schemas,
visited) to replace nested refs, and include a brief concrete circular-reference
scenario comment showing resolveAllRefs returning null when a ref is revisited;
reference resolveRef, resolveAllRefs, and the visited parameter so readers can
locate and follow the propagation.

In `@packages/storybook-addon/src/panel-utils.ts`:
- Around line 10-18: getMethodColor currently matches only exact uppercase
strings so lowercase or mixed-case inputs fall to the default; normalize the
incoming method inside getMethodColor (e.g., call method = method?.toUpperCase()
or toLowerCase() and then switch against the normalized values) so matching is
case-insensitive and existing cases ('GET','POST','PUT','DELETE') still work;
ensure you handle null/undefined safely before normalization.

In `@packages/storybook-addon/src/types.ts`:
- Around line 146-151: HandlerInfo.interface currently types method as string;
replace that with the runtime HTTP method union type exported by your runtime
types (e.g., HttpMethod or HttpVerb) by importing that type and using it for the
HandlerInfo.method property, update the import list at the top of the file to
include the union type, and run type checks to ensure HandlerInfo now enforces
only valid HTTP methods.

In `@src/core/typegen/index.ts`:
- Around line 121-126: The code building parameter keys uses manual single-quote
interpolation for non-identifier names which doesn't escape special characters;
update the key generation in the params loop (where param.name, optional,
paramType, and ctx.lines.push are used) to use JSON.stringify(param.name)
instead of `'${param.name}'` so names are consistently and safely quoted/escaped
before pushing `  ${key}${optional}: ${paramType};` into ctx.lines; keep the
identifier-regex branch intact and only replace the quoted branch with
JSON.stringify.

In `@src/runtime/handler-utils.ts`:
- Around line 180-184: The TypeScript cast from HttpHandler to Record<string,
unknown> is flagged; change the intermediate cast to go through unknown to
satisfy the compiler by updating the assignment for handlerAny to use a
double-cast (handler as unknown as Record<string, unknown>) so you can then read
handlerAny.resolver into resolver as before (keep the resolver variable and its
type unchanged).

In `@src/runtime/types.ts`:
- Around line 157-161: The HandlerFactory declaration uses a plain string for
the method parameter; change its type to the existing HTTP method union used by
HandlerMetadata (e.g. HandlerMetadata['method'] or the shared HTTPMethod union)
so method is strictly typed; update the HandlerFactory signature (function name:
HandlerFactory, parameter: method) to use that union type and ensure existing
callsites conform to the narrower type.

[coderabbitai]

✅ Actions performed

Full review triggered.

[adrozdenko]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[adrozdenko]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@docs/vault/patterns/typescript/cycle-detection-in-schema-comparison.md`:
- Around line 1-12: Add a file-level markdownlint disable for MD041 to the top
of this frontmatter file so the frontmatter first-line rule is suppressed;
specifically insert a single HTML comment like <!-- markdownlint-disable MD041
--> (or equivalent project-config directive) at the very start of the file
containing id: pattern-typescript-cycle-detection-in-schema-comparison / title:
Cycle Detection in Recursive Schema Comparison so the linter ignores the
frontmatter-first-line heading rule.

In `@packages/storybook-addon/src/preview.ts`:
- Around line 63-66: The async event handlers bound to the Storybook channel
(handleScenarioChange, handleLatencyChange, handleNetworkChange) can reject and
cause unhandled promise rejections; update the channel.on registrations so each
callback runs the async handler and attaches a .catch() to swallow/log errors
(e.g., channel.on(EVENTS.SCENARIO_CHANGE, (...args) =>
handleScenarioChange(...args).catch(err => /* log */)); do this for
handleScenarioChange, handleLatencyChange, and handleNetworkChange while leaving
resetHandlers as-is.

In `@src/core/generator/index.ts`:
- Around line 42-48: The catch block that unconditionally falls back to
generateBasicHandlers should instead detect whether the subprocess error
indicates a missing binary (error.code === 'ENOENT') and only then log the
fallback and call generateBasicHandlers(spec, options); for any other error
rethrow it so failures surface; also log the fallback message unconditionally
(not only when options.verbose) and include the actual error message (use error
instanceof Error ? error.message : String(error)) in the log; locate and update
the catch handling around the msw-auto-mock subprocess invocation and the
generateBasicHandlers call to implement this logic.

[coderabbitai]

✅ Actions performed

Full review triggered.

[adrozdenko]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[adrozdenko]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[adrozdenko]

@coderabbitai full review

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/cli/commands/verify.ts`:
- Around line 40-42: The CLI currently lets the config file override the CLI
flag because the ContractStore is constructed with config?.contracts?.dir before
options.contract; change the precedence to use options.contract ??
config?.contracts?.dir ?? DEFAULTS.CONTRACTS_DIR so CLI option wins. Also
inspect what config.contracts.dir contains: because ContractStore (constructor
and methods referenced as ContractStore) appends "/contracts" internally, pass
either the parent directory (use a dirname of config.contracts.dir) or the
explicit contracts directory as appropriate to avoid double-nesting; adjust the
value you pass into new ContractStore accordingly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect ContractStore class and constructor
rg -n "class ContractStore" src/ -A 20

Repository: adrozdenko/pactwork

Length of output: 297

---

🏁 Script executed:

#!/bin/bash
# Check the constructor implementation more closely
rg -n "constructor" src/core/contracts/index.ts -A 10

Repository: adrozdenko/pactwork

Length of output: 292

---

🏁 Script executed:

#!/bin/bash
# Look at the entire ContractStore class to understand baseDir handling
fd "index.ts" src/core/contracts/ -x cat -n {}

Repository: adrozdenko/pactwork

Length of output: 4301

---

🏁 Script executed:

#!/bin/bash
# Check how other CLI commands use ContractStore (e.g., init.ts, start.ts)
fd "init.ts\|start.ts" src/cli/commands/ -x cat -n {}

Repository: adrozdenko/pactwork

Length of output: 45

---

🏁 Script executed:

#!/bin/bash
# Look at DEFAULTS.CONTRACTS_DIR value
rg -n "DEFAULTS.*CONTRACTS_DIR" src/

Repository: adrozdenko/pactwork

Length of output: 384

---

Fix precedence: CLI option should override config file setting.

The precedence order is reversed—CLI options should take priority. Currently config?.contracts?.dir is checked first, allowing config to override options.contract. This should be:

options.contract ?? config?.contracts?.dir ?? DEFAULTS.CONTRACTS_DIR

Additionally, verify what config.contracts.dir actually points to. If it contains the full path to a contracts directory (e.g., .pactwork/contracts), passing it directly to ContractStore may result in double-nesting since ContractStore appends /contracts internally at lines 22, 37, 53, 67, 88. If config stores the parent directory path, the suggested dirname() approach from the original review may be unnecessary, but the precedence fix is required either way.

🤖 Prompt for AI Agents

In `@src/cli/commands/verify.ts` around lines 40 - 42, The CLI currently lets the
config file override the CLI flag because the ContractStore is constructed with
config?.contracts?.dir before options.contract; change the precedence to use
options.contract ?? config?.contracts?.dir ?? DEFAULTS.CONTRACTS_DIR so CLI
option wins. Also inspect what config.contracts.dir contains: because
ContractStore (constructor and methods referenced as ContractStore) appends
"/contracts" internally, pass either the parent directory (use a dirname of
config.contracts.dir) or the explicit contracts directory as appropriate to
avoid double-nesting; adjust the value you pass into new ContractStore
accordingly.

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/cli/commands/validate.ts (1)

11-20: 🧹 Nitpick | 🔵 Trivial

Consider exporting ValidateOptions for API consistency.

Other CLI commands (e.g., VerifyOptions, ScenariosOptions) export their options interfaces. For programmatic API consistency, consider exporting ValidateOptions as well.

♻️ Proposed fix

-interface ValidateOptions {
+export interface ValidateOptions {

🤖 Fix all issues with AI agents

In `@packages/storybook-addon/src/Panel.test.tsx`:
- Around line 193-208: The test defines a local MAX_LOG_ENTRIES constant which
can drift from the implementation; instead export the constant from the Panel
implementation (export const MAX_LOG_ENTRIES) and import it into the test
(replace the local const with the imported MAX_LOG_ENTRIES) so the test uses the
authoritative value used by the Panel component and will fail if the
implementation changes; update the test cases that reference MAX_LOG_ENTRIES and
any variable names (e.g., entries, newEntry, updated) to use the imported
constant.

In `@src/cli/commands/types.ts`:
- Around line 54-55: The call to ensure the directory uses
path.dirname(outputPath) but outputPath is built as path.join(outputDir,
'types.ts'), so simplify by calling await fs.ensureDir(outputDir) instead of
await fs.ensureDir(path.dirname(outputPath)); update the ensureDir invocation
wherever outputPath is computed (reference symbols: outputPath, outputDir,
fs.ensureDir, path.dirname) to directly use outputDir for clarity and slight
performance improvement.

In `@src/core/config/index.ts`:
- Around line 25-30: Update ConfigLoadError to use the native Error cause
option: change the constructor to call super(message, { cause }) instead of
assigning a manual public readonly cause field (e.g., in class ConfigLoadError's
constructor use super(message, { cause }) and remove the manual this.cause
assignment/field), preserve this.name = 'ConfigLoadError' and keep the
constructor signature (message: string, cause?: unknown) so callers continue to
work.

In `@src/core/reporter/index.ts`:
- Line 115: The console.log call is producing a duplicated message because the
literal "Validation failed with " is prefixed outside and repeated inside
escapeGitHubAnnotation; update the call so the prefix appears only once—either
pass only the interpolated message to escapeGitHubAnnotation (e.g.,
escapeGitHubAnnotation(`Validation failed with ${errorCount} error(s)`) and
prepend the ::error:: prefix outside) or remove the outer literal and include
the ::error:: prefix inside the escaped string; modify the line that calls
escapeGitHubAnnotation and uses errorCount so the final logged string reads
"::error::Validation failed with X error(s)" exactly once.

In `@src/runtime/with-network-error.ts`:
- Around line 213-215: The input validation for failureProbability is
incomplete: add a Number.isFinite(failureProbability) check alongside the
existing range checks in the with-network-error logic so NaN/Infinity are
rejected; update the condition that currently reads "if (failureProbability < 0
|| failureProbability > 1)" to also verify finite numeric input (e.g., using
Number.isFinite) and throw the same Error('failureProbability must be between 0
and 1') when that check fails so the error injection behavior in the
with-network-error module correctly rejects NaN/Infinity.

[adrozdenko]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@docs/vault/_schema.md`:
- Around line 5-17: Add documentation for the related_pattern frontmatter field:
update the field list explanation to include "related_pattern" and note it's
optional and points to a related pattern or anti-pattern (e.g.,
"related_pattern: anti-pattern-xxx  # optional: links to related
pattern/anti-pattern"), and update the example frontmatter to include that
inline comment; ensure references to "related_pattern" appear alongside other
fields (id, title, category, severity, tags, applies_to, created) in the
_schema.md content so readers see it documented consistently.

In `@package.json`:
- Around line 54-56: Update project docs and CI to reflect the package.json
engines change: add a clear "Minimum Node version: >=20.11.0" statement to
README.md and any relevant setup docs, and modify the CI workflow
(.github/workflows/ci.yml) to pin the runner node-version to '20.11.0' (or a
later explicit version) instead of the generic '20' so CI matches the "node":
">=20.11.0" constraint in package.json; ensure the README wording matches the
exact string from package.json and that the workflow's node-version key uses the
same version syntax.

In `@SECURITY.md`:
- Around line 21-27: The claim about no network requests is wrong because
SwaggerParser.validate() is invoked with default options, which allows external
$ref resolution and will make HTTP/HTTPS requests; fix by either (A) disabling
external resolution by calling SwaggerParser.validate(spec, { resolve: {
external: false } }) where SwaggerParser.validate() is called (the parser
module), or (B) update SECURITY.md to explicitly state that external network
requests may occur when OpenAPI specs contain external $ref references; locate
the SwaggerParser.validate() usage in the parser code and apply option (A) or
update SECURITY.md as described.

In `@src/core/scenarios/types.ts`:
- Around line 1-4: Header comment "Scenario types for Pactwork Phase 2" is
inconsistent with PR context mentioning Phase 3; update the top-of-file header
comment (the header comment block containing the string "Scenario types for
Pactwork Phase 2") to reference "Phase 3" (or the correct phase name) so
documentation matches the AI summary and PR context.

In `@src/runtime/with-sequence.ts`:
- Around line 77-78: The code indexes MSW's http object using methodLower when
creating sequenceHandler (const methodLower = method.toLowerCase() ...; const
sequenceHandler = http[methodLower](...)), but it lacks runtime validation that
methodLower is a supported key; update with the same defensive check used in
createHandler: verify that typeof http[methodLower] === "function" (or
methodLower in Object.keys(http)) before calling it, and if invalid throw a
clear error mentioning the unsupported method and the handler metadata; this
keeps methodLower/http/sequenceHandler semantics but prevents runtime crashes
when an unexpected method is passed.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if Node version requirement is documented in README
echo "=== Checking README.md for Node version ==="
if [ -f README.md ]; then
  rg -n "20\.11|node.*20" README.md
else
  echo "README.md not found"
fi

# Check CI configuration for Node version
echo -e "\n=== Checking CI configurations ==="
if [ -d .github/workflows ]; then
  fd -e yml -e yaml . .github/workflows --exec grep -l "node" {} \;
else
  echo ".github/workflows not found"
fi

# Also check for any node version specifications in CI files
echo -e "\n=== Node version in CI files ==="
if [ -d .github/workflows ]; then
  fd -e yml -e yaml . .github/workflows --exec grep -A2 -B2 "node" {} \; 2>/dev/null
fi

Repository: adrozdenko/pactwork

Length of output: 807

---

Document Node 20.11.0 minimum version requirement.

The engine requirement raises the minimum to >=20.11.0, but this is not documented in README.md. Additionally, CI configurations in .github/workflows/ci.yml use generic version 20 instead of the specific minimum 20.11.0, which could allow Node 20.10.x to run in CI and mask compatibility issues. Update README.md with the new minimum version requirement and adjust CI to explicitly use node-version: '20.11.0' (or later) to match the package.json constraint.

🤖 Prompt for AI Agents

In `@package.json` around lines 54 - 56, Update project docs and CI to reflect the
package.json engines change: add a clear "Minimum Node version: >=20.11.0"
statement to README.md and any relevant setup docs, and modify the CI workflow
(.github/workflows/ci.yml) to pin the runner node-version to '20.11.0' (or a
later explicit version) instead of the generic '20' so CI matches the "node":
">=20.11.0" constraint in package.json; ensure the README wording matches the
exact string from package.json and that the workflow's node-version key uses the
same version syntax.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for parseSpec implementation and SwaggerParser configuration
echo "=== Searching for parseSpec implementation ==="
fd -t f "index.ts|parser.ts" src/ | head -20

echo "=== Looking for SwaggerParser usage and configuration ==="
rg -n "SwaggerParser|resolve.*external" --type ts -A 5 -B 2 src/

echo "=== Checking for any parseSpec function ==="
rg -n "parseSpec\(" --type ts -A 10 -B 2 src/

Repository: adrozdenko/pactwork

Length of output: 11333

---

🏁 Script executed:

#!/bin/bash
# Look for the actual parser configuration
echo "=== Search for swagger parser imports and usage ==="
rg -n "swagger-parser|SwaggerParser" --type ts src/

echo "=== Check for resolve configuration ==="
rg -n "resolve:" --type ts -B 3 -A 3 src/

Repository: adrozdenko/pactwork

Length of output: 436

---

🏁 Script executed:

#!/bin/bash
# List files in src/core/parser to understand structure
echo "=== Files in parser directory ==="
fd -t f . src/core/parser/

echo "=== Content of main parser files ==="
for file in $(fd -t f . src/core/parser/ | head -5); do
  echo "=== File: $file ==="
  wc -l "$file"
done

Repository: adrozdenko/pactwork

Length of output: 429

---

🏁 Script executed:

cat -n src/core/parser/index.ts

Repository: adrozdenko/pactwork

Length of output: 7764

---

Update SECURITY.md to accurately reflect network request behavior or configure SwaggerParser to disable external resolution.

The current claim "Does not make network requests except when explicitly configured" is inaccurate. SwaggerParser.validate() at line 15 of src/core/parser/index.ts is called without any configuration options, meaning it uses default behavior which dereferences all $ref values—including external HTTP/HTTPS references that trigger network requests.

Either:

1. Pass { resolve: { external: false } } to SwaggerParser.validate() to disable external resolution, or
2. Update the documentation to acknowledge that network requests will occur when the OpenAPI spec contains external $ref values

🤖 Prompt for AI Agents

In `@SECURITY.md` around lines 21 - 27, The claim about no network requests is
wrong because SwaggerParser.validate() is invoked with default options, which
allows external $ref resolution and will make HTTP/HTTPS requests; fix by either
(A) disabling external resolution by calling SwaggerParser.validate(spec, {
resolve: { external: false } }) where SwaggerParser.validate() is called (the
parser module), or (B) update SECURITY.md to explicitly state that external
network requests may occur when OpenAPI specs contain external $ref references;
locate the SwaggerParser.validate() usage in the parser code and apply option
(A) or update SECURITY.md as described.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Minor documentation inconsistency: "Phase 2" vs "Phase 3".

The file header comment references "Phase 2" but the AI summary and related PR context reference "Phase 3". Consider updating for consistency if this is indeed Phase 3.

🤖 Prompt for AI Agents

In `@src/core/scenarios/types.ts` around lines 1 - 4, Header comment "Scenario
types for Pactwork Phase 2" is inconsistent with PR context mentioning Phase 3;
update the top-of-file header comment (the header comment block containing the
string "Scenario types for Pactwork Phase 2") to reference "Phase 3" (or the
correct phase name) so documentation matches the AI summary and PR context.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/cli/commands/init.ts (1)

122-159: ⚠️ Potential issue | 🟠 Major

Escape paths when generating config to support Windows and special characters.

Interpolating raw paths into single-quoted JS/TS strings breaks when paths contain backslashes or single quotes. Windows paths like C:\Users\test\spec.yaml will cause \t and \U sequences to be misinterpreted as escape sequences. Use JSON.stringify() to safely escape both values.

🛠️ Suggested fix

 function generateConfigFile(options: ConfigOptions): string {
   const { specPath, outputDir, typescript } = options
+  const specLiteral = JSON.stringify(specPath)
+  const outputLiteral = JSON.stringify(outputDir)

   if (typescript) {
     return `import { defineConfig } from 'pactwork'

 export default defineConfig({
   spec: {
-    path: '${specPath}',
+    path: ${specLiteral},
   },
   generate: {
-    output: '${outputDir}',
+    output: ${outputLiteral},
     typescript: true,
   },
   contracts: {
     dir: '.pactwork/contracts',
     consumer: 'frontend',
     provider: 'api',
   },
 })
 `
   }

   return `/** `@type` {import('pactwork').PactworkConfig} */
 export default {
   spec: {
-    path: '${specPath}',
+    path: ${specLiteral},
   },
   generate: {
-    output: '${outputDir}',
+    output: ${outputLiteral},
     typescript: false,
   },
   contracts: {
     dir: '.pactwork/contracts',
     consumer: 'frontend',
     provider: 'api',
   },
 }
 `
 }

src/core/generator/index.ts (1)

98-100: ⚠️ Potential issue | 🟡 Minor

Allow maxArrayLength = 0 to pass through.

The truthy check skips valid zero values.

Suggested fix

-  if (options.maxArrayLength) {
+  if (options.maxArrayLength !== undefined) {
     args.push('--max-array-length', String(options.maxArrayLength))
   }

🤖 Fix all issues with AI agents

In @.gitignore:
- Around line 21-22: Update the .gitignore entry for test coverage: replace the
root-anchored pattern "/coverage/" with a non-anchored "coverage/" so coverage
directories are ignored recursively throughout the repo (e.g.,
src/core/coverage/ and packages/storybook-addon/coverage/); locate and edit the
"/coverage/" line in .gitignore and change it to "coverage/".

In `@SECURITY.md`:
- Around line 21-27: The SECURITY.md claim that the app "does not make network
requests except when explicitly configured" is inaccurate because
src/core/parser/index.ts calls SwaggerParser.validate(specPath) which resolves
external $ref URLs by default; fix this by changing the call to pass options to
disable external resolution (use SwaggerParser.validate(specPath, { resolve: {
external: false } })) so external HTTP/HTTPS fetching is turned off, and update
SECURITY.md line 27 to either accurately state the default behavior or note that
external resolution is explicitly disabled in the parser; target symbols:
SwaggerParser.validate and the SECURITY.md statement about network requests.

In `@src/core/validator/index.ts`:
- Around line 125-131: The comment claims MSW-style params are normalized but
the code only strips template literals and normalizes the leading slash; update
the normalization by converting MSW path params (colon-prefixed segments like
:id) into OpenAPI-style brace params (e.g., {id}) where urlPath is manipulated,
or alternatively change the comment to remove the claim about MSW params;
specifically, in the normalization block that modifies urlPath (the same place
that does the template literal strip and leading-slash fix), implement a
transformation that replaces any `:param` path segment with `{param}` so
pathsMatch and subsequent comparisons see consistent param syntax.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's see the full context of the .gitignore file
cat -n .gitignore | head -30

Repository: adrozdenko/pactwork

Length of output: 537

---

🏁 Script executed:

# Check the overall directory structure to understand the monorepo layout
fd -t d -L --max-depth 2 | head -20

Repository: adrozdenko/pactwork

Length of output: 176

---

🏁 Script executed:

# Search for all coverage directories in the repository
fd -t d '^coverage$' --color=never

Repository: adrozdenko/pactwork

Length of output: 81

---

🏁 Script executed:

# Check if there are .gitignore files in subdirectories
find . -name .gitignore -type f | grep -v '.git/'

Repository: adrozdenko/pactwork

Length of output: 75

---

Fix gitignore pattern to ignore coverage directories throughout the repository.

The current pattern /coverage/ only ignores the coverage directory at the repository root. However, a coverage directory exists at src/core/coverage/ that will not be ignored by this pattern and could be accidentally committed.

Change the pattern from /coverage/ to coverage/ (without the leading slash) to ignore coverage directories everywhere in the repository, including in src/core/ and packages/storybook-addon/.

Current pattern

# Test coverage (root-level only)
/coverage/

🤖 Prompt for AI Agents

In @.gitignore around lines 21 - 22, Update the .gitignore entry for test
coverage: replace the root-anchored pattern "/coverage/" with a non-anchored
"coverage/" so coverage directories are ignored recursively throughout the repo
(e.g., src/core/coverage/ and packages/storybook-addon/coverage/); locate and
edit the "/coverage/" line in .gitignore and change it to "coverage/".

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find SwaggerParser calls and their configuration

# First, locate the parser files
find src -name "*parser*" -type f | head -20

# Search for SwaggerParser usage
rg -n "SwaggerParser" src/ -A 5 -B 2

# Check specifically for resolve.external configuration
rg -n "resolve.*external|external.*false" src/ -A 2 -B 2

Repository: adrozdenko/pactwork

Length of output: 1530

---

🌐 Web query:

SwaggerParser validate default behavior external $ref resolution

💡 Result:

For @apidevtools/swagger-parser (the JS/TS library that exports SwaggerParser):

• SwaggerParser.validate() calls dereference() internally, so it will resolve and dereference $ref as part of validation. [2]
• By default it will also follow external $ref (other files / URLs). This is explicitly called out as the default security posture (“attempts to resolve any files referenced using $ref”). [4]
• To prevent external $ref resolution, disable it via options:
  • Newer docs: resolve: { external: false } (external refs are then “ignored”). [1]
  • Older/alternate option shape (documented in the package docs): $refs: { external: false } (default is true). [3]

Sources: [1] Options docs, [2] SwaggerParser.validate() docs, [3] package docs showing $refs.external default, [4] security note about default external resolution.

---

Update SECURITY.md line 27 to accurately reflect that SwaggerParser.validate() makes network requests for external $ref URLs by default.

The code at src/core/parser/index.ts:15 calls SwaggerParser.validate(specPath) without disabling external resolution. By default, @apidevtools/swagger-parser fetches HTTP/HTTPS URLs referenced via $ref, which contradicts the documented claim that it "does not make network requests except when explicitly configured." Either fix the code to pass { resolve: { external: false } } to disable external resolution, or update the documentation to accurately state that external references are resolved by default.

🤖 Prompt for AI Agents

In `@SECURITY.md` around lines 21 - 27, The SECURITY.md claim that the app "does
not make network requests except when explicitly configured" is inaccurate
because src/core/parser/index.ts calls SwaggerParser.validate(specPath) which
resolves external $ref URLs by default; fix this by changing the call to pass
options to disable external resolution (use SwaggerParser.validate(specPath, {
resolve: { external: false } })) so external HTTP/HTTPS fetching is turned off,
and update SECURITY.md line 27 to either accurately state the default behavior
or note that external resolution is explicitly disabled in the parser; target
symbols: SwaggerParser.validate and the SECURITY.md statement about network
requests.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Comment says MSW params are normalized but code doesn’t.

Either implement the :param → {param} normalization or adjust the comment to avoid confusion.

♻️ Possible implementation

-    // Convert MSW path params :id to OpenAPI style {id} for comparison
-    // (we'll handle this in pathsMatch, but normalize here too)
-    urlPath = urlPath.replace(/^\/+/, '/') // Ensure single leading slash
+    // Normalize leading slash and MSW-style params for consistent matching
+    urlPath = urlPath
+      .replace(/^\/+/, '/')
+      .replace(/:([A-Za-z0-9_]+)/g, '{$1}')

📝 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

	// Strip template literal variables like ${baseURL}, ${BASE_URL}, etc.
	urlPath = urlPath.replace(/\$\{[^}]+\}/g, '')
	// Convert MSW path params :id to OpenAPI style {id} for comparison
	// (we'll handle this in pathsMatch, but normalize here too)
	urlPath = urlPath.replace(/^\/+/, '/') // Ensure single leading slash
	// Strip template literal variables like ${baseURL}, ${BASE_URL}, etc.
	urlPath = urlPath.replace(/\$\{[^}]+\}/g, '')
	// Normalize leading slash and MSW-style params for consistent matching
	urlPath = urlPath
	.replace(/^\/+/, '/')
	.replace(/:([A-Za-z0-9_]+)/g, '{$1}')

🤖 Prompt for AI Agents

In `@src/core/validator/index.ts` around lines 125 - 131, The comment claims
MSW-style params are normalized but the code only strips template literals and
normalizes the leading slash; update the normalization by converting MSW path
params (colon-prefixed segments like :id) into OpenAPI-style brace params (e.g.,
{id}) where urlPath is manipulated, or alternatively change the comment to
remove the claim about MSW params; specifically, in the normalization block that
modifies urlPath (the same place that does the template literal strip and
leading-slash fix), implement a transformation that replaces any `:param` path
segment with `{param}` so pathsMatch and subsequent comparisons see consistent
param syntax.