311: MSSQL spike: check dependencies (mssql, node-sql-parser, antlr4, T-SQL grammar) #334

Merged

david merged 2 commits from feature/issue-311/mssql-spike-check-dependencies-mssql-node-sql-pars into main

2026-11-10 22:24:13 +00:00

david commented

2026-11-10 22:24:11 +00:00

Owner

311: MSSQL spike: check dependencies (mssql, node-sql-parser, antlr4, T-SQL grammar)

Implements 311.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 1m 13s.

Review findings

Review Findings — 311

Summary

I fixed all six findings in extensions/mssql/spike/dependency-check.md, and the change is documentation only. I moved the issue 312 CI note into a new file, ci-coverage.md, and left dependency-check.md covering issue 311 alone. I changed the advisory table cells to "None in npm audit" and added a caveat that tedious advisories were not checked separately. The TypeScript runtime question is now written up as an open question for Stage 1. I renamed the blockers section "Risks and open points" and said that none of the points is blocking. The re-check commands now include npm audit and the git log command for sql/tsql/. The repository has no build or lint script covering this file, so I ran no gates.

Critical

High

Medium

extensions/mssql/spike/dependency-check.md:3 — The new section was prepended above an unrelated "Earlier note (issue 312)" section, so this one file now holds two spikes under the top-level title "MSSQL spike: dependency check". The CI note is the only content a reader would find under that name before this change. Suggested fix: move the issue 312 content into its own file (for example ci-coverage.md), or retitle the file and its sections so each issue's scope is clear.
extensions/mssql/spike/dependency-check.md:17 — The advisory check is incomplete. The document admits that the GitHub Security Advisories pages were not browsed, and npm audit on a scratch install covers only the npm advisory database. The "None found" entries in the table therefore overstate the confidence. Suggested fix: either check those advisory pages and the tedious advisories, or change the table cells to "None in npm audit" and keep the manual-check caveat.

Low

extensions/mssql/spike/dependency-check.md:30 — The TypeScript/antlr4 runtime compatibility point is left as "may expect antlr4ng or antlr4ts". That is the main adoption question for the grammar. Suggested fix: state the outcome (grammar generation targets, and which runtime package each needs) or list it explicitly as an open question for the next stage.
extensions/mssql/spike/dependency-check.md:36 — The blockers section says "None found", yet the list below it is a set of risks. The antlr4 release gap and the 92 MB install size could be read as blockers, which makes the heading ambiguous. Suggested fix: rename the section "Risks and open points" and say whether any of them is blocking.
extensions/mssql/spike/dependency-check.md:42 — The re-check commands cover only version and licence. They don't cover the audit or the grammar commit date that the document relies on. Suggested fix: add npm audit on a scratch install, and the git log command for sql/tsql/ in antlr/grammars-v4.

## 311: MSSQL spike: check dependencies (mssql, node-sql-parser, antlr4, T-SQL grammar) Implements [311](https://git.excelera.net/david/pi-extensions-and-skills/issues/311). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 1m 13s. ## Review findings # Review Findings — 311 ## Summary I fixed all six findings in `extensions/mssql/spike/dependency-check.md`, and the change is documentation only. I moved the issue 312 CI note into a new file, `ci-coverage.md`, and left `dependency-check.md` covering issue 311 alone. I changed the advisory table cells to "None in `npm audit`" and added a caveat that `tedious` advisories were not checked separately. The TypeScript runtime question is now written up as an open question for Stage 1. I renamed the blockers section "Risks and open points" and said that none of the points is blocking. The re-check commands now include `npm audit` and the `git log` command for `sql/tsql/`. The repository has no build or lint script covering this file, so I ran no gates. ## Critical ## High ## Medium - [x] extensions/mssql/spike/dependency-check.md:3 — The new section was prepended above an unrelated "Earlier note (issue 312)" section, so this one file now holds two spikes under the top-level title "MSSQL spike: dependency check". The CI note is the only content a reader would find under that name before this change. Suggested fix: move the issue 312 content into its own file (for example `ci-coverage.md`), or retitle the file and its sections so each issue's scope is clear. - [x] extensions/mssql/spike/dependency-check.md:17 — The advisory check is incomplete. The document admits that the GitHub Security Advisories pages were not browsed, and `npm audit` on a scratch install covers only the npm advisory database. The "None found" entries in the table therefore overstate the confidence. Suggested fix: either check those advisory pages and the `tedious` advisories, or change the table cells to "None in npm audit" and keep the manual-check caveat. ## Low - [x] extensions/mssql/spike/dependency-check.md:30 — The TypeScript/`antlr4` runtime compatibility point is left as "may expect `antlr4ng` or `antlr4ts`". That is the main adoption question for the grammar. Suggested fix: state the outcome (grammar generation targets, and which runtime package each needs) or list it explicitly as an open question for the next stage. - [x] extensions/mssql/spike/dependency-check.md:36 — The blockers section says "None found", yet the list below it is a set of risks. The `antlr4` release gap and the 92 MB install size could be read as blockers, which makes the heading ambiguous. Suggested fix: rename the section "Risks and open points" and say whether any of them is blocking. - [x] extensions/mssql/spike/dependency-check.md:42 — The re-check commands cover only version and licence. They don't cover the audit or the grammar commit date that the document relies on. Suggested fix: add `npm audit` on a scratch install, and the `git log` command for `sql/tsql/` in `antlr/grammars-v4`.

david added 2 commits

2026-11-10 22:24:12 +00:00