ReviewOS

also looking at this

stacks/buddy

fix(deps-file): anchor the dependency regex to a key, and rewrite each match from its own captures

#1454
Merged glennmichael123 wants to merge fix/deps-file-anchored-key-regex into main
3 files +216 -41
9 days ago

Closes #1453.

updateDependencyFile could emit a deps.yaml that no longer parses, silently dropping a dependency key. Hit in stacksjs/bun-query-builder#1168, whose only check was a docs deploy, so it went green; the breakage would have surfaced as Setup Pantry failing before any job ran.

Before and after, on the issue's input

# That is not hypothetical: `sqlite: latest` was fine only because the SDK did
# not support sqlite.org and skipped it with a warning.
dependencies:
  bun: ^1.3.14
  mysql: ^8.0.45
  postgres: ^18.0.0
  sqlite: ^3.52.0

Applying postgres → 18.6 and sqlite → 3.53.4:

Before (YAML Parse error: Unexpected token, sqlite key gone):

# That is not hypothetical: `sqlite: latest`3.53.4 was fine only because the SDK did
# not support sqlite.org and skipped it with a warning.
dependencies:
  bun: ^1.3.14
  mysql: ^8.0.45
  postgres: ^18.6sqlite: latest`3.53.4

After (parses, comment byte-identical):

# That is not hypothetical: `sqlite: latest` was fine only because the SDK did
# not support sqlite.org and skipped it with a warning.
dependencies:
  bun: ^1.3.14
  mysql: ^8.0.45
  postgres: ^18.6
  sqlite: ^3.53.4

What changed

Two defects did the damage, and both are gone:

  1. The regex was not anchored to a key position. (\s*\bNAME\b\s*:\s*)(…) matched the name anywhere, including in prose, and that comment match was first, so its captures drove the replacement. Its greedy leading \s* also reached back across the newline before the real key, which is what collapsed two lines into one. It is now ^([ \t]*)(key)… with the m flag: the match is pinned to a key, and the indent cannot cross a line. A commented-out key (# redis: ^7.4.1) and a quoted neighbour ("@types/node" when updating node) no longer match either.
  2. A g regex was used with one replacement built from the first match, so the first occurrence's rendered text was written over every other occurrence. It is now a replacer function, so each match is rewritten from its own captures.

The other two defects in the issue follow from the anchor. A version is no longer captured with adjacent punctuation, so "*" and "latest" reach the dynamic-version guard instead of defeating it, and the /^(\D*)/ prefix read no longer treats a quote as part of the version. Quoting style is now preserved instead of stripped.

As defense in depth, the result is round-tripped through Bun.YAML.parse and the file is left untouched if the rewrite broke it. Deliberately only when this function is what broke it: a file that already did not parse is still updated, and a test pins that, so the guard cannot quietly stop updating a file for an unrelated reason.

The end of the line is deliberately not anchored, so a CRLF file still matches.

On the one test that changed

respect-latest.test.ts asserted toContain('python.org: "*"'), which was satisfied by the corrupt line python.org: "*"3.13.5. So it passed for months against output that Bun.YAML.parse rejects. It now parses the result before asserting anything, and checks that the new version appears nowhere.

Verification

  • Six new regression tests in regression-deps-constraint.test.ts, of which five fail against the previous implementation. The sixth pins the parse guard's scope, which had no previous behaviour to contradict.
  • Full suite: 2405 pass, 0 fail across 146 files.
  • tsc --noEmit clean, pickier clean.

🤖 Generated with Claude Code

3 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.