Add adrs - #1
Open
grahampacker-ms wants to merge 15 commits into
Open
Conversation
|
THIS SOFTWARE IS CONTRIBUTED SUBJECT TO THE TERMS OF THE FINOS Corporate Contributor License Agreement. THIS SOFTWARE IS LICENSED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE AND ANY WARRANTY OF NON-INFRINGEMENT, ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. THIS SOFTWARE MAY BE REDISTRIBUTED TO OTHERS ONLY BY EFFECTIVELY USING THIS OR ANOTHER EQUIVALENT DISCLAIMER IN ADDITION TO ANY OTHER REQUIRED LICENSE TERMS. |
grahampacker-ms
force-pushed
the
add-adrs
branch
from
January 17, 2025 18:19
28b1db7 to
4fc0f4c
Compare
grahampacker-ms
pushed a commit
that referenced
this pull request
Jul 13, 2026
…s to bare workspace (finos#2661) * chore(calm-suite): A1 — remove vestigial config and rewrite file: deps to bare workspace Implements STRUCT-01. Deletions (7 paths — review fix: calmstudio-workspace manifest kept): - calm-suite/calm-studio/vercel.json (no Vercel deploy target) - calm-suite/calm-studio/packages/calmscript/ (DSL stub — not consumed) - calm-suite/calm-studio/apps/studio/.releaserc.json - calm-suite/calm-studio/packages/calm-core/.releaserc.json - calm-suite/calm-studio/packages/extensions/.releaserc.json - calm-suite/calm-studio/packages/mcp-server/.releaserc.json - calm-suite/calm-studio/packages/calmscript/.releaserc.json (via dir delete) Rewrites (11 file: -> bare semver "*" across 6 package.json files): - packages/calm-core: @finos/calm-models - packages/mcp-server: @calmstudio/calm-core (deps + devDeps), @finos/calm-models - packages/github-action: @calmstudio/calm-core, @calmstudio/mcp - packages/vscode-extension: @calmstudio/mcp - packages/web-component: @calmstudio/calm-core, @calmstudio/extensions - apps/studio: @calmstudio/calm-core, @calmstudio/extensions Trim (1 line): - calm-suite/calm-studio/package.json (calmstudio-workspace manifest): remove `packages/calmscript` entry from workspaces[] (now-deleted directory) Review fix per @rocketstack-matt feedback on finos#2661: the calmstudio-workspace manifest (`calm-suite/calm-studio/package.json`) is KEPT, not deleted, because .github/workflows/automated-release-calm-studio.yml runs `npx multi-semantic-release` from `calm-suite/calm-studio/` and relies on this manifest's workspaces[] array to discover @calmstudio/* packages. The eventual removal of this manifest is deferred to A3 (#1-03), where it co-locates with the `git mv calm-suite/* -> root` move and the workflow's `working-directory: calm-suite/calm-studio` update. Verified locally: - `npm ci` exits 0 - `npm run package --workspace calm-plugins/vscode` exits 0 (vsce packages 8.5MB .vsix) - `cd calm-suite/calm-studio && npx multi-semantic-release --dry-run` exits 0 (all 8 @calmstudio/* packages discovered via the workspaces[] array) Lockfile regeneration follows in a separate commit per AGENTS.md ritual. Signed-off-by: Gourav Shah <gjs@opsflow.sh> * chore(deps): regenerate lockfile after A1 file: → bare-workspace rewrite Lockfile reflects: - 11 file: -> bare semver "*" workspace deps (calm-core, mcp-server, github-action, vscode-extension, web-component, apps/studio) - calm-suite/calm-studio/packages/calmscript entry removed (directory deleted in A1's first commit) - @docusaurus/faster@3.10.1 retained (added by finos#2673 on main) - minimatch@10.2.5 hoisted to root (matches main; vsce CJS require works) - @codemirror/autocomplete@6.20.3 preserved (svelte-codemirror-editor peer) - All 19 rollup platform-specific optional dependency variants preserved (validate-lockfile guard upheld) Regen method: incremental `npm install --package-lock-only` against the current origin/main lockfile, then a one-line jq script to remove the stale `calm-suite/calm-studio/packages/calmscript` entry. Avoids the known arborist crash on a clean `rm -rf node_modules package-lock.json && npm install` flow. Verified locally: - `npm ci` exits 0 - `cd calm-suite/calm-studio && npx multi-semantic-release --dry-run` exits 0 (8 @calmstudio/* packages discovered) - `npm run package --workspace calm-plugins/vscode` exits 0 (vsce packages 8.5 MB .vsix) Signed-off-by: Gourav Shah <gjs@opsflow.sh> --------- Signed-off-by: Gourav Shah <gjs@opsflow.sh> Co-authored-by: Matthew Bain <66839492+rocketstack-matt@users.noreply.github.com>
grahampacker-ms
pushed a commit
that referenced
this pull request
Jul 13, 2026
finos#2762) * feat(calm-hub-ui): namespace card-browse page with segmented type tabs Replace the Phase 1 placeholder namespace body with the real browse experience (redesign problems #1/finos#2/finos#3). - SegmentedTypeTabs: underline tabs with per-type counts and active/resting/ zero-count states (zero-count dimmed-but-visible and still selectable) - ItemCard: type-tinted striped thumbnail, name, 2-line description, TypeBadge; resolves the item's latest-version detail route on activation - EmptyState rendered in the grid area when a type has zero items - TypeBadge resource-type pill (architecture/pattern/flow/standard/adr/interface) - Active type is URL-backed via ?type=; tab counts reuse the Phase 1 counts endpoint (no new fetch); CountBadge gains a dimmed zero-count state * test(calm-hub): assert versionCount in standards list integration tests The standards summary now includes versionCount, so the standards-list response is a 4-key map. Update the Mongo/Nitrite end-to-end assertions to expect versionCount (2 versions: 1.0.0 + 2.0.0), fixing the failing end_to_end_verify_standard_details_and_if_they_are_updated checks. * fix(calm-hub-ui): harden NamespacePage item open Carries the review fix from the removed NamespaceResourceGroup forward to NamespacePage.openItem: catch a rejected resolveResourceDetailPath and log when it returns no path, instead of an unhandled rejection / silent skip. * test(calm-hub): match NamespaceArchitectureSummary versionCount arg Phase 2 adds versionCount to NamespaceArchitectureSummary; update the counts cache test (carried up from phase 1) to the 4-arg constructor so calm-hub compiles on this and later phases. * fix(calm-hub-ui): address review feedback on namespace browse page - Hub: a failed namespace-counts fetch now flags failure separately so the type tabs render resting (no badge) instead of a dimmed 0 that contradicts the independently-loaded item grid. - Derive NamespaceResourceType as Exclude<TypeInUI, 'Controls'> and export a shared COUNT_FIELD map; MobileNavMenu uses it instead of a hand switch whose silent default would drop counts for any future type. - Extract useActiveType into its own file (component-decomposition rule). - Memoize activeItems for consistency with the surrounding memoized values. * fix(calm-hub): guard version-count cast and unify namespace summaries - Guard the `versions` sub-document cast in all 8 Mongo/Nitrite stores with an instanceof check, so a non-map value counts as 0 rather than a ClassCastException 500-ing the whole namespace list response. - Merge the four byte-identical NamespaceArchitecture/Flow/Pattern/Standard summaries into a single NamespaceResourceSummary, and replace the copy-pasted null-guard+count snippet with VersionKeySelector.versionCount(...) shared across all eight stores. - Rename the misspelled namespaceStanadardSummary local (Copilot).
grahampacker-ms
pushed a commit
that referenced
this pull request
Aug 5, 2026
…inos#2884) (finos#2923) * feat(calm-hub): canonicalize version spellings in the version stores VERSION_REGEX makes both separators optional, so six different paths all denote version 1.0.0 and the API accepts every one of them: 1.0.0, 1-0-0, 1.0-0, 1-0.0, 1.00 and 100. The old shape wrote the version as a map key via replace('.', '-'), which folded the four dotted/dashed spellings together but left 100 and 1.00 as keys of their own — one logical version already stored under three keys. The header/version shape stores the version as a field value on its own document, so writing it verbatim would give one version up to six documents, each invisible to a read using any of the other five. Mongo's unique index cannot catch this (uniqueness is per stored string) and Nitrite has no index at all. SemanticVersionOrder ranks the spellings equally but ordering cannot merge them. Canonicalize at the helpers' entry points rather than in the callers, so all seven resource types inherit it and reads and writes cannot disagree about the spelling. Reuses ResourceValidationConstants.VERSION_REGEX rather than restating the pattern: the set to fold is exactly the set the API accepts, so a second copy would be a bug waiting on the two to drift. Input the regex rejects, including null, passes through unchanged — validation belongs to the resource layer, and a store that rewrote unrecognised input would turn a rejectable request into a document stored under a version nobody asked for. Surfaced by Copilot review on finos#2908. * feat(calm-hub): port the Architecture stores onto the version helpers Moves Architecture from one-document-per-namespace to the header/version shape of ADR 0001, on both backends. ArchitectureStore is unchanged, so nothing above the store layer moves. Both implementations shrink to translation: which domain exception each missing thing produces, and constructing the domain object. All document handling and, on Nitrite, all locking now live in the shared helpers. MongoResourceSlice and MongoUpsertPush drop out of the Architecture path as ADR 0003 predicted — paging is ordinary skip/limit now that headers are one document per resource, and there is no create race to retry now that the id comes from the atomic counter before the header insert. Adds updateHeaderDetails to both helpers, an operation ADR 0003 missed. Writing a version also renames the resource: the old shape set name and description in the same atomic update that wrote the content. Two consequences of splitting that update, both recorded in the ADR: - It runs only after the version write succeeds. A rejected create matched nothing under the old shape and so left the name untouched; calling it first would rename on a request that then 409s. Pinned by a test on both backends. - The atomicity is gone. A failure there reports an error for a version that was stored. It is still thrown rather than swallowed like the versionCount write, because these are user-supplied values and losing a rename silently is a wrong answer, not a stale display number. Behaviour preserved deliberately, including where it is wrong: - A null name overwrites a stored one, so a version write carrying only architectureJson wipes the display name. Logged as bugs.md finos#2 with a test pinning it, rather than fixed here — a port shouldn't smuggle in a behaviour change. - PUT still creates a version that never existed (bugs.md #1). - Nitrite still validates JSON before checking the architecture exists while Mongo reports the missing architecture first, and still echoes name/description back on the created object where Mongo doesn't. Two behaviours do change, both consequences of the shape rather than choices made here: - An architecture with no versions returns an empty list instead of reporting itself missing. The old shape had nowhere to look but the versions map, so it conflated the two; the header settles it now. Called out in ADR 0003. - Malformed JSON on create is rejected before the counter is drawn, so it no longer burns a sequence value or risks a header with no version. No migration step yet, so a deployment's existing data has not moved and this code would not find it. That lands next, in this PR. ADR 0001-0003 statuses updated to say so. * feat(calm-hub): migrate Architecture storage to the header/version shape Adds the version 2 -> 3 migration that moves existing data into the shape the ported stores read, plus the two collateral fixes the integration suite surfaced. The migration is one step per backend, both declaring fromVersion() == 2. SchemaMigrationRunner treats a duplicate version as fatal, but the two never coexist: each is gated by @LookupIfProperty on a different calm.database.mode, so exactly one is ever a bean. It is a 1 -> N fan-out, not an in-place shrink. One namespace document holding an array of architectures, each with an embedded map of every version's content, becomes N headers plus M version documents, and the original is deleted rather than edited down. The Mongo step also transitions indexes, because MongoIndexInitializationStep created a unique index on architectures.namespace alone — which enforces exactly the old one-document-per-namespace shape and makes a second architecture in a namespace impossible. That step is immutable, so the drop belongs here. The drop tolerates the index already being absent but propagates any other failure, since silently leaving the constraint in place would fail every subsequent insert with nothing to explain why. A failed step leaves the schema version unadvanced and is retried next startup, so three things make a partial run safe to repeat: only documents still carrying the architectures array are selected, headers and versions are written with upsert rather than blind insert, and the old document is deleted only once its contents are rewritten. The integration test covers all three against a real MongoDB, which is the only place the unique indexes can actually reject a bad second pass. Two things the ADRs and the implementation plan both missed, found by running the integration suite rather than by reading: - Search was silently broken. Both search stores read the architectures collection directly via searchNamespacedCollection, whose getList call returns null for a header document — so the loop skipped every document and architecture search returned zero results, with no error and nothing logged. Adds searchHeaderCollection alongside, rather than replacing, since rollout is per-type and the two shapes coexist. - EndToEndResource seeded every Mongo integration test with the old constraint, via MongoIndexInitializationStep.createIndexes(). It now performs the same index transition the migration does. MongoDocumentSizeLimitIntegration failed for a better reason: it grew one architecture's history until the namespace document exceeded 16MB and expected a 413, and under the new shape all 19 writes succeed because each version is its own document. Rather than drop the assertion, it is split. The 413 regression test moves to Pattern, which still accumulates history into one document and still needs the coverage, as do the other five types and Control permanently. A new test writes ~24MB of architecture history and asserts every write succeeds. Both halves now run against the same real MongoDB with the same payload size. * feat(calm-hub): seed architectures in the new shape at schema version 3 init-mongo.js hand-writes documents in the storage shape, so it changes with it: architectures are now seeded as one header document each plus a sibling architectureVersions collection. Changing the seed documents alone is not enough, and the reason is the substance of this change. A fresh database starts at schema version 0, so SchemaMigrationRunner would run every step on first startup, and step 0 creates a unique index on architectures.namespace permitting one document per namespace — which new-shape data violates before any migration reaches the collection, since finos.fluxnova seeds six architectures. Step 0 would throw, the runner would leave the migration lock held, and CalmHub would refuse every request until an administrator cleared it by hand. Not a degradation: the stack does not come up. Step 0 is a committed migration step and therefore immutable, so it cannot be taught to skip that index. So the script also records the schemaVersion marker at the latest version, and the runner returns before taking the lock. The cost is that skipping step 0 skips every index it creates, not just the problematic one. Without replacing them a seeded stack silently loses duplicate-prevention on concurrent POSTs across namespaces, domains, schemas, controls, userAccess and the audit trail. The script therefore creates the full inventory itself, including for the six types that have not migrated yet. The resulting indexes were diffed collection by collection against MongoIndexInitializationStep: identical, except architectures which becomes unique (namespace, architectureId), plus architectureVersions unique (namespace, architectureId, version). That duplication is a standing maintenance coupling, recorded in ADR 0001 and in calm-hub/AGENTS.md: when a type migrates, three things move together — its seed documents, its entry in the seed's index list, and LATEST_SCHEMA_VERSION. Miss the version bump and the stack runs migrations it does not need; miss the index list and duplicate-prevention disappears with nothing to show for it. The alternative considered was seeding the old shape and letting the migration convert it on first startup, which keeps the index inventory in one place but leaves the seed permanently expressing a shape the application cannot read. Verified against a real MongoDB rather than reasoned about: 11 headers and 11 versions written, no old-shape documents left, versionCount and architectureId as int32 (a double breaks the stores' getInteger), dot-separated versions, workshop/architectures/2 preserved for the decorator and ADR links that reference it, the marker written as {_id: "schemaVersion", version: 3} in the calm collection, and no migrationLock document created. Also adds architectureVersions to NitriteDBConfig's collection list. The store creates it on demand anyway, so this only keeps the list honest about what a fresh standalone database contains. * docs(calm-hub): correct ADR claims that the Architecture work invalidated Audit of all four ADRs against the code after Architecture migrated. Nine statements had gone stale or were wrong when written. ADR 0001: - The status line said no migration step existed. Both now do, so an existing deployment's data moves on first startup after upgrading. - The consequence asking for a 16MB regression test described it as absent and named MongoConcurrencyIntegration as its future home. It exists, in MongoDocumentSizeLimitIntegration, and asserts both halves. - "Open questions (not yet decided)" listed three questions, all of them struck through and resolved. ADR 0002: - Listed Architecture among versionCount()'s callers; those call sites are gone, leaving Pattern, Flow and Standard on both backends. - Prescribed replace('-', '.') for the migration's key conversion. The step calls CanonicalVersion.of instead, deliberately: it is the same conversion the write path uses, so migrated data is addressable by the spelling the store looks for, and it leaves unrecognised keys alone rather than inventing new ones. ADR 0003: - Claimed MongoUpsertPush and MongoResourceSlice both stay solely for MongoControlStore, which was never true. MongoUpsertPush has eight callers and outlives the redesign; MongoResourceSlice has exactly one, MongoPatternStore, so it is deletable when Pattern migrates. - Named SemanticVersionOrder and CanonicalVersion in the status line without describing either, while ADR 0002 pointed here for the canonicalization rationale. Both now have a section covering what they do and why the coupling to VERSION_REGEX is deliberate. - Said getArchitectureVersions conflates "no versions" with "not found" in the present tense. It did; the port fixed it. README: the table showed a bare "Accepted" for three ADRs that are built for one type out of seven, which is exactly the misreading the file's own warning is there to prevent. * fix(calm-hub): close six defects found reviewing the Architecture migration All six were confirmed against a real MongoDB or reproduced in tests before being fixed, rather than taken on the reviewer's word. Seeding over a pre-migration database hid every architecture. Every data block in init-mongo.js is guarded by countDocuments() === 0, but the index and schema-version blocks added for the new shape were unconditional. Run the script against a database that already holds old-shape architectures — which the script otherwise anticipates — and it skips seeding, creates the new indexes without dropping namespace_1, then stamps schemaVersion 3. The runner then reports nothing to migrate, the split step never runs, and the store queries {namespace, architectureId} against documents that have neither: no architectures in listings, search or counts. Reproduced on a container. The script now detects pre-migration documents and refuses to stamp the version or touch the architecture indexes, saying why. The migration collapsed versions silently and then miscounted them. The old shape wrote keys via replace('.', '-'), folding 1.0.0 and 1-0-0 together but leaving 100 and 1.00 — both accepted by VERSION_REGEX — as keys of their own. All canonicalise to 1.0.0, so only one version document can survive, but writing each key in turn let the last overwrite the earlier content while versionCount still counted every key. Both steps now collapse on the canonical key, keep the first, count the distinct set, and WARN naming the discarded key so the loss is recoverable from a backup. This is the same defect already documented in the canonicalization commit, not applied to the migration itself. The migration could exhaust the heap. It materialised every old-shape namespace document at once — precisely the documents it exists to break up, each up to 16MB and several times that parsed. An OOM is not a clean failure: the step throws, the runner leaves the migration lock held, and every instance sharing the database refuses requests until an administrator clears it. It now collects _ids and re-reads one document at a time. Creating an architecture could strand an unremovable header. The old single $push was all-or-nothing; header and version are now two writes, so a failed version write — a payload over 16MB, say — left a header that no endpoint can delete, showing in listings and search with versionCount 0 forever. Both stores now delete the header when the first version write fails. The discarded createVersion boolean is treated the same way: it means nothing was written, so returning the caller's payload as a 201 would report success for content that does not exist. Adds deleteHeader to both helpers for the compensation, tests for every path above, and reworks the Mongo migration step's test stub to model the two-pass read by id rather than returning one document for every lookup. * fix(calm-hub): collapse colliding version keys in the seed script too The code review caught this defect in both migration steps and it was fixed there; seedVersionedResource is a third instance that nobody looked at. Raised by Copilot on finos#2923. versionCount came from the raw key count while version documents were written one per raw key with a canonicalised value. Two source keys meaning the same version — canonicalVersion treats "1-0-0" and "100" as 1.0.0 — would therefore produce two documents with the same version field and a count that could never match them. Latent rather than live: the current seed data has one version per architecture. But the failure is worse than it first looks, because the inserts happen ~4700 lines before the unique index is created. The duplicates land successfully, createIndex then fails with DuplicateKey, and the script aborts before recording the schema version. On the next startup the migration runs, its own createIndex hits the same duplicates, the step throws, and the runner leaves the migration lock held — CalmHub refuses every request until an administrator clears it by hand. Collapses by canonical version first, keeps the first key, logs the dropped one, and derives both versionCount and the inserted documents from the collapsed map — the same treatment as collapseToCanonicalVersions in the two migration steps. Verified against a real MongoDB with a deliberately colliding key injected: one warning, one version document, versionCount 1, and the script runs to completion and records the schema version. The unmodified seed is unchanged at 11 architectures and 11 versions with no warnings. * fix(calm-hub): pin the schema version before seeding, and only on an empty database Two failure modes in the version pin, both raised on finos#2923 and both reproduced against a real MongoDB before fixing. A seed that died partway bricked the hub. Architecture headers were written ~570 lines before the version marker, so an interrupted run — container killed, disk full — left new-shape headers with no marker. Startup then ran step 0, whose unique index on architectures.namespace cannot be built over several architectures in one namespace, the step threw, and the runner left the migration lock held: every instance returning 503 until someone cleared migrationLock by hand. Nothing re-runs the script either, since docker-entrypoint-initdb.d only fires on an empty data directory. The marker and the indexes are now written before any data, so an interrupted seed leaves a startable hub with incomplete data rather than an unstartable one. The guard was also too narrow. It asked only whether pre-migration architecture documents existed, but the pin suppresses every step, not just the architecture split. A database with existing namespaces and no architectures passed it, so pinning silently skipped NamespaceAccessBackfillStep and those namespaces never received their `* read` grant — going dark once entitlements are enforced. The pin now requires a genuinely empty database, which is the only case it was meant for; anything else keeps version 0 and lets the real migrations run. Architecture seeding is gated on the same condition. It is the only type seeded in the header/version shape, which depends on the index swap above; writing it into a database that will instead be migrated on startup would leave documents step 0's index build chokes on. The other types are seeded in the shape step 0 already expects, so they are unaffected. Verified across four states: an empty database pins and seeds as before; a database with namespaces but no architectures no longer pins, so the entitlement backfill still runs; a database with old-shape architectures neither pins nor seeds, leaving the migration to convert it; and a simulated partial seed now leaves step 0 skipped rather than failing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.