Deep review plan for fork vs upstream

[Sam-Bolling]

Plan:

1. Establish baseline vs upstream (add upstream remote, fetch, diff fork-only commits/files).
2. Dependency/build integrity review (manifests, lockfiles, CI scripts, version drifts, supply-chain risk).
3. API & contract compatibility review (public surfaces vs upstream, breaking-change checks).
4. Code correctness sweep (edge cases, error handling, concurrency, null/undefined safety, validation).
5. Security review (secrets/config, auth/crypto patterns, injection/SSRF/XXE, dependency CVEs).
6. Testing coverage analysis (map tests to changes, identify gaps, propose high-value cases).
7. Performance/efficiency pass (hot paths, network/IO, data structures; quick wins + profiling targets).
8. Docs/developer UX check (README/setup/examples; migration/upgrade notes).

Outcome: actionable findings and recommended fixes per area.

[Sam-Bolling]

Comprehensive Review Report (Steps 1–8)

Overview

• Completed all child issues (Steps 1–8). Below is a consolidated summary of results and recommendations.

Step 1 — Baseline vs Upstream
Results:

• Upstream: camptocamp/ogc-client, upstream/main @ 53a6449 (2025‑12‑17).
• Merge base equals upstream HEAD; fork is 69 commits ahead.
• Diff stats: 100 files changed; ~45,188 insertions / 168 deletions.
• Hotspots: CSAPI core, SensorML, SWE Common, validators, parsers.
• Risk flags: CI workflows removed; large spec files added; swe-validator.spec.ts empty.
  Recommendations:
• Keep a stable merge-base record in future reviews.
• Restore CI workflows and pin action SHAs once ready.

Step 2 — Dependency & Build Integrity
Results:

• Direct deps: root runtime only @rgrove/parse-xml; large dev toolchain; app uses Vue + Vite.
• npm audit: 15 vulns (1 critical, 3 high, 10 moderate, 1 low).
• CI workflows currently absent; build/test not enforced.
• Tooling drift: root uses Vite 5.x; app uses Vite 4.x.
  Recommendations:
• Prioritize fixes for critical/high (form-data, ws, rollup, validator).
• Re-enable CI to enforce lint/test/build.
• Align Vite versions across root/app or document split.

Step 3 — API & Contract Compatibility
Results:

• Public exports expanded with CSAPI navigator/parsers/validators/models (additive).
• No removals found in entry exports; low breaking-change risk for upstream consumers.
  Recommendations:
• Document new CSAPI APIs explicitly in README and docs.
• Consider minor version bump to reflect additive surface.

Step 4 — Code Correctness Sweep
Key risks found:

1. Query options dropped in navigator helper methods

• _applySystemsQuery missing id/geom/foi/recursive, etc.
• _applySamplingFeaturesQuery missing id/geom/foi/observedProperty/controlledProperty.
• _applyDatastreamsQuery missing id/q/system/foi/resultTime.
• _applyProceduresQuery missing id/datetime.

2. SensorML position → geometry extraction likely wrong for DataRecord/Vector

• extractGeometry reads field.value/coordinate.value vs schema component.

3. SensorML collection parsing not implemented (SystemCollectionParser.parseSensorML throws).
4. RequestBuilderOptions.format unused; SensorML builders ignore validate/strict.
5. GeoJSON validator only checks presence of uid, not type.
   Recommendations:

• Align query helpers with interfaces; avoid silently ignoring parameters.
• Fix extractGeometry for SWE Common data structures.
• Either implement SensorML collection parsing or document limitation.
• Use or remove RequestBuilderOptions.format to avoid misleading API.
• Tighten CSAPI UID validation type checks.

Step 5 — Security Review
Results:

• No hardcoded secrets detected; no tracked .env files.
• npm audit flagged multiple vulnerabilities (see Step 2).
• XML parsing via @rgrove/parse-xml; no external entity use observed.
• CI workflows missing (no action pinning or secret checks).
  Recommendations:
• Address audit findings; re-enable CI with pinned actions.
• Consider adding a lightweight secret scan in CI.

Step 6 — Testing Coverage Analysis
Results:

• 49 test suites identified.
• Coverage run blocked by failing tests:
  • endpoint.spec.ts (strict equality on links; error type mismatch)
  • parsers/resources.spec.ts (SWE “not applicable” error text mismatch)
  • constraint-validator.spec.ts (numeric timestamp handling)
  • shared/http-utils.spec.ts (esbuild worker path; timeout)
  • swe-validator.spec.ts empty suite
    Recommendations:
• Fix the 5 blockers, rerun coverage, then record module % and gaps.
• Add SWE validator tests to cover untested module.

Step 7 — Performance & Efficiency
Results:

• Likely hotspots: CSAPI parse/validate pipeline; GeoJSON validation; SWE text codec.
• Large spec files should remain doc/test only (not runtime).
  Recommendations:
• Consider short-circuiting validation in non‑strict mode.
• Provide caller‑supplied format to skip detection in batch parsing.
• Add benchmark targets for parsing/validation throughput.

Step 8 — Docs & Developer UX
Results:

• README references upstream demo and coverage badge, but CI/coverage are disabled.
• CSAPI support not mentioned in README standards list.
• Node >=20 requirement missing in README.
• app/README.md still references Vue CLI despite Vite usage.
• examples/README.md only covers STAC; no CSAPI examples.
  Recommendations:
• Update README to reflect CSAPI support and fork docs URL.
• Add Node version requirement and current scripts (build/test/coverage).
• Update app README to Vite tooling references.
• Add CSAPI examples or links.

Consolidated Recommendations (Top Priority)

1. Restore CI workflows and enforce lint/test/build.
2. Fix correctness issues in navigator helper query handling.
3. Add SWE validator tests and resolve current failing suites to unlock coverage metrics.
4. Address critical/high npm audit findings.
5. Update README/docs to reflect CSAPI additions and current toolchain.

If you want, I can open follow‑up issues for each top priority item and start on fixes.