You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Close all 9 DSL migration gaps (spec 038): identity resolution pipeline + Nodes/Teams/OTel fixes (#122)
## Summary
Closes **all 9 open gaps** in `analysis/dsl-gaps-detected.md` (GAP-001 →
GAP-009) — spec
[`038-close-dsl-gaps`](specs/038-close-dsl-gaps/spec.md). All gaps are
marked `Status: RESOLVED` (0 OPEN). Green on **net10 and net481**; full
suite **1,411 passed / 2 skipped (live ADO) / 0 failed**.
## What changed
**Identity resolution pipeline (GAP-001)** — the headline work:
- New abstractions in `Abstractions.Agent/Identity`: `IIdentityAdapter`,
`IdentityCandidate`, `IIdentityMatchingStrategy` (+ `IdentityMatch`).
- `IdentitiesOrchestrator.PrepareAsync` implements resolution steps 2–3
(UPN → display-name) against the **target tenant**, caches results,
writes `prepare-report.json`, emits the `identity.prepare` span +
`identity.adapter.query` child spans + the full prepare metric set.
- `IIdentityTranslationTool.Translate()` (renamed from
`IIdentityLookupTool.Resolve()` — **history preserved via `git mv`**)
reads the cache synchronously. Adds `DefaultIdentity`.
- **All three connectors** implement `IIdentityAdapter` (FR-005/019):
`SimulatedIdentityAdapter`, `AzureDevOpsIdentityAdapter` (ADO SDK
`IdentityHttpClient`), `TfsIdentityAdapter` (net481, reduced-capability
empty+Warning). Dispatched by `CompositeIdentityAdapter` on target
connector type via `AddIdentityAdapter<T>`.
- Matching strategies **own their adapter fetch** (`ResolveAsync`) — no
magic-string dispatch in the orchestrator.
**Other gaps:**
- GAP-002/003: `NodesModule` `ReplicateSourceTree` skip-guard;
`_NodeTransformTool`→`_nodeTranslationTool`; `INodeEnsurer` confirmed
absent.
- GAP-004: default-team structured warning verified.
- GAP-005: `TeamImportOrchestrator.TranslatePath()` returns `null` on
untranslatable (no silent pass-through); callers skip + warn.
- GAP-006: skip team members whose identity resolves to the configured
default.
- GAP-007: deleted the architecturally-impossible CLI
`@us1-write-idempotency` scenario (CLI has no package-fs access —
Principle VI).
- GAP-008/009: OTel in-memory exporter tests (`ExportMetricsTests`,
per-test-scoped `MeterProvider`).
**Compliance:** net481 guard remediation (FR-018/020 — removed
non-compliant interface + DI-hiding `#if` guards); observability gate
applied; doc-sync (`.agents/30-context`, operator-guide,
configuration-reference).
## Reviewer notes / known divergences (tracked in
`specs/038-close-dsl-gaps/discrepancies.md`)
- **ATDD form**: scenarios are covered by **code-first DSL + unit + live
SystemTests** (this repo's evolved convention), not new Reqnroll
bindings. The stale identity feature file rewrite is the remaining
cleanup.
- **`ImportAsync` tool-param** retained as a deliberate
**DI-cycle-avoidance** seam (tool→orchestrator dependency); full
lifecycle consolidation is a focused follow-up.
- Metric names are `platform.workitems.export.*` (the spec's
`migration.*` names were illustrative) — D-007.
- Stale "TFS is source-only" comments across pre-existing files spawned
as a separate cleanup task — D-006.
## Validation
- `dotnet build --no-incremental` — 0 errors (net10 + net481).
- `dotnet test` — 1,411 passed / 2 skipped / 0 failed across 10
assemblies.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Improved identity resolution with UPN and display-name matching plus
connector-specific adapters.
* New agent-context extension to refresh the managed Spec Kit section in
agent context files.
* **Improvements**
* Identity translation seam with configurable fallbacks and clearer
skip-and-warn behaviours for teams and nodes.
* Nodes import now skips when replication is disabled.
* Deterministic per-test OpenTelemetry metrics support.
* **Documentation**
* Expanded spec, plan, quickstart and operator guides for DSL migration
gap closure.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Copy file name to clipboardExpand all lines: .agents/20-guardrails/workflow/testing-rules.md
+59-10Lines changed: 59 additions & 10 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -10,9 +10,9 @@ MSTest conventions, test naming, and organisation. See also: [coding-standards.m
10
10
| --- | --- | --- | --- | --- |
11
11
| 1 (highest) | Unit Tests |`[TestCategory("UnitTests")]`| < 50 ms | All logic, branching, transforms. No I/O, no DI. Single class/method in isolation. |
12
12
| 2 | Domain Tests (Internal DSL + MSTest) |`[TestCategory("DomainTests")]`| < 500 ms | Business behaviour across collaborating domain objects via the internal DSL. |
13
-
| 3 | Simulated System Tests |`[TestCategory("SystemTests_Simulated")]`| < 10 s | End-to-end with `Simulated` connector. No network. |
14
-
| 3a | Smoke System Tests |`[TestCategory("SystemTests_Smoke")]`| < 30 s | Critical-path subset of system tests run on every PR. |
15
-
| 4 (lowest) | Live System Tests |`[TestCategory("SystemTests")]`/`[TestCategory("SystemTests_Live")]`| < 60 s | Requires live ADO/TFS. Environment-gated. |
13
+
| 3 | Simulated System Tests |`[TestCategory("SystemTest_Simulated")]`| < 10 s | End-to-end with `Simulated` connector. No network. |
14
+
| 3a | Smoke System Tests |`[TestCategory("SystemTest_Smoke")]`| < 30 s | Critical-path subset of system tests run on every PR. |
15
+
| 4 (lowest) | Live System Tests |`[TestCategory("SystemTest")]`/`[TestCategory("SystemTest_Live")]`| < 60 s | Requires live ADO/TFS. Environment-gated. |
16
16
17
17
### Distinguishing UnitTests from DomainTests
18
18
@@ -44,17 +44,17 @@ Every time a test file is **created, edited, moved, or touched in any way**, eve
44
44
| --- | --- |
45
45
| Test uses `DevOpsMigrationPlatform.Testing` DSL |`[TestCategory("DomainTests")]`|
46
46
| Test is isolated unit test (no DSL, no I/O) |`[TestCategory("UnitTests")]`|
47
-
| Test uses `Simulated` connector end-to-end |`[TestCategory("SystemTests_Simulated")]`|
48
-
| Test is a critical-path smoke subset |`[TestCategory("SystemTests_Smoke")]`|
49
-
| Test targets live ADO/TFS |`[TestCategory("SystemTests")]` or `[TestCategory("SystemTests_Live")]`|
47
+
| Test uses `Simulated` connector end-to-end |`[TestCategory("SystemTest_Simulated")]`|
48
+
| Test is a critical-path smoke subset |`[TestCategory("SystemTest_Smoke")]`|
49
+
| Test targets live ADO/TFS |`[TestCategory("SystemTest")]` or `[TestCategory("SystemTest_Live")]`|
50
50
51
51
**Enforcement rules — all are blocking, none are optional:**
52
52
53
53
1.**Missing tag on touch:** If any `[TestMethod]` or `[TestClass]` in a touched file lacks `[TestCategory]`, add the correct tag in the same edit. This applies to every method in the file, not just the method being modified.
54
54
2.**Wrong tag on touch:** If a tag is incorrect (wrong category, old name), correct it in the same edit.
55
55
3.**Delegation does not exempt:** If a sub-agent or delegated run added or modified a test, the calling agent is responsible for verifying tags before closing the task. "The delegated run didn't add it" is not a valid completion state.
56
56
4.**No partial compliance:** Applying the tag to the new method while leaving existing uncategorised methods in the same file is non-compliant. Fix the whole file.
57
-
5.**Category names are canonical:** Only the exact strings `UnitTests`, `DomainTests`, `SystemTests_Simulated`, `SystemTests_Smoke`, `SystemTests`, `SystemTests_Live` are valid. Any other value is non-compliant and must be corrected on contact.
57
+
5.**Category names are canonical:** Only the exact strings `UnitTests`, `DomainTests`, `SystemTest_Simulated`, `SystemTest_Smoke`, `SystemTest`, `SystemTest_Live` are valid. Any other value is non-compliant and must be corrected on contact.
58
58
6. The `nkda-testdsl-*` skills must apply `[TestCategory("DomainTests")]` to all converted tests.
59
59
7. The `nkda-testdsl-refactor` skill must verify and correct all category tags in any file it touches.
60
60
@@ -66,11 +66,60 @@ Every time a test file is **created, edited, moved, or touched in any way**, eve
66
66
67
67
---
68
68
69
+
## Mandatory DSL Migration (Touch = Convert)
70
+
71
+
> **HARD GATE — no exceptions, no deferrals, no delegation excuses.**
72
+
> Legacy Reqnroll is a **migration debt**, not an editable test style. The moment you need to
73
+
> *change the behaviour* of a legacy `.feature` file or its `*Steps.cs`, that feature family
74
+
> MUST be migrated to the internal DSL — you do not edit the old style in place.
75
+
76
+
**Trigger.** Any change to the *behaviour or scenarios* of a legacy Reqnroll `.feature` file, or
77
+
to its `[Binding]`/`[Given]`/`[When]`/`[Then]` step definitions, obligates migration of that
78
+
**whole feature family** to the code-first internal DSL before the task is complete. This sits
79
+
alongside Touch = Tag: a touched legacy test must be both migrated **and** correctly categorised.
80
+
81
+
**How.** Migration is performed *only* via the DSL orchestration skill — do not hand-roll it:
82
+
83
+
```text
84
+
nkda-testdsl-autonomous {feature}
85
+
```
86
+
87
+
`{feature}` is the touched `.feature` path, step-file path, family folder, or named family. The
88
+
skill runs the full loop (assess → DSL design → extraction → conversion → refactor → verification)
89
+
and produces code-first `[TestCategory("DomainTests")]` tests under
90
+
`tests/<Project>.Tests/<Area>/<Behaviour>Tests.cs` using the
Copy file name to clipboardExpand all lines: .agents/30-context/domains/connector-model.md
+11-1Lines changed: 11 additions & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -22,7 +22,17 @@ Target connector coverage evolves by capability; do not treat current TFS target
22
22
23
23
## Client Integration Boundary
24
24
25
-
Azure DevOps connectors obtain clients from `IAzureDevOpsClientFactory`. This is the only permitted way to create API clients. No direct `VssConnection` instantiation.
25
+
Azure DevOps connectors obtain clients from `IAzureDevOpsClientFactory`. This is the only permitted way to create API clients. No direct `VssConnection` instantiation. (The identity adapter uses `IAzureDevOpsClientFactory.CreateIdentityClientAsync` → `IdentityHttpClient`.)
26
+
27
+
## Identity Adapter (`IIdentityAdapter`)
28
+
29
+
`IIdentityAdapter` is the connector abstraction for querying the **target** tenant by UPN/display name during the Prepare phase (distinct from `IIdentitySource`, which enumerates the source at export time). Each connector lives at its own project boundary — **no `#if` guards**:
-`TfsIdentityAdapter` (Infrastructure.TfsObjectModel, net481) — reduced capability: returns empty + structured Warning (the TFS Identity Service does not expose UPN/display-name search). Modeled explicitly in the contract result, not a stub.
34
+
35
+
Registered via `AddIdentityAdapter<T>("<connectorType>")` and dispatched by `CompositeIdentityAdapter` on `ITargetEndpointInfo.ConnectorType`.
Copy file name to clipboardExpand all lines: .agents/30-context/domains/identity-and-mapping.md
+16-1Lines changed: 16 additions & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -60,7 +60,22 @@ During `PrepareAsync`, the `IdentitiesModule`:
60
60
61
61
### "Identity is a Cross-Cutting Service" Rule
62
62
63
-
No module should implement its own identity resolution. All modules consume the `IIdentityMappingService` injected at construction. The `IdentitiesModule` must complete export before any other module begins import.
63
+
No module should implement its own identity resolution. All modules consume the cross-cutting identity seam injected at construction. The `IdentitiesModule` must complete export before any other module begins import.
64
+
65
+
### Identity Resolution Architecture (spec 038)
66
+
67
+
The resolution pipeline is composed of four collaborating abstractions:
68
+
69
+
| Abstraction | Role | Lives in |
70
+
|---|---|---|
71
+
|`IIdentityTranslationTool`|**Tool seam** consumed by all modules (`TeamImportOrchestrator`, `RevisionFolderProcessor`, `WorkItemsModule`, `IdentitiesModule`). `Translate(source)` is **synchronous** and reads cached Prepare-phase results only. Exposes `IsEnabled` and `DefaultIdentity`. Configured under `MigrationPlatform:Tools:IdentityTranslation`. (Renamed from the former `IIdentityLookupTool`; `Resolve()` → `Translate()`.) |`Abstractions.Agent/Tools`|
72
+
|`IIdentitiesOrchestrator`| Owns the phase lifecycle (`PrepareAsync`/`ExportAsync`/`ImportAsync`/`ValidateAsync`). `PrepareAsync` performs the live matching, caches results (`source → target descriptor`), and writes `prepare-report.json`. `ResolvePrepared(source)` is the synchronous cache read the tool delegates to. |`Abstractions.Agent/Modules`|
73
+
|`IIdentityAdapter`|**Connector-specific** query of the live **target** tenant (`FindByUpnAsync`, `FindByDisplayNameAsync`). Called by `PrepareAsync` only. Implementations: `SimulatedIdentityAdapter`, `AzureDevOpsIdentityAdapter` (SDK `IdentityHttpClient`), `TfsIdentityAdapter` (reduced capability — empty + Warning). Dispatched by `CompositeIdentityAdapter` on the target connector type. |`Abstractions.Agent/Identity` (+ connector projects) |
74
+
|`IIdentityMatchingStrategy`|**Pure** ordered match step. `UpnIdentityMatchingStrategy` (step 2) then `DisplayNameIdentityMatchingStrategy` (step 3, Unicode-NFC, case-insensitive). Returns `IdentityMatch(Descriptor, MatchCount)`; ambiguity (`MatchCount > 1`) is logged by the orchestrator. |`Abstractions.Agent/Identity`|
75
+
76
+
**Four-step order** (executed in PrepareAsync for steps 2–3; in the tool for 1 & 4): (1) explicit `mapping.json` override → (2) UPN match → (3) display-name match → (4) configured `DefaultIdentity`, else source pass-through. Ambiguous matches and adapter failures are **not** treated as resolved: PrepareAsync logs a structured warning, records them as **unresolved** in `prepare-report.json`, and they remain subject to the import-blocking rule above (line 59). The step-4 default/pass-through applies only at translation time for identities the operator has not subsequently mapped.
77
+
78
+
`IIdentityMappingService` remains the override/default resolver the orchestrator consults internally; modules do not call it directly — they go through `IIdentityTranslationTool`.
Refresh the managed Spec Kit section inside the active coding agent's context/instruction file (e.g. `CLAUDE.md`, `.github/copilot-instructions.md`, `AGENTS.md`).
11
+
12
+
## Behavior
13
+
14
+
The script reads the agent-context extension config at
15
+
`.specify/extensions/agent-context/agent-context-config.yml` to discover:
16
+
17
+
-`context_file` — the path of the coding agent context file to manage.
18
+
-`context_markers.start` / `.end` — the delimiters surrounding the managed section. Defaults to `<!-- SPECKIT START -->` and `<!-- SPECKIT END -->` when the field is missing.
19
+
20
+
It then creates, replaces, or appends the managed block so that the section points at the most recent plan path when one can be discovered (`specs/<feature>/plan.md`).
21
+
22
+
If `context_file` is empty or the file cannot be located, the command reports nothing to do and exits successfully.
Refresh the managed Spec Kit section inside the active coding agent's context/instruction file (e.g. `CLAUDE.md`, `.github/copilot-instructions.md`, `AGENTS.md`).
11
+
12
+
## Behavior
13
+
14
+
The script reads the agent-context extension config at
15
+
`.specify/extensions/agent-context/agent-context-config.yml` to discover:
16
+
17
+
-`context_file` — the path of the coding agent context file to manage.
18
+
-`context_markers.start` / `.end` — the delimiters surrounding the managed section. Defaults to `<!-- SPECKIT START -->` and `<!-- SPECKIT END -->` when the field is missing.
19
+
20
+
It then creates, replaces, or appends the managed block so that the section points at the most recent plan path when one can be discovered (`specs/<feature>/plan.md`).
21
+
22
+
If `context_file` is empty or the file cannot be located, the command reports nothing to do and exits successfully.
0 commit comments