perf: scale Full Synchronisation, fix OpenLDAP membership batching#1094
Merged
Conversation
…r chunk The Scale500k25kGroups snapshot build stalled for hours partway through Step 4 (group membership assignment), stuck at ~16000/24997 groups with slapd pinned at 95% CPU. Root cause: the populator emitted one LDAP modify per member. back-mdb stores each group as a single entry, so every "add: member" rewrites the whole entry; building the 315k-member AllStaff group one member at a time is O(N^2). Measured against the real base image: adding 20000 members took 60.3s per-member vs 1.83s as multi-valued modifies (identical resulting membership). The gap widens with N, so the 315k-member group extrapolates to ~4 hours and never completed within the run. Emit a single multi-valued modify per 5000-member chunk instead: all values are applied in one entry rewrite (~O(N)), each request stays under OpenLDAP's authenticated socket-buffer cap, and the directory state is byte-identical. This also cuts the number of accesslog entries the build generates by orders of magnitude (previously enough to fill the 128GB accesslog and log MDB_MAP_FULL on every write). Editing the populator changes the snapshot content hash, so cached Scenario 8 snapshots rebuild automatically on the next run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Snapshot images were bloated to ~138GB (e.g. jim-openldap:s8-scale100k5kgroups) because docker commit baked in the live accesslog. /bitnami/openldap is not a Docker volume (the base image declares none), so the whole live tree is committed; the previous code dropped the accesslog only from the .provisioned restore copy, never the live one, leaving the full accesslog in the image layer. The Scale500k25kGroups build fills the entire 128GB accesslog cap. Remove the live accesslog MDB before the .provisioned backup and commit. slapd is idle post-populate and holds the file open, so the unlink is safe and docker commit (which walks the filesystem tree) excludes the now-unlinked file. start-openldap.sh already recreates a fresh accesslog on every boot, so nothing is lost. Doing it before the backup also removes the now-redundant .provisioned accesslog cleanup and avoids copying the accesslog at all. Validated against the real base image: an accesslog-inflated build committed at 276MB unchanged vs 210MB with the fix (the accesslog is excluded); the fixed image boots, restores its data, and its fresh accesslog correctly logs new writes (verified via cn=accesslog reqType entries), so delta-import change capture is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…c ordinals PR #1060 configured the API's JsonStringEnumConverter with allowIntegerValues:false, so every enum-typed request DTO property (and query-string enum) must be a string name; numeric ordinals now return 400. Six cmdlets still mapped their ValidateSet string parameters to integer enum ordinals before sending them, so they broke against the API: - New-JIMSyncRule (direction) - the failure that surfaced this, in Setup-Scenario8 - New-JIMRunProfile (runType) - New-JIMSchedule (triggerType, patternType, intervalUnit) - Set-JIMSchedule (triggerType, patternType, intervalUnit) - Switch-JIMMatchingMode (mode) - Get-JIMScheduleExecution (status, in the query string) Each parameter's ValidateSet already equals the exact enum member names, so the int-mapping switches were redundant as well as wrong; send the validated string directly. #1060's note claimed the PowerShell module already sent strings; it did not, because the module shipped no request-body assertion for enum fields and no integration run had exercised these cmdlets since #1060 merged. Adds EnumSerialisation.Tests.ps1 pinning all six cmdlets to the string-name contract (red before the fix, green after). Full JIM.PowerShell Pester suite: 1064 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the enum-as-string fix. Four more cmdlets sent numeric enum ordinals via a hashtable map instead of a switch, so the first sweep (which only matched the switch form) missed them. They broke against the API's allowIntegerValues:false converter (PR #1060), surfacing at Scenario 8 Step 13 (Set-JIMMetaverseObjectType -DeletionRule): - New-JIMMetaverseObjectType / Set-JIMMetaverseObjectType (deletionRule) - New-JIMMetaverseAttribute (type, attributePlurality) - Set-JIMMetaverseAttribute (type + attributePlurality on the schema PATCH, renderingHint on the metadata PATCH) Each parameter's ValidateSet already equals the exact enum member names, so send the validated string directly. The one exception is -Type, whose ValidateSet exposes 'Integer' as a friendly alias for the AttributeDataType member 'Number'; that alias is normalised to 'Number' before sending (sending 'Integer' would 400 as an undefined enum value). Set-JIMMetaverseAttribute back-fills an unsupplied schema value from the current attribute, which the API already returns as a name, so those flow through unchanged. A repository-wide sweep for all int-producing forms (switch{int}, @{name=int} maps inline and multi-line, $map[...] into bodies, bare int literals, [int] casts) confirms these are the last of them; the remaining [int] casts are all entity IDs. Extends EnumSerialisation.Tests.ps1 to all six metaverse cases (incl. the Integer->Number alias) and updates the one existing Metaverse test that asserted the old integer schema body. Full JIM.PowerShell Pester suite: 1070 passing. Live-API verified: deletionRule=1 -> 400 (the Step 13 error), "WhenLast..." -> 200. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Scale500k25kGroups exposed a quadratic slowdown exporting huge groups: slapd duplicate-checks each added member against every existing value with a linear scan and rewrites the whole entry per modify, so at batch size 100 a 500K-member group needs 5,000 round trips whose cost grows with the entry (measured 0.02s -> 1.3s per modify at 350K members). - Raise the LDAP Connector's default Modify Batch Size from 100 to 1000 (10x fewer round trips; existing Connected Systems keep their stored value, new ones pick up the new default) - Enable olcSortVals for 'member' in the OpenLDAP test image so the duplicate check is a binary search; applied at image build only, as sortvals must not be reconciled onto snapshot data stored unsorted (the init script is part of the snapshot content hash, so existing snapshots rebuild automatically) - Scale the OpenLDAP container memory limit per template (2G default, up to 12G at Scale1m) so the MDB page-cache working set fits at scale - Document sortvals tuning in the public LDAP Connector docs with a link to the OpenLDAP tuning guide, and record the accesslog logold write-amplification constraint in test/CLAUDE.md (JIM's delta import needs reqOld to type deletes, so logold cannot be dropped yet) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… runs The Scale500k25kGroups run failed at the confirming import: FixupCrossBatchChangeRecordReferenceIdsAsync inherited 6.5M unresolved change record reference values (written by the sync and export stages with ReferenceValueId nulled for cross-batch FK safety) and tried to resolve them in a single UPDATE, which blew the 300s bulk command timeout. Measured: the matching join scan takes ~17s; the unbounded write side is what dies. - Rewrite the fixup as two phases: one scan materialises every resolution into a session-local temp table, then bounded UPDATE batches (default 250K rows) apply them, each statement well inside the timeout. Partial progress is durable and the operation is idempotent, so an interrupted run resumes with less to do. - Resolve at export completion too, where most of the backlog is created, so the next import no longer inherits a multi-million-row debt. The import-end call remains as the safety net. - Cover the rewritten SQL with real-PostgreSQL regression tests (multi-batch resolution, case-insensitive DN matching, Connected System scoping, non-reference exclusion, unresolvable-row termination, idempotency), opt-in via JIM_TEST_RESET_*. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ment timeout The Scale500k25kGroups re-run (2026-07-19) got past the change record fixup (previous commit, proven at 6.16M rows in production) and failed one step later: GetPendingExportsLightweightByConnectedSystemIdAsync loaded 525K Pending Exports joined to 9.8M attribute value changes in a single collection-Include EF statement, which the server-side statement_timeout (5 min) killed during the confirming import's reconciliation. - Load in keyset-paginated chunks (default 50K Pending Exports per page; measured 0.66s per chunk at full scale): a page of headers, then value changes by PendingExportId, with Attribute navigations stitched from a shared lookup instead of a per-row Include. - AsNoTracking throughout: the background pre-load context was already disposed immediately after the query (entities detached), and the duplicate self-heal deletes via raw SQL, so tracking bought nothing. - Real-PostgreSQL regression tests: completeness across chunk boundaries, Attribute stitching, system and null-CSO scoping, and duplicate self-heal across chunks (unique index dropped to simulate the legacy data the self-heal exists to repair). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Activity "Operations" tab (ActivityDetail) pegged JIM.Web at 100% CPU and took many seconds to load for activities with tens of thousands of RPEIs, even though it is server-paginated to 100 rows. GetActivityRunProfileExecutionItemHeadersAsync Include()d each RPEI's ConnectedSystemObject plus its entire (multi-valued) AttributeValues collection, materialised full tracked entities for the page, then projected in memory, so a page landing on a few large group CSOs pulled tens of thousands of member attribute-value rows into memory to render 100 grid rows. Rewrite as an AsNoTracking SQL projection: resolve the live display name / external id / type via correlated scalar subqueries (run only for the page's rows), falling back to the RPEI snapshot columns. External-id value columns are projected individually; a single multi-column subquery projection makes EF Core emit a ROW_NUMBER() window over the WHOLE AttributeValues table (millions of rows) instead of a correlated subquery. Also hoist the outcome-type filter's enum ToString() out of the predicate so it translates to SQL instead of throwing. Measured against the real Scale500k25kGroups dev database on a 26,824-RPEI activity: default page 1.3s cold (was many seconds / 100% CPU and effectively unusable); deep paging, display-name sort and outcome-type filtering all execute. Behaviour preserved (live values with snapshot fallback); existing tests green. No index needed: the ORDER BY Id top-N sort is ~8ms; the cost was purely the per-row graph materialisation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ort link Reference recall stages a membership-removal Pending Export for a referencing group on every synchronisation page that deletes one of its members, and the delete-then-create persistence coalesces those into a single Pending Export. Emitting one RPEI per page-flush reported that single export as many RPEIs (21,824 for 5,421 real Pending Exports on Scale500k25kGroups, and >100x for hub groups referenced by leavers on hundreds of pages), inflating TotalPendingExports and bloating the operations table with rows pointing at superseded Pending Exports. - Defer recall RPEI emission to end of run, keyed by referencing CSO (last write wins: the final flush's Pending Export already carries every prior page's removals), so exactly one RPEI is emitted per group, carrying the coalesced Pending Export and the full removal count. - Persist PendingExportId in both raw RPEI insert paths (single-connection INSERT and parallel COPY); it was silently dropped, so every recall RPEI landed with a NULL link and the operations-tab drill-down could not load the Pending Export. - Populate ExternalIdSnapshot/ObjectTypeSnapshot on recall RPEIs via one end-of-run Summary-tier lookup (correlated scalar subqueries, no member-value materialisation), so each stays self-describing if its group CSO is later deleted. Adds dedup unit tests and RequiresPostgres regression tests for the raw-insert column mapping and the snapshot projection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The largest end-to-end validation is Scenario8 CrossDomainEntitlementSync at Scale500k25kGroups: 500,000 users and 24,997 groups synchronised cross-domain across two directories, with the largest group at 495,000 members. Update the README, docs landing page, and roadmap from "100K+ object scale", and correct the LDAP connector tuning note that incorrectly claimed testing "up to 1 million users" (JIM has not been run at that size). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split the open-ended "100,000+ objects" tier into 100k-250k and 250k-500k rows, and add a callout with the measured peak from the largest validated scenario (~55 GB total host RAM for the Scenario8 cross-domain run: two ~500k-object directories with groups up to 495,000 members). Notes that large group memberships, not raw object count, are the dominant memory driver, so the same object count with only small groups needs considerably less. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The integration runner composes docker-compose.override.yml, whose dev
database profile (256MB shared_buffers, 1GB max_wal_size) previously
applied at every template size. At Scale500k25kGroups this triggered
WAL-pressure checkpoints every ~12 seconds ("checkpoints are occurring
too frequently" logged 1,488 times over the run), each stalling bulk
COPY streams for up to 78s.
The override's database command and shm_size are now parameterised via
JIM_DB_* environment variables (unchanged dev defaults when unset), and
Run-IntegrationTests.ps1 assigns a profile per Scale template before
compose up, mirroring the OPENLDAP_PRIMARY_MEMORY scaling. Scale
profiles also space checkpoints (15min) and enable lz4 WAL compression,
which shrinks the full-page writes that dominate bulk-load WAL. The
override now sets effective_io_concurrency=1000, matching the NVMe
assumption in docker-compose.yml (it previously fell back to
PostgreSQL's spinning-disk default of 16).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ValuesEqual compared expected vs actual multi-valued sets by scanning the whole actual set linearly for every expected value: O(n^2) in membership size. At Scale500k25kGroups this cost ~36s of CPU per large group and 2,073s (66%) of the 52-minute confirming Full Synchronisation, measured via per-CSO diagnostic spans. A HashSet.SetEquals fast path now settles the overwhelmingly common same-typed no-drift case in O(n); the pairwise scan is retained as a fallback for the cross-type semantics hash equality cannot see (Guid vs its string representation, byte array content), itself filtered by an O(1) hash lookup so only genuinely hash-missing values pay for a scan. The regression test compares two identical 30,000-member reference sets under a generous 2s ceiling (9.9s before the fix, milliseconds after); a second test locks in the Guid-to-Text fallback semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e list check Two costs in the confirming Full Import's processing phase (2,537s vs 366s for the equivalent create-path import at Scale500k25kGroups): - GetReferenceExternalIdsAsync ran once per existing imported object: 535,425 individual round trips measured via pg_stat_statements. A new batched GetReferenceExternalIdsForCsosAsync fetches a whole hydration page's lookups in one = ANY query; every requested ID is present in the result (empty when the CSO has no resolved references), so only CSOs matched outside the hydrated page fall back to the single-CSO query. Proven against real PostgreSQL for per-owner grouping, secondary-over-primary preference and single-CSO parity. - The update-list duplicate guard scanned the growing list with List.Any per object: ~1.4x10^11 Guid comparisons across 525K objects. A companion HashSet now gives O(1) membership, maintained at every site that appends to the update list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GetConnectedSystemObjectsAsync paged with OFFSET, which re-scans and discards all rows before the requested page, so per-page cost grows linearly with page number: 977s across 1,050 page loads (~200ms early pages degrading to ~1.5s late ones) during the Scale500k25kGroups confirming Full Synchronisation. The full sync loop now chains a keyset cursor (afterId, introduced on the repository in the previous commit) starting at Guid.Empty, keeping every page O(pageSize). The cursor is always the last row of the previous page exactly as the database returned it: PostgreSQL orders uuids bytewise, which differs from .NET's Guid.CompareTo, so each engine's ordering must be paired with its own comparison; a client-side max would skip rows. Offset behaviour is unchanged for random-access callers (UI paging). Real-PostgreSQL tests prove completeness and non-overlap of the cursor chain in both the watermark and standard loading branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The worker main loop only delayed in its idle branch; while any task was processing it spun heartbeat UPDATE + cancellation SELECT pairs as fast as the two round trips allowed. Measured at Scale500k25kGroups: 6.7M heartbeat updates, 6.7M cancellation checks and 16.4M connection-pool resets over one run (~380 queries/second of constant background load against the same database the sync was writing to), plus ~294M referential-integrity trigger subqueries server-side. The busy branch now paces at 2s, matching the idle cadence. Stale-task recovery tolerates far coarser heartbeats, and up to 2s of extra cancellation latency is acceptable. Validated at runtime: 9 heartbeat updates over an ~18s task window (0.5/s) on the rebuilt worker. Also records the changelog entries for the synchronisation performance fixes in this series. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes the last outstanding item on #952: pre-upgrade backups must capture the encryption key set alongside the database, or a rollback to that backup leaves every stored secret undecryptable. The Upgrading page was a stub, so a bare backup note would not have been read. Replaces it with a full how-to: pre-upgrade checklist, connected and air-gapped procedures, what happens during the upgrade window (the worker applies migrations; web and scheduler gate on readiness), verification, PowerShell module, and rollback. Also corrects Verify Startup in the deployment guide, which told readers to look for "Database migrations applied" / "Database is up to date" in the worker logs. Neither string is emitted anywhere in the codebase; migration timing is logged at Verbose only. Points at the readiness endpoint and the web service's actual Information-level wait message instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the Schedule quiesce/restore steps to the upgrade procedure. A Schedule firing mid-upgrade starts a Run Profile execution that the service stop then interrupts, so Schedules come off first and the Activity drain follows. Both halves record the enabled set rather than blanket-toggling: Schedules deliberately disabled before the upgrade must stay disabled, which a naive "enable everything afterwards" would silently undo. Leaving Schedules off is itself a silent failure mode (JIM looks healthy, nothing synchronises), so re-enabling is a checked step. Also fixes a PowerShell doc example selecting a non-existent property: the schedule DTO exposes IsEnabled, not Enabled, so `Select-Object Name, Enabled` renders an empty column. Found while writing the Where-Object filter for the disable snippet, where the same mistake would have made the command a silent no-op. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sweeps the customer-facing upgrade and deployment prose from "database migrations" to "database upgrades". Entity Framework Core is an implementation detail; an administrator planning a maintenance window does not need to know which ORM applies the schema change, only that it happens automatically and gates readiness. The rollback section keeps the term, because it invokes `dotnet ef migrations list` directly and the argument is a migration name. configuration.md's "provider migration" is an unrelated sense and is left alone. Also refreshes the Upgrading card on the administration index, which still advertised the stub's "migration notes". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rollback procedure told customers to run `dotnet ef migrations
list` and `dotnet ef database update` inside the jim.web container.
That is developer tooling in a customer-facing runbook, and it could
never have worked: the final jim.web image is built on the ASP.NET
runtime base, which ships neither the .NET SDK nor the dotnet-ef tool,
so both commands fail on a production deployment.
Replaces it with the procedure customers actually have available:
stop, restore the pre-upgrade backup (database and encryption keys as
a matched pair), pin JIM_VERSION back, start, verify, re-enable
Schedules. Adds a warning against running an older JIM against an
upgraded schema, and says plainly not to roll back at all without a
usable backup.
Also drops the EF Core reference from the factory-reset cmdlet notes,
adds an explicit {#restoring} anchor to the emoji-headed Restoring
section so the new cross-link resolves, and replaces a "contact
support" instruction that named a channel the docs never document.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`docker compose exec jim.web dotnet ef ...` appeared in four live docs as a rollback path, an air-gapped troubleshooting fallback, and a "first run" step. It cannot work in any of them: every service image is built on the ASP.NET runtime base, which carries neither the .NET SDK nor the dotnet-ef tool, and the dev compose override does not swap in an SDK stage either. It is also unnecessary, since the worker applies pending migrations at startup and gates the other services on readiness. RELEASE_PROCESS.md now defers to the customer-facing rollback and restore procedures rather than duplicating them, and states the release-time implication: restoring a backup is the only production rollback, so migrations should stay additive enough that a fix-forward release is always available. Backup guidance in the same file said "back up your database"; it now says database and encryption keys as a matched pair, matching #952. Also removes the same phantom worker log strings from RELEASE_PROCESS.md that were corrected in deployment.md earlier on this branch, and adds an explicit {#rolling-back} anchor so the new cross-references resolve past the emoji heading. plans/done/RELEASE_PROCESS_HARDENING.md keeps its copies; it is a point-in-time record and exempt from retro-editing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Audited every example and Output table in docs/powershell/ against the
DTOs the cmdlets actually return. Eight of twenty files referenced
properties that do not exist. Because PowerShell resolves an unknown
property to $null rather than erroring, these fail silently.
Most severe, certificates.md's "Remove all disabled certificates":
Get-JIMCertificate | Where-Object { -not $_.Enabled } | Remove-JIMCertificate -Force
TrustedCertificateHeader exposes IsEnabled, so $_.Enabled was $null for
every row, -not $null was true for every row, and -Force suppressed the
confirmation prompt. Copy-pasting that example deleted the entire
trusted certificate store without prompting.
The rest render empty columns or match nothing: activities.md filtered
execution items on a Status property they do not have (and on an
ActivityStatus value, "Failed", that is not in the enum; it is
FailedWithError); run-profiles.md filtered on Type rather than RunType,
so "start all full imports" started none; connected-systems.md filtered
connector definitions on a Capabilities collection that exists in no
shape; index.md demonstrated the Attributes dictionary on the by-Id
result, which returns AttributeValues instead.
Left alone deliberately: synchronisation-rules.md's $_.Enabled is
correct, as SyncRuleHeader really does name it Enabled. A blanket
Enabled -> IsEnabled sweep would have broken it.
Also fixes the two cmdlets whose comment-based help carried the same
bad example, so Get-Help stops teaching it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…1078) The Activity detail page re-aggregated every Run Profile Execution Item and Sync Outcome on each 3-second progress poll (four GROUP BYs over ~525k RPEIs and millions of outcome rows at Scale500k), costing ~85 minutes of cumulative database time per large run, and again on every visit to a completed Activity. Stats are now maintained as incremental (ActivityId, Dimension, Key) counter rows, upserted by the bulk RPEI/outcome persistence paths with a deadlock-safe multi-row ON CONFLICT accumulate, and finalised to exact values (recomputed from the persisted rows) when the Activity reaches a terminal status. GetActivityRunProfileExecutionStatsAsync reads counters for in-progress and finalised Activities; completed Activities predating the counter table aggregate once and are finalised lazily on first read. In-flight counters are advisory (the only known drift source is post-insert ErrorType changes during confirming-import reconciliation); finalisation reconciles exactly, and a finalisation failure never blocks Activity completion. This is the derivation-side enabler for #202/#307 (real-time progress push): the counter row is exactly the payload a progress API or SignalR publisher needs, and the batch-flush upsert site is where a future NOTIFY goes. TDD: calculator unit tests, ActivityServer finalisation-ordering tests (red first via new APIs), and a RequiresPostgres fixture proving the single-connection and parallel COPY insert paths write counters, the in-progress stats read serves from counters (skew test), finalisation reconciles deliberately skewed counters, and legacy Activities lazily finalise. The fixture also points the JIM_DB_* variables at the scratch database so the parallel write path cannot touch a live one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The usage stamp ran a tracked read-then-save on the same ApiKeys row for every authenticated API request. During the Scale500k25kGroups validation run (2026-07-21), the confirming import's write bursts briefly stalled the database; queued stamp writes convoyed on the row lock past the 30s command timeout, and EF's SaveChangesFailed diagnostic logged the tolerated best-effort failures at error level, tripping the integration error watcher and aborting the run. RecordUsageAsync is now a single ExecuteUpdateAsync with the throttle threshold in the predicate (at most one stamp per key per 30 seconds): no SELECT round trip, no change tracker, no SaveChangesFailed event on failure, and concurrent stamps collapse to no-ops instead of queueing. In-memory provider falls back to a tracked update with identical semantics, mirroring IncrementAggregatedFailedAuthenticationAsync. TDD: in-memory behavioural tests (throttle red against the old always-overwrite implementation) plus a RequiresPostgres fixture proving the relational set-based path stamps, throttles, and re-stamps after the interval. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three defects where the module and its docs disagreed. In each case the
documented behaviour was the desirable one, so the cmdlet was fixed
rather than the doc downgraded, except where the behaviour could not
honestly exist.
Get-JIMScheduleExecution silently ignored a piped Schedule. Its
-ScheduleId took pipeline input by property name, but a Schedule
exposes Id, so nothing bound, the filter was skipped, and the caller
received every execution in the system whilst believing they had
filtered on one Schedule. An [Alias('Id')] cannot fix this: the cmdlet
already has its own -Id parameter, and PowerShell rejects a parameter
alias colliding with another parameter's name outright, even across
mutually exclusive sets. Adds -InputObject instead, preferring an
explicit -ScheduleId when one is supplied, so objects carrying a
scheduleId property keep their existing binding.
Reset-JIMServiceSetting was documented as accepting pipeline input but
its Key parameter carried no pipeline attribute, so the documented
piping example prompted for a mandatory parameter instead of running.
Invoke-JIMExampleDataTemplate's -Wait was documented as blocking until
generation completed; it only emitted a warning saying it was not
implemented. It cannot be implemented as things stand, because the
execute endpoint returns a bare 202 with no body, leaving the client no
handle to poll. The switch is removed and the asynchronous behaviour
documented, pointing at Activities for monitoring.
Also reconciles docs/powershell/run-profiles.md, which was written
against an earlier cmdlet design: wrong parameter names and invalid
ValidateSet values, parameter sets that do not exist, Guid fields typed
as int, and undocumented parameters. Two examples used -Id against
Get-JIMRunProfile, where Id aliases ConnectedSystemId; one of them
piped the result into Remove-JIMRunProfile -Force, which would have
deleted every Run Profile on that Connected System rather than one.
Pester: 1093 passing, 0 failing (baseline 1070).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
"Pipeline deletion" understated what the example does. Get-JIMConnectedSystem -Name supports wildcards, so "Decommissioned*" piped into Remove-JIMConnectedSystem -Force deletes every matching Connected System and its connector space, with no prompt. Same shape as the Run Profile example corrected in c8b1442: a reader's mental model of "delete the thing" against a behaviour of "delete everything matching". Found by sweeping every destructive pipeline example in the PowerShell docs after that fix; the others are either scoped to an exact identifier or already say "multiple" in the title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds scripts/Lint-DocExamples.ps1, run alongside the changelog and
docs-coupling lints.
A PowerShell alias serves both binding paths, so [Alias('Id')] on
Get-JIMRunProfile's -ConnectedSystemId parameter, added so a piped
Connected System binds, also makes "-Id 42" a valid command-line
spelling. A doc example reading "Get-JIMRunProfile -Id 42" therefore
looks like "Run Profile 42" and means "every Run Profile on Connected
System 42"; one shipped example piped exactly that into
Remove-JIMRunProfile -Force. Removing the alias is not an option, as it
would break the documented piping, so the lint requires documentation
to spell parameters out instead.
Builds the cmdlet/alias/real-name map from the module's AST, then parses
the extracted example code with the same parser rather than matching
text, so continuations and pipelines resolve and -Id cannot collide with
-Identity. Only ```powershell fences and Get-Help .EXAMPLE blocks are
scanned; parameter tables and prose legitimately document these aliases
and are structurally out of reach.
Currently reports zero violations across 603 fenced blocks and 451
examples, so it lands as a regression gate rather than a backlog.
Verified against adversarial fixtures beyond the agent's own suite: the
original defect is caught with the correct real-parameter name, an alias
mid-pipeline and across backtick continuation is caught with accurate
line attribution, and an alias documented in a parameter table, a
genuine -Id parameter, and a non-PowerShell fence all pass clean.
Pester: 1102 passing, 0 failing (baseline 1093).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cs policy Adds src/JIM.PowerShell/CLAUDE.md, which auto-loads when working in the module subtree. Every rule in it was broken and shipped during the recent audit; two of the resulting examples destroyed data when copy-pasted. It leads with why the module needs its own rules: PowerShell resolves an unknown property to $null rather than erroring, and $null is falsy, so a wrong property name in a negated filter matches every row instead of none. That is how "remove all disabled certificates" became "remove all certificates". Nothing in dotnet build, dotnet test or Pester detects it. Covers the parameter-alias hazard in full, since it has no mechanical guard at the point of declaration: why aliases exist (pipeline binding matches on property name), that an alias cannot be pipeline-only so it always becomes a command-line spelling too, that removing one to fix the ambiguity would break documented piping, that PowerShell rejects an 'Id' alias outright when the cmdlet has its own -Id parameter, and the -InputObject pattern to use when it does. Also documents output shapes differing per parameter set, destructive-example labelling, keeping docs and behaviour in agreement, and Pester practice. Separately, docs/CLAUDE.md asserted that cmdlet parameter tables live in autogenerated help "not in mkdocs". No generation exists anywhere in the repository, and those pages hand-maintain 37 to 61 table rows each, so the policy described a system that was never built whilst the real, drift-prone duplication went unacknowledged. That duplication is the root cause of the audit's findings. Corrected to state the exception and name generation as the fix that would remove the class of defect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ents Sequencing uses the REST blocked-by dependency endpoints, containment uses the GraphQL addSubIssue/removeSubIssue mutations; plain gh issue commands expose neither, which is how prose comments crept in as the relationship mechanism. Bodies keep the rationale only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ario8-membership-batching
…ario8-membership-batching
JayVDZ
enabled auto-merge (squash)
July 21, 2026 11:01
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Redundant ToString() call, missed Where/Select opportunities, and ContainsKey+indexer instead of TryGetValue; no behavioural change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
origin/main, including the Dependabot batch that bumpedSystem.Security.Cryptography.Xml(and related Microsoft.* packages) to 10.0.10, resolving an NU1903 high-severity audit failure that was blocking buildsTest plan
dotnet build JIM.slnclean, 0 warnings/errorsdotnet test JIM.slnclean: 3,780 tests, 0 failed (111 skipped)🤖 Generated with Claude Code