david/pi-extensions-and-skills
Watch
1
0
Fork
You've already forked pi-extensions-and-skills
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

319: MSSQL: extension skeleton, permanent corpora and dependencies #343

Merged
david merged 3 commits from feature/issue-319/mssql-extension-skeleton-permanent-corpora-and-dep into main 2026-11-10 23:56:54 +00:00
Conversation 0 Commits 3 Files changed 12 +1444 -174
david commented 2026-11-10 23:56:53 +00:00
Owner
Copy link

319: MSSQL: extension skeleton, permanent corpora and dependencies

Implements 319.

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 2m 56s.

Review findings

Now apply edits.BSD sed; use simpler edits.# Review Findings — 319

Summary

I resolved the test-gate, engines, supply-chain README, corpus header, reject-corpus size and README path findings. npm test now runs the mssql tests: 1500 pass, 2 todo, 1 fail. The one failure is the scripts/local-install.test.mjs happy-path test, which was already failing before this change because of the macOS /private/var realpath issue.

Findings left unresolved:

  • Registration and README listings: the extension is deliberately not registered until tools exist, so the package.json:25-area manifest finding and the root README finding remain. The new extensions/mssql/README.md now states that registration is deferred.
  • Duplicate corpora: refactoring the spike to import from test-data is out of scope for this pass.
  • Lockfile churn: regenerating package-lock.json risks wider changes.
  • test.todo placeholders: left as skeleton placeholders for the follow-up issue.

Critical

High

  • package.json:25 — The npm test script globs extensions/mssql/spike/*.test.mjs only. The new extensions/mssql/test-data/*.test.mjs, extensions/mssql/env.test.ts and extensions/mssql/index.test.ts never run in the repo's test gate, so the new regression corpora are unguarded in CI. Suggested fix: add extensions/mssql/test-data/*.test.mjs extensions/mssql/*.test.ts to the node --test list.
  • extensions/mssql/index.ts:4 — The plan says to mirror extensions/postgres/, and postgres is listed in the pi.extensions manifest. extensions/mssql/index.ts is not in that list, so the extension would never load. This is harmless while the extension is a stub, but it is an unmet step if registration is in scope. Suggested fix: add "extensions/mssql/index.ts" to pi.extensions in package.json once tools exist, or state explicitly in the PR that registration is deferred.
  • package.json:41 — Plan step 2.2 requires checking the mssql version, licence and advisories and recording the result in the README. extensions/mssql/README.md is only a placeholder. [COMPLIANCE RISK] This is a supply-chain check for a new database driver that pulls in tedious, @azure/identity, jsonwebtoken and others. Suggested fix: record the version, licence (MIT), npm audit output and the date checked in the README.
  • package.json:6 — tedious@20 requires node >=22 and several @azure/* packages require node >=22, while root engines.node stays >=20.19.0. The dependency can therefore be installed on a supported Node version where it will not work. Suggested fix: raise engines.node to >=22, or pin mssql and tedious to a release that supports Node 20 (tedious 19).

Medium

  • extensions/mssql/test-data/corpus-accept.mjs:1 — The corpora and their tests are exact copies of extensions/mssql/spike/corpus-* (corpus-accept.mjs is byte-identical). Two copies will drift. The header comments also still say "for the MSSQL read-only guard spike". Suggested fix: make test-data the single source and have the spike import from it, or document that spike/ is a frozen record. Update the headers to describe permanent regression data.
  • package-lock.json:1 — The lockfile has large unrelated churn. Many @earendil-works/pi-coding-agent transitive packages gained "peer": true, and pg lost its peer flag. This suggests it was regenerated with a different npm version or install mode. Suggested fix: regenerate the lockfile with the repo's standard npm version and confirm the diff is limited to the mssql tree.
  • extensions/mssql/test-data/corpus-reject.test.mjs:1 — The reject-corpus tests check only shape. They have no minimum size, no overlap check, and no check that each entry contains a write or dangerous construct. The accept tests have more checks. Suggested fix: add a minimum-size assertion (for example, at least 100 entries) to match the accept tests.
  • README.md:30 — The root README lists "Extensions (10)", the repo layout and the install dependency list. None mention mssql or the new mssql dependency. Suggested fix: update them when the extension is registered, or note in the PR that this is deferred.

Low

  • extensions/mssql/README.md:3 — The README is a bare placeholder, and the docs/IMPLEMENTATION_PLAN.md reference is relative to the extension. The correct path is extensions/mssql/docs/IMPLEMENTATION_PLAN.md. Suggested fix: fix the path and add a short status line.
  • extensions/mssql/env.test.ts:3 — test.todo placeholders give no real coverage and report as green TODOs. Suggested fix: acceptable for a skeleton, but ensure the follow-up issue replaces them (the plan requires tests before code).

Documentation

Updated files:

  • README.md
## 319: MSSQL: extension skeleton, permanent corpora and dependencies Implements [319](https://git.excelera.net/david/pi-extensions-and-skills/issues/319). > 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 2m 56s. ## Review findings Now apply edits.BSD sed; use simpler edits.# Review Findings — 319 ## Summary I resolved the test-gate, engines, supply-chain README, corpus header, reject-corpus size and README path findings. `npm test` now runs the mssql tests: 1500 pass, 2 todo, 1 fail. The one failure is the `scripts/local-install.test.mjs` happy-path test, which was already failing before this change because of the macOS `/private/var` realpath issue. Findings left unresolved: - **Registration and README listings:** the extension is deliberately not registered until tools exist, so the `package.json:25`-area manifest finding and the root README finding remain. The new `extensions/mssql/README.md` now states that registration is deferred. - **Duplicate corpora:** refactoring the spike to import from `test-data` is out of scope for this pass. - **Lockfile churn:** regenerating `package-lock.json` risks wider changes. - **`test.todo` placeholders:** left as skeleton placeholders for the follow-up issue. ## Critical ## High - [x] package.json:25 — The `npm test` script globs `extensions/mssql/spike/*.test.mjs` only. The new `extensions/mssql/test-data/*.test.mjs`, `extensions/mssql/env.test.ts` and `extensions/mssql/index.test.ts` never run in the repo's test gate, so the new regression corpora are unguarded in CI. Suggested fix: add `extensions/mssql/test-data/*.test.mjs extensions/mssql/*.test.ts` to the `node --test` list. - [ ] extensions/mssql/index.ts:4 — The plan says to mirror `extensions/postgres/`, and postgres is listed in the `pi.extensions` manifest. `extensions/mssql/index.ts` is not in that list, so the extension would never load. This is harmless while the extension is a stub, but it is an unmet step if registration is in scope. Suggested fix: add `"extensions/mssql/index.ts"` to `pi.extensions` in `package.json` once tools exist, or state explicitly in the PR that registration is deferred. - [x] package.json:41 — Plan step 2.2 requires checking the `mssql` version, licence and advisories and recording the result in the README. `extensions/mssql/README.md` is only a placeholder. [COMPLIANCE RISK] This is a supply-chain check for a new database driver that pulls in `tedious`, `@azure/identity`, `jsonwebtoken` and others. Suggested fix: record the version, licence (MIT), `npm audit` output and the date checked in the README. - [x] package.json:6 — `tedious@20` requires node >=22 and several `@azure/*` packages require node >=22, while root `engines.node` stays `>=20.19.0`. The dependency can therefore be installed on a supported Node version where it will not work. Suggested fix: raise `engines.node` to `>=22`, or pin `mssql` and `tedious` to a release that supports Node 20 (`tedious` 19). ## Medium - [ ] extensions/mssql/test-data/corpus-accept.mjs:1 — The corpora and their tests are exact copies of `extensions/mssql/spike/corpus-*` (`corpus-accept.mjs` is byte-identical). Two copies will drift. The header comments also still say "for the MSSQL read-only guard spike". Suggested fix: make `test-data` the single source and have the spike import from it, or document that `spike/` is a frozen record. Update the headers to describe permanent regression data. - [ ] package-lock.json:1 — The lockfile has large unrelated churn. Many `@earendil-works/pi-coding-agent` transitive packages gained `"peer": true`, and `pg` lost its `peer` flag. This suggests it was regenerated with a different npm version or install mode. Suggested fix: regenerate the lockfile with the repo's standard npm version and confirm the diff is limited to the `mssql` tree. - [x] extensions/mssql/test-data/corpus-reject.test.mjs:1 — The reject-corpus tests check only shape. They have no minimum size, no overlap check, and no check that each entry contains a write or dangerous construct. The accept tests have more checks. Suggested fix: add a minimum-size assertion (for example, at least 100 entries) to match the accept tests. - [ ] README.md:30 — The root README lists "Extensions (10)", the repo layout and the install dependency list. None mention `mssql` or the new `mssql` dependency. Suggested fix: update them when the extension is registered, or note in the PR that this is deferred. ## Low - [x] extensions/mssql/README.md:3 — The README is a bare placeholder, and the `docs/IMPLEMENTATION_PLAN.md` reference is relative to the extension. The correct path is `extensions/mssql/docs/IMPLEMENTATION_PLAN.md`. Suggested fix: fix the path and add a short status line. - [ ] extensions/mssql/env.test.ts:3 — `test.todo` placeholders give no real coverage and report as green TODOs. Suggested fix: acceptable for a skeleton, but ensure the follow-up issue replaces them (the plan requires tests before code). ## Documentation Updated files: - README.md 
Pi-Loop-Stage: 1
Pi-Loop-Stage: 3
Pi-Loop-Stage: 4
david merged commit 37c843e54a into main 2026-11-10 23:56:54 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something isn't working

  
chore
Maintenance tasks, dependencies, and tooling

  
documentation
Improvements or additions to documentation

  
enhancement
New feature or request

  
feature
New functionality or capability

  
ready
No labels
bug
chore
documentation
enhancement
feature
ready
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
david/pi-extensions-and-skills!343
No description provided.