Skip to content

fix(pre-release): 8 blockers ahead of the 1.0.0 build#6

Merged
transitrix merged 1 commit into
mainfrom
fix/pre-release-blockers
May 21, 2026
Merged

fix(pre-release): 8 blockers ahead of the 1.0.0 build#6
transitrix merged 1 commit into
mainfrom
fix/pre-release-blockers

Conversation

@transitrix
Copy link
Copy Markdown
Owner

Summary

Addresses the pre-release code review (orchestrator, 2026-05-21) — eight crash- or panel-breaking issues that Valerii's manual hand-test of every notation would have hit. Hand-test → build → release runs through main; landing this before that build means the hand-test doesn't keep tripping over the same class of bug.

Out of scope by orchestrator instruction:

  • process-blueprint validator — the reference pattern for blocker 1, already correct.
  • TX-R001 — confirmed working.

Blockers

  1. Validators crash on null / non-object array elements — all 7 notation validators (goals, fgca, capability-map, process-map, applications, products, scenarios) read element fields after Array.isArray(...) passes but with no per-element object guard. goals: [null] or goals: ['x'] threw TypeError instead of returning valid:false. Added if (!el || typeof el !== 'object') at the top of every element loop, matching the process-blueprint pattern. fgca/validate.ts also pre-computed id sets via doc.factors.map(f => f.id) before any guard — switched to a filtering collectIds helper.

  2. goals/validate.ts — non-numeric level slipped through. g.level > maxLevel did string/number compares on string levels (silently wrong) and undefined > N on missing levels (always false). Now validates level as required-and-finite and bails on the entry.

  3. goals/layout.tsplaceSubtree recurses without a visited set. layoutGoalTree is exported and reachable without validateGoalTree, so a parent-of-itself or mutual cycle caused a stack overflow. Added a placed Set; subsequent placements for the same id short-circuit, and the child list filters out already-placed ids. Tests cover self-parent and 2-node mutual cycle.

  4. fgca/layout.tsc.activity_ids unguarded. validateFGCADoc treats activity_ids as optional (?? []), but the renderer's c.activity_ids.forEach and changes.flatMap(c => c.activity_ids) threw on a change with no activity_ids. Defaulted both spots to ?? [].

  5. extension/src/goals-preview.tscheckCycles passed the wrapper instead of .root. The dead-code cycle detector never fired for duplicated goal_ids. Fixed to pass tree.root.

  6. extension/package.jsonactivationEvents listed only 3 of 11 notations. In a cold VS Code window the extension never activated for fgca, fga, activities, blocks, applications, products, process-map, scenarios, capability-map, or process-blueprint — previews, auto-preview, and editor-title buttons all failed to appear. Added workspaceContains: entries for every notation file extension.

  7. src/serve-ui.tscreateReadStream(...).pipe(res) had no error handler. A disk error mid-stream (headers already sent) crashed the process. Attached an error handler that logs and destroys the response socket — the only safe action once headers are out.

  8. src/serve-ui.tsisInsideRoot accepted a different Windows drive. path.relative('C:\a','D:\b') returns D:\b, which does not start with .., so the containment check passed. Replaced the relative-based test with a direct prefix comparison using path.sep. Exported isInsideRoot for direct unit testing.

Tests

23 new regression tests across the diagrams package and the serve-ui core module:

Test file New tests Covers
packages/diagrams/src/goals/__tests__/validate.test.ts 4 blocker 1 (goals), blocker 2
packages/diagrams/src/goals/__tests__/layout.test.ts 2 blocker 3
packages/diagrams/src/fgca/__tests__/validate.test.ts 4 blocker 1 (fgca)
packages/diagrams/src/fgca/__tests__/layout.test.ts 1 blocker 4
packages/diagrams/src/capability-map/__tests__/validate.test.ts 2 blocker 1 (capability-map)
packages/diagrams/src/process-map/__tests__/validate.test.ts 2 blocker 1 (process-map)
packages/diagrams/src/applications/__tests__/validate.test.ts 3 blocker 1 (applications + integrations)
packages/diagrams/src/products/__tests__/validate.test.ts 2 blocker 1 (products)
packages/diagrams/src/scenarios/__tests__/validate.test.ts 3 blocker 1 (factors_view, REF_SPECS)
tests/serve-ui.test.ts 5 blocker 8 (incl. Windows cross-drive guard)

Counts:

  • diagrams: 303 / 303 passing (was 280; +23 new tests).
  • core: 247 / 247 passing.
  • tsc --noEmit clean across root / packages/diagrams / extension.

Test plan

  • npx vitest run --pool=forks --poolOptions.forks.singleFork=true in packages/diagrams — 303 passing.
  • npx vitest run --pool=forks --poolOptions.forks.singleFork=true at repo root — 247 passing.
  • tsc --noEmit at root, in packages/diagrams, and in extension — clean.
  • Manual (after merge): build the extension and confirm the activation events fire for every notation in a fresh VS Code window.
  • Manual (after merge): feed each preview a malformed YAML (goals: [null], missing fields, a self-parent goal) and verify the error panel appears instead of a crash.

Note on scope

This is one connected concern — preflight robustness for the hand-test — so it ships as one PR per the "one concern, one PR" rule. The should-fix and nit items from the same review will follow as a separate PR.

🤖 Generated with Claude Code

Addresses the pre-release code review (orchestrator, 2026-05-21). Eight
crash- or panel-breaking issues that the manual hand-test of every
notation would have hit. Process-blueprint and TX-R001 are deliberately
left untouched — the former is the reference pattern for blocker 1, the
latter is confirmed working.

## Blockers

1. **Validator crashes on null / non-object array elements** — all seven
   notation validators (`goals`, `fgca`, `capability-map`, `process-map`,
   `applications`, `products`, `scenarios`) read element fields after
   `Array.isArray(...)` passes but with no per-element object guard.
   `goals: [null]` or `goals: ['x']` would throw `TypeError` instead of
   returning `valid:false`. Added `if (!el || typeof el !== 'object')`
   at the top of every element loop, matching the `process-blueprint`
   pattern. `fgca/validate.ts` also pre-computed id sets via
   `doc.factors.map(f => f.id)` before any guard — switched to a
   filtering `collectIds` helper so the precompute itself can't crash.

2. **goals/validate.ts — non-numeric `level` slips through.** The
   `g.level > maxLevel` check did a string/number compare on string
   levels (silently wrong) and an `undefined > N` compare on missing
   levels (always false, slipped through). Validate `level` as
   required-and-finite and bail out on the entry.

3. **goals/layout.ts — `placeSubtree` recurses without a visited set.**
   `layoutGoalTree` is exported and reachable without
   `validateGoalTree`, so a parent-of-itself or mutual cycle caused a
   stack overflow. Added a `placed` Set; subsequent placements for the
   same id short-circuit and the child list filters out already-placed
   ids. Tests cover self-parent and a 2-node mutual cycle.

4. **fgca/layout.ts — `c.activity_ids` unguarded.** `validateFGCADoc`
   treats `activity_ids` as optional (`?? []`), but the renderer's
   `c.activity_ids.forEach` and `changes.flatMap(c => c.activity_ids)`
   threw on a change with no activity_ids. Defaulted both spots to
   `?? []`.

5. **extension/src/goals-preview.ts — `checkCycles` passed the wrapper
   instead of `.root`.** `checkCycles(raw.goals_tree, ...)` cast the
   wrapper object to the root type, so `node.goal_id` read `undefined`
   and the cycle detector was dead code. Fixed to pass `tree.root`.

6. **extension/package.json — `activationEvents` listed only 3 of 11
   notations.** In a cold VS Code window the extension never activated
   for fgca, fga, activities, blocks, applications, products,
   process-map, scenarios, capability-map, or process-blueprint —
   previews, the auto-preview handler, and editor-title buttons all
   failed to appear. Added `workspaceContains:` entries for every
   notation file extension.

7. **src/serve-ui.ts — `createReadStream(...).pipe(res)` had no error
   handler.** A disk error mid-stream (headers already sent) crashed
   the process. Attached an `error` handler that logs and destroys the
   response socket — the only safe action once headers are out.

8. **src/serve-ui.ts — `isInsideRoot` accepted a different Windows
   drive.** `path.relative('C:\a','D:\b')` returns `D:\b`, which
   does not start with `..`, so the containment check passed. Replaced
   the relative-based test with a direct prefix comparison using
   `path.sep`. Exported `isInsideRoot` for direct unit testing.

## Tests

- 23 new regression tests across the diagrams package + serve-ui.
- diagrams: **303 / 303** (was 280) passing.
- core: **247 / 247** passing.
- `tsc --noEmit` clean across root / `packages/diagrams` / `extension`.

`process-blueprint` and TX-R001 untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

📊 Metrics Regression Report

All metrics within tolerance

Summary

  • Diagrams tested: 9
  • Successful: 9
  • Failed: 0
  • Violations: 0

Details

ai-expense-approval.bpmn.yaml

Metric Baseline Current Delta Status
crossings 1 1 0.00
bends 10 10 0.00
edgeLength 1992.6666666666665 1992.6666666666665 0.00 (0.0%)
waypointDensity 3 3 0.00 (0.0%)
spineDeviation 0 0 0.00
emptyArea 0.5869700748129676 0.5869700748129676 0.00
portViolations 0 0 0.00

feature-release.bpmn.yaml

Metric Baseline Current Delta Status
crossings 1 1 0.00
bends 13 13 0.00
edgeLength 3460 3460 0.00 (0.0%)
waypointDensity 3 3 0.00 (0.0%)
spineDeviation 0 0 0.00
emptyArea 0.5500909090909091 0.5500909090909091 0.00
portViolations 0 0 0.00

large-cyclic-workflow.bpmn.yaml

Metric Baseline Current Delta Status
crossings 1 1 0.00
bends 29 29 0.00
edgeLength 7042.666666666667 7042.666666666667 0.00 (0.0%)
waypointDensity 3.16 3.16 0.00 (0.0%)
spineDeviation 82 82 0.00
emptyArea 0.7965322633390317 0.7965322633390317 0.00
portViolations 2 2 0.00

order-fulfillment.bpmn.yaml

Metric Baseline Current Delta Status
crossings 0 0 0.00
bends 2 2 0.00
edgeLength 704 704 0.00 (0.0%)
waypointDensity 2.3333333333333335 2.3333333333333335 0.00 (0.0%)
spineDeviation 0 0 0.00
emptyArea 0.5462701149425288 0.5462701149425288 0.00
portViolations 0 0 0.00

order-processing.bpmn.yaml

Metric Baseline Current Delta Status
crossings 0 0 0.00
bends 2 2 0.00
edgeLength 704 704 0.00 (0.0%)
waypointDensity 2.3333333333333335 2.3333333333333335 0.00 (0.0%)
spineDeviation 0 0 0.00
emptyArea 0.5462701149425288 0.5462701149425288 0.00
portViolations 0 0 0.00

simple-approval.bpmn.yaml

Metric Baseline Current Delta Status
crossings 0 0 0.00
bends 8 8 0.00
edgeLength 1542 1542 0.00 (0.0%)
waypointDensity 3.142857142857143 3.142857142857143 0.00 (0.0%)
spineDeviation 41 41 0.00
emptyArea 0.7764882411250177 0.7764882411250177 0.00
portViolations 0 0 0.00

simple-linear.bpmn.yaml

Metric Baseline Current Delta Status
crossings 0 0 0.00
bends 0 0 0.00
edgeLength 440 440 0.00 (0.0%)
waypointDensity 2 2 0.00 (0.0%)
spineDeviation 0 0 0.00
emptyArea 0.5258771929824562 0.5258771929824562 0.00
portViolations 0 0 0.00

small-dense-approval.bpmn.yaml

Metric Baseline Current Delta Status
crossings 0 0 0.00
bends 14 14 0.00
edgeLength 2158 2158 0.00 (0.0%)
waypointDensity 3.272727272727273 3.272727272727273 0.00 (0.0%)
spineDeviation 41 41 0.00
emptyArea 0.8091206777597997 0.8091206777597997 0.00
portViolations 0 0 0.00

xlarge-stress-test.bpmn.yaml

Metric Baseline Current Delta Status
crossings 1 1 0.00
bends 90 90 0.00
edgeLength 11756.583333333332 11756.583333333332 0.00 (0.0%)
waypointDensity 3.6666666666666665 3.6666666666666665 0.00 (0.0%)
spineDeviation 148 148 0.00
emptyArea 0.8282701728700941 0.8282701728700941 0.00
portViolations 0 0 0.00

@transitrix transitrix merged commit 182f1fb into main May 21, 2026
1 check passed
@transitrix transitrix deleted the fix/pre-release-blockers branch May 21, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants