318: MSSQL spike: write RESULT.md, apply the gate, open the Stage 1 PR #340

Merged

david merged 2 commits from feature/issue-318/mssql-spike-write-result-md-apply-the-gate-open-th into main

2026-11-10 23:12:40 +00:00

david commented

2026-11-10 23:12:38 +00:00

Owner

318: MSSQL spike: write RESULT.md, apply the gate, open the Stage 1 PR

Implements 318.

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 09s.

Review findings

Review Findings — 318

Summary

I fixed the High finding and the two Low findings, all in documentation. In RESULT.md I corrected the CI coverage claim, added the install and generate commands, and split the two risk tags. I also corrected the contradicting follow-up in ci-coverage.md. I did not run any gates, because only documentation changed and the repo has no build or lint script.

Two Medium findings remain. Both concern commit and PR naming and the Jira gate action, which fall outside the working-tree changes and need an owner decision. The pipeline sets the commit message, and I may not commit. Whether applying the gate includes commenting on the M4–M8 issues is ambiguous.

Critical

High

extensions/mssql/spike/RESULT.md:38 — The "CI test coverage" section says the root npm test script does not run MSSQL extension tests and that "Nothing [is] registered in the root package.json". This is wrong. package.json:34 already includes extensions/mssql/spike/*.test.mjs. Suggested fix: reword to say the spike tests are registered in the root npm test and skip when the optional dependencies are absent. Say that only future extensions/mssql/*.test.ts tests are unregistered. Also correct the contradicting "Recommended follow-ups" and "Nothing under…" statements in ci-coverage.md, which is not part of this diff.

Medium

extensions/mssql/spike/RESULT.md:48 — The commit message is 318: MSSQL spike: ... and carries a Pi-Loop-Stage trailer. AGENTS.md requires issue-<N>: <summary> for issue-driven labels. This applies if #318 is labelled feature or enhancement. Suggested fix: reword the commit to issue-318: .... Open the PR as issue-318: <title> with Closes #318. The RESULT.md text already states this, but the commit does not follow it.
extensions/mssql/spike/RESULT.md:49 — Next step 2 says Stage 2 issues "have not been commented on" and that "the owner or a follow-up should add the STOP note". The task title says to apply the gate, which suggests the STOP note should be recorded on the blocked M4–M8 issues. Suggested fix: confirm whether applying the gate includes labelling or commenting on those issues. If it does, do it. If it does not, state in the PR body why not.

Low

extensions/mssql/spike/RESULT.md:17 — The "Pass rule applied" line and the scores reproduce only when node-sql-parser and antlr4 are installed and the grammar is generated (Java is required). The reproduction steps are only referenced via README.md. Suggested fix: add a one-line pointer to the exact install and generate commands, or note that the run date and versions are as stated.
extensions/mssql/spike/RESULT.md:44 — "[SECURITY RISK]" and "[BEST PRACTICE]" are placed on the same paragraph, with the second tag trailing the sentence. Suggested fix: tag the two recommendations separately, and note that a read-only login is a trade-off against needing DB admin involvement.

## 318: MSSQL spike: write RESULT.md, apply the gate, open the Stage 1 PR Implements [318](https://git.excelera.net/david/pi-extensions-and-skills/issues/318). > 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 09s. ## Review findings # Review Findings — 318 ## Summary I fixed the High finding and the two Low findings, all in documentation. In `RESULT.md` I corrected the CI coverage claim, added the install and generate commands, and split the two risk tags. I also corrected the contradicting follow-up in `ci-coverage.md`. I did not run any gates, because only documentation changed and the repo has no build or lint script. Two Medium findings remain. Both concern commit and PR naming and the Jira gate action, which fall outside the working-tree changes and need an owner decision. The pipeline sets the commit message, and I may not commit. Whether applying the gate includes commenting on the M4–M8 issues is ambiguous. ## Critical ## High - [x] extensions/mssql/spike/RESULT.md:38 — The "CI test coverage" section says the root `npm test` script does not run MSSQL extension tests and that "Nothing [is] registered in the root `package.json`". This is wrong. `package.json:34` already includes `extensions/mssql/spike/*.test.mjs`. Suggested fix: reword to say the spike tests are registered in the root `npm test` and skip when the optional dependencies are absent. Say that only future `extensions/mssql/*.test.ts` tests are unregistered. Also correct the contradicting "Recommended follow-ups" and "Nothing under…" statements in `ci-coverage.md`, which is not part of this diff. ## Medium - [ ] extensions/mssql/spike/RESULT.md:48 — The commit message is `318: MSSQL spike: ...` and carries a `Pi-Loop-Stage` trailer. AGENTS.md requires `issue-<N>: <summary>` for issue-driven labels. This applies if #318 is labelled feature or enhancement. Suggested fix: reword the commit to `issue-318: ...`. Open the PR as `issue-318: <title>` with `Closes #318`. The RESULT.md text already states this, but the commit does not follow it. - [ ] extensions/mssql/spike/RESULT.md:49 — Next step 2 says Stage 2 issues "have not been commented on" and that "the owner or a follow-up should add the STOP note". The task title says to apply the gate, which suggests the STOP note should be recorded on the blocked M4–M8 issues. Suggested fix: confirm whether applying the gate includes labelling or commenting on those issues. If it does, do it. If it does not, state in the PR body why not. ## Low - [x] extensions/mssql/spike/RESULT.md:17 — The "Pass rule applied" line and the scores reproduce only when `node-sql-parser` and `antlr4` are installed and the grammar is generated (Java is required). The reproduction steps are only referenced via README.md. Suggested fix: add a one-line pointer to the exact install and generate commands, or note that the run date and versions are as stated. - [x] extensions/mssql/spike/RESULT.md:44 — "[SECURITY RISK]" and "[BEST PRACTICE]" are placed on the same paragraph, with the second tag trailing the sentence. Suggested fix: tag the two recommendations separately, and note that a read-only login is a trade-off against needing DB admin involvement.

david added 2 commits

2026-11-10 23:12:39 +00:00