diff --git a/docs/adapters/azuredevops.md b/docs/adapters/azuredevops.md index 95485c85..7223950b 100644 --- a/docs/adapters/azuredevops.md +++ b/docs/adapters/azuredevops.md @@ -273,10 +273,17 @@ adapter.import_artifact( ) # Access imported proposal -proposal = project_bundle.change_tracking.proposals["123"] +proposal = project_bundle.change_tracking.proposals["add-feature-x"] print(f"Imported: {proposal.title} - {proposal.status}") ``` +Selective imports preserve the native Azure DevOps payload, including +`fields`, so the imported work item remains valid input for proposal parsing and +bridge sync flows. When no OpenSpec metadata is present, imported proposal names +fall back to a title-derived slug such as `add-feature-x`; if that slug already +exists, the adapter appends the source work item ID (for example, +`add-feature-x-123`) to keep the proposal name stable and readable. + ### Status Synchronization ```python diff --git a/openspec/CHANGE_ORDER.md b/openspec/CHANGE_ORDER.md index 02f36d91..9600f69a 100644 --- a/openspec/CHANGE_ORDER.md +++ b/openspec/CHANGE_ORDER.md @@ -157,6 +157,7 @@ These are derived extensions of the same 2026-02-15 plan and are required to ope | backlog-core | 06 | ✅ backlog-core-06-refine-custom-field-writeback (implemented 2026-03-03; archived) | [#310](https://github.com/nold-ai/specfact-cli/issues/310) | #173 | | backlog-core | 07 | backlog-core-07-ado-required-custom-fields-and-picklists | [#337](https://github.com/nold-ai/specfact-cli/issues/337) | ✅ #310 | | bugfix | 01 | bugfix-backlog-html-export-validation | TBD | — | +| bugfix | 02 | bugfix-02-ado-import-payload-slugging | [#427](https://github.com/nold-ai/specfact-cli/issues/427) | — | ### backlog-scrum diff --git a/openspec/changes/bugfix-02-ado-import-payload-slugging/TDD_EVIDENCE.md b/openspec/changes/bugfix-02-ado-import-payload-slugging/TDD_EVIDENCE.md new file mode 100644 index 00000000..83f4140d --- /dev/null +++ b/openspec/changes/bugfix-02-ado-import-payload-slugging/TDD_EVIDENCE.md @@ -0,0 +1,40 @@ +# TDD Evidence: bugfix-02-ado-import-payload-slugging + +## Failing First + +- `2026-03-20`: `hatch run pytest tests/unit/adapters/test_ado.py tests/unit/adapters/test_github.py tests/integration/sync/test_ado_backlog_sync.py tests/integration/sync/test_backlog_sync.py tests/unit/sync/test_bridge_sync_import.py -q` + failed with `5 failed, 73 passed` before the fix. The failures showed two regressions: + selective ADO fetch reduced the work item to a summary payload without native + `fields`, and shared backlog import fell back to numeric-only change IDs such + as `123` instead of title-derived slugs. +- `2026-03-20`: audit of adjacent import paths confirmed that the shared + `import_backlog_item_as_proposal()` helper is used by both the ADO and GitHub + adapters, so the title-first normalization fix had to be applied in the + shared backlog adapter rather than only in the ADO implementation. + +## Passing + +- `2026-03-20`: `hatch run pytest tests/unit/adapters/test_ado.py tests/unit/adapters/test_github.py tests/unit/sync/test_bridge_sync_import.py -q` + passed with `76 passed`. This covers native selective fetch for ADO and + GitHub, title-first imported change IDs, deterministic collision suffixes, + and the bridge selective-import contract. + +- `2026-03-20`: `openspec validate bugfix-02-ado-import-payload-slugging --strict` + passed. +- `2026-03-20`: `hatch run format` + passed after reformatting the touched files. +- `2026-03-20`: `hatch run type-check` + passed with `0 errors`. +- `2026-03-20`: `hatch run yaml-lint` + passed. +- `2026-03-20`: `hatch run contract-test` + exited `0` using cached results (`No modified files detected - using cached results`). +- `2026-03-20`: `hatch run smart-test` + exited `0`; the command skipped mapped test execution for this delta and emitted only its standard coverage warning. +- `2026-03-20`: `hatch run specfact code review run --json --out .codex-bugfix-02-review.json src/specfact_cli/adapters/ado.py src/specfact_cli/adapters/backlog_base.py src/specfact_cli/adapters/github.py tests/unit/adapters/test_ado.py tests/unit/adapters/test_github.py tests/unit/sync/test_bridge_sync_import.py tests/integration/sync/test_ado_backlog_sync.py tests/integration/sync/test_backlog_sync.py` + exported the governed review report to `.codex-bugfix-02-review.json`. The overall report still fails because the touched legacy adapter files already carry a large historical baseline, but filtering the report to the lines changed by this fix yields `0` findings. + +## Outstanding Gate Caveat + +- `2026-03-20`: `hatch run lint` + still exits non-zero in this worktree because the repository-wide lint script (`ruff format . --check && basedpyright ... && ruff check . && pylint src tests tools`) surfaces existing baseline findings outside this bugfix. The modified lines for this change were reviewed separately via `.codex-bugfix-02-review.json` and returned `0` changed-line findings. diff --git a/openspec/changes/bugfix-02-ado-import-payload-slugging/design.md b/openspec/changes/bugfix-02-ado-import-payload-slugging/design.md new file mode 100644 index 00000000..361c1142 --- /dev/null +++ b/openspec/changes/bugfix-02-ado-import-payload-slugging/design.md @@ -0,0 +1,54 @@ +# Design: Fix ADO selective bridge import payload contract and title-based change IDs + +## Overview + +The failure in issue `#425` is a contract break inside the selective bridge import path: + +1. `BridgeSync` fetches one backlog item by ID for import. +2. `AdoAdapter.fetch_backlog_item()` returns a reduced summary payload. +3. `AdoAdapter.import_artifact()` and `extract_change_proposal_data()` still expect the native ADO work item shape with `fields`. +4. Import fails before OpenSpec change creation. + +Once the raw payload is restored, the fallback naming path still degrades to the numeric ADO work item ID when no OpenSpec metadata is present. That behavior is technically unique but operationally poor. The fallback should stay readable first, with the numeric source ID kept as provenance, not as the primary OpenSpec change name. + +## Design Decisions + +### 1. Preserve the provider-native payload on selective import + +`fetch_backlog_item()` for ADO should return the native work item document needed by the import path. If older call sites benefit from convenience keys such as `title`, `state`, and `description`, those can remain as additive compatibility fields, but they must not replace or strip the provider-native shape. + +This keeps the import contract coherent: + +- `fetch_backlog_item()` returns an artifact suitable for `import_artifact()` +- `extract_change_proposal_data()` reads the same native payload +- contract tests can verify the round trip directly + +### 2. Normalize imported change IDs in one shared place + +The fallback naming rule should be centralized in the shared backlog import path rather than hidden in one adapter-specific edge case. The normalizer should: + +- prefer an existing OpenSpec change ID if found in provider metadata +- otherwise derive a kebab-case slug from the proposal title +- append a deterministic suffix such as `-` when the slug already exists or would otherwise be ambiguous +- avoid using a raw numeric source ID as the entire change name unless the source artifact truly has no usable title + +This lets ADO fix its immediate bug while also protecting similar adapters and commands. + +### 3. Audit adjacent import commands, not just the failing ADO branch + +The regression appeared because one side of the contract evolved while the other kept assuming a richer payload. The implementation should therefore audit all nearby paths that do one or more of: + +- call `fetch_backlog_item()` +- call `extract_change_proposal_data()` +- call `import_backlog_item_as_proposal()` +- translate provider IDs into OpenSpec change IDs + +The goal is not a broad refactor. The goal is to prove that similar commands either already preserve the native payload correctly or are covered by targeted defensive tests after this fix. + +## Implementation Outline + +1. Add failing tests for ADO selective import and title-based slug fallback. +2. Restore native payload preservation in ADO selective fetch. +3. Add shared title-first change-ID normalization in the backlog import path. +4. Re-run targeted tests against adjacent adapters/call sites to confirm the contract holds. +5. Update docs and release notes for the patch behavior change. diff --git a/openspec/changes/bugfix-02-ado-import-payload-slugging/proposal.md b/openspec/changes/bugfix-02-ado-import-payload-slugging/proposal.md new file mode 100644 index 00000000..6c1bb4d0 --- /dev/null +++ b/openspec/changes/bugfix-02-ado-import-payload-slugging/proposal.md @@ -0,0 +1,45 @@ +# Change: Fix ADO selective bridge import payload contract and title-based change IDs + +## Why + +Issue [#425](https://github.com/nold-ai/specfact-cli/issues/425) confirms a real regression in the ADO selective import path used by `specfact project sync bridge --adapter ado --mode bidirectional --backlog-ids `. The current bridge flow calls `fetch_backlog_item()` and then imports the returned artifact as an OpenSpec change proposal, but `AdoAdapter._get_work_item_data()` strips the provider payload down to summary fields and drops the native `fields` object that `AdoAdapter.import_artifact()` and `extract_change_proposal_data()` still expect. Valid ADO work items therefore fail import with `ADO work item must have fields`. + +After a local hotfix restores the raw payload, the follow-up behavior is still wrong: when the ADO item does not already contain OpenSpec metadata in its description or comments, the generated change ID falls back to the numeric work item ID instead of a readable slug derived from the title. That makes imported OpenSpec changes hard to review, easy to collide, and inconsistent with the rest of the backlog import workflow. + +This change fixes both defects together and explicitly audits adjacent import commands that rely on the same `fetch_backlog_item()` to `import_artifact()` contract so we do not repeat the same summary-vs-native payload mistake in nearby adapters or bridge entry points. + +## What Changes + +- **MODIFY** `AdoAdapter.fetch_backlog_item()` and its internal work-item fetch helpers so selective ADO import returns the native work item payload, including `fields`, while preserving convenience keys such as `title`, `state`, and `description` for existing callers. +- **MODIFY** the ADO change-proposal extraction path so imported proposals derive a kebab-case change ID from the work item title when no OpenSpec metadata is already embedded in the source artifact. +- **MODIFY** shared backlog import normalization so duplicate or numeric-only fallback IDs get a deterministic suffix that keeps the readable slug and reserves the provider numeric ID for source tracking metadata instead of primary naming. +- **ADD** regression coverage for selective `project sync bridge` / bridge import flows, including raw payload preservation, title-based slug generation, duplicate-slug collision handling, and cross-checks for similar adapter import commands. +- **REVIEW** adjacent bridge/adapters that call `fetch_backlog_item()`, `extract_change_proposal_data()`, or `import_backlog_item_as_proposal()` so similar commands either share the same helper or are covered by explicit contract tests. + +## Capabilities + +### Modified Capabilities + +- `devops-sync`: selective ADO bridge imports must preserve the provider-native work item payload required for OpenSpec proposal import and must create readable change IDs when no prior metadata exists. +- `backlog-adapter`: adapter import contracts must preserve required native fields during single-item import and normalize imported proposal IDs title-first instead of defaulting to raw numeric source IDs. + +## Impact + +- **Affected code**: `src/specfact_cli/adapters/ado.py`, `src/specfact_cli/adapters/backlog_base.py`, and the selective import orchestration in `src/specfact_cli/sync/bridge_sync.py`; adjacent adapter call sites may need small defensive updates if the audit finds the same contract gap elsewhere. +- **Affected tests**: targeted unit/integration coverage under `tests/unit/adapters/`, `tests/unit/specfact_cli/adapters/`, `tests/unit/specfact_cli/sync/`, and any command-audit coverage that validates bridge command surfaces. +- **Documentation**: user-facing sync and ADO adapter docs in `docs/` plus any command reference examples that show selective bridge import should be reviewed so the fixed import behavior and change-ID expectations are documented. +- **Release impact**: patch release. No new command surface, but behavior changes are user-visible because valid ADO work items will import successfully and produce stable human-readable OpenSpec change IDs. +- **Sequencing**: no hard blocker in `openspec/CHANGE_ORDER.md`; the change should still be linked under backlog feature #357 and epic #186 and back to the originating bug issue. + +## Related Issues + +- Originating bug report: [#425](https://github.com/nold-ai/specfact-cli/issues/425) +- Parent feature: [#357](https://github.com/nold-ai/specfact-cli/issues/357) +- Parent epic: [#186](https://github.com/nold-ai/specfact-cli/issues/186) + +## Source Tracking + +- **GitHub Issue**: #427 +- **Issue URL**: +- **Repository**: nold-ai/specfact-cli +- **Last Synced Status**: open diff --git a/openspec/changes/bugfix-02-ado-import-payload-slugging/specs/backlog-adapter/spec.md b/openspec/changes/bugfix-02-ado-import-payload-slugging/specs/backlog-adapter/spec.md new file mode 100644 index 00000000..6ac1f711 --- /dev/null +++ b/openspec/changes/bugfix-02-ado-import-payload-slugging/specs/backlog-adapter/spec.md @@ -0,0 +1,22 @@ +## MODIFIED Requirements + +### Requirement: BacklogAdapter Interface + +The system SHALL provide a standard `BacklogAdapter` interface that all backlog sources (GitHub, ADO, JIRA, GitLab, etc.) must implement. + +#### Scenario: Selective proposal import preserves provider-native payload + +- **GIVEN** an adapter supports selective backlog import by explicit item reference +- **WHEN** bridge sync fetches one item through `fetch_backlog_item()` and passes the result into proposal import +- **THEN** the fetched artifact preserves the provider-native fields required by `extract_change_proposal_data()` or `import_artifact()` +- **AND** adapter-specific convenience fields may be added without discarding the native structure +- **AND** contract tests cover the `fetch_backlog_item()` to `import_artifact()` round trip for supported adapters + +#### Scenario: Imported proposal IDs normalize title-first across adapters + +- **GIVEN** an imported backlog artifact has no embedded OpenSpec change ID metadata +- **AND** the source artifact has a usable human-readable title +- **WHEN** the adapter or shared backlog import path constructs the proposal change ID +- **THEN** the change ID is derived from a normalized title slug +- **AND** a numeric provider ID is used only as source tracking metadata or as a deterministic suffix when needed for uniqueness +- **AND** the system does not default to a numeric-only change name while a usable title is available diff --git a/openspec/changes/bugfix-02-ado-import-payload-slugging/specs/devops-sync/spec.md b/openspec/changes/bugfix-02-ado-import-payload-slugging/specs/devops-sync/spec.md new file mode 100644 index 00000000..634dbae6 --- /dev/null +++ b/openspec/changes/bugfix-02-ado-import-payload-slugging/specs/devops-sync/spec.md @@ -0,0 +1,29 @@ +## MODIFIED Requirements + +### Requirement: Azure DevOps Backlog Sync Support + +The system SHALL support Azure DevOps work items as a backlog adapter in the DevOps sync workflow. + +#### Scenario: Selective ADO import preserves native payload for proposal import + +- **GIVEN** a user runs `specfact project sync bridge --adapter ado --mode bidirectional --backlog-ids 123456` +- **WHEN** bridge sync fetches that single ADO work item for import as an OpenSpec change proposal +- **THEN** the adapter returns the provider-native work item payload with a populated `fields` object +- **AND** the payload may include convenience keys such as `title`, `state`, or `description` without removing the native `fields` structure +- **AND** proposal import does not fail for a valid work item with `ADO work item must have fields` + +#### Scenario: Selective ADO import derives a human-readable change ID when metadata is absent + +- **GIVEN** an imported ADO work item has no existing OpenSpec change ID embedded in its description or comments +- **AND** the work item title is `Selective import keeps ADO payload` +- **WHEN** the adapter generates the OpenSpec change proposal during import +- **THEN** the resulting change ID is derived from the title as kebab-case +- **AND** the work item numeric ID remains in source tracking metadata instead of becoming the entire change name + +#### Scenario: Duplicate title slug appends deterministic source suffix + +- **GIVEN** a title-derived slug already exists in `openspec/changes/` +- **AND** another imported ADO work item with ID `123456` resolves to the same title slug +- **WHEN** the second proposal is created +- **THEN** the final change ID keeps the readable title slug and appends a deterministic suffix such as `-123456` +- **AND** the system does not fall back to using only the raw numeric work item ID as the change name diff --git a/openspec/changes/bugfix-02-ado-import-payload-slugging/tasks.md b/openspec/changes/bugfix-02-ado-import-payload-slugging/tasks.md new file mode 100644 index 00000000..970f1fb9 --- /dev/null +++ b/openspec/changes/bugfix-02-ado-import-payload-slugging/tasks.md @@ -0,0 +1,61 @@ +## 0. GitHub sync + +- [x] 0.1 Export or sync this change to a `[Change]` GitHub issue in `nold-ai/specfact-cli` +- [x] 0.2 Update `proposal.md` Source Tracking with the synced issue number, URL, repository, and status +- [x] 0.3 Ensure the synced change issue has labels `enhancement`, `openspec`, and `change-proposal` +- [x] 0.4 Add the synced change issue to the `SpecFact CLI` GitHub project +- [x] 0.5 Link the synced change issue under parent feature `#357` (epic lineage `#186`) +- [x] 0.6 Record the relation back to originating bug `#425` + +## 1. Branch and baseline + +- [x] 1.1 Create worktree: `scripts/worktree.sh create bugfix/bugfix-02-ado-import-payload-slugging` +- [x] 1.2 Bootstrap Hatch in the worktree: `hatch env create` +- [x] 1.3 Reproduce the current selective ADO import failure and record the failing command/output in `openspec/changes/bugfix-02-ado-import-payload-slugging/TDD_EVIDENCE.md` +- [x] 1.4 Audit adjacent import paths that use `fetch_backlog_item()`, `extract_change_proposal_data()`, or `import_backlog_item_as_proposal()` and record the scoped call sites before code changes + +## 2. Write failing tests from spec scenarios + +- [x] 2.1 Add unit tests in `tests/unit/adapters/test_ado.py` proving selective `fetch_backlog_item()` preserves the native ADO payload with populated `fields` +- [x] 2.2 Add unit tests in `tests/unit/adapters/test_ado.py` proving imported ADO change IDs derive from title slugs when no OpenSpec metadata exists +- [x] 2.3 Add collision tests proving duplicate title slugs append a deterministic source-ID suffix instead of degrading to numeric-only names +- [x] 2.4 Add bridge/import contract tests in the selective import path (`tests/unit/specfact_cli/sync/` or adjacent bridge tests) proving `fetch_backlog_item()` output remains valid input for proposal import +- [x] 2.5 Add or extend audit coverage for similar adapter commands so nearby `fetch_backlog_item()` implementations are checked for the same contract assumption +- [x] 2.6 Run targeted tests and capture the failing results in `TDD_EVIDENCE.md` + +## 3. Implement payload preservation and title-first slugging + +- [x] 3.1 Update `src/specfact_cli/adapters/ado.py` so selective ADO fetch returns the native work item payload while preserving compatibility summary keys +- [x] 3.2 Add shared title-first change-ID normalization in `src/specfact_cli/adapters/backlog_base.py` or a nearby shared helper used by proposal import +- [x] 3.3 Update ADO proposal extraction/import flow to use the shared normalizer and keep numeric source IDs in source tracking instead of primary naming +- [x] 3.4 Patch any adjacent adapter or bridge call sites found in the audit where the same summary-vs-native payload mistake or numeric-only fallback can occur +- [x] 3.5 Improve diagnostics or guard rails so missing native payload structure is reported clearly if an adapter violates the import contract in future + +## 4. Verify and evidence + +- [x] 4.1 Re-run the targeted adapter and bridge tests; record passing results in `TDD_EVIDENCE.md` +- [x] 4.2 Run `hatch run format` +- [x] 4.3 Run `hatch run type-check` +- [ ] 4.4 Run `hatch run lint` +- [x] 4.5 Run `hatch run yaml-lint` +- [x] 4.6 Run `hatch run contract-test` +- [x] 4.7 Run `hatch run smart-test` + +## 5. Documentation research and update + +- [x] 5.1 Review affected docs in `docs/`, `README.md`, and command references for selective bridge import and ADO adapter behavior +- [x] 5.2 Update the relevant ADO sync/import documentation to describe the corrected selective import behavior and title-based change-ID fallback +- [ ] 5.3 If new or moved docs are required, verify front matter and sidebar navigation entries in `docs/_layouts/default.html` + +## 6. Module signing, version, and changelog + +- [ ] 6.1 Run `hatch run ./scripts/verify-modules-signature.py --require-signature` +- [ ] 6.2 If any module manifests changed, bump module versions and re-sign before re-running verification +- [ ] 6.3 Bump patch version across the required version files +- [ ] 6.4 Add a `CHANGELOG.md` entry for the bugfix release describing the ADO import contract fix and title-based slugging correction + +## 7. PR and cleanup + +- [ ] 7.1 Open a PR from `bugfix/bugfix-02-ado-import-payload-slugging` to `dev` +- [ ] 7.2 Ensure CI passes and the PR links both the synced change issue and bug `#425` +- [ ] 7.3 After merge, remove the worktree and delete the local branch diff --git a/src/specfact_cli/adapters/ado.py b/src/specfact_cli/adapters/ado.py index 5fbfeae6..adafab50 100644 --- a/src/specfact_cli/adapters/ado.py +++ b/src/specfact_cli/adapters/ado.py @@ -426,7 +426,7 @@ def extract_change_proposal_data(self, item_data: dict[str, Any]) -> dict[str, A Change ID extraction priority: 1. Description footer (legacy format): *OpenSpec Change Proposal: `id`* 2. Comments (new format): **Change ID**: `id` in OpenSpec Change Proposal Reference comment - 3. Work item ID (fallback) + 3. Work item ID (fallback, normalized during shared proposal import) """ if not isinstance(item_data, dict): msg = "ADO work item data must be dict" @@ -689,7 +689,15 @@ def import_artifact( pass # Path operations will respect external_base_path in OpenSpec adapter # Import ADO work item as change proposal using backlog adapter pattern - proposal = self.import_backlog_item_as_proposal(artifact_path, "ado", bridge_config) + existing_proposals = ( + dict(project_bundle.change_tracking.proposals) if getattr(project_bundle, "change_tracking", None) else {} + ) + proposal = self.import_backlog_item_as_proposal( + artifact_path, + "ado", + bridge_config, + existing_proposals=existing_proposals, + ) if not proposal: msg = "Failed to import ADO work item as change proposal" @@ -1533,12 +1541,14 @@ def _get_work_item_data(self, work_item_id: int | str, org: str, project: str) - try: response = self._ado_get(url, headers=headers, timeout=10) work_item_data = response.json() + if not isinstance(work_item_data, dict): + return None fields = work_item_data.get("fields", {}) - return { - "title": fields.get("System.Title", ""), - "state": fields.get("System.State", ""), - "description": fields.get("System.Description", ""), - } + if isinstance(fields, dict): + work_item_data.setdefault("title", fields.get("System.Title", "")) + work_item_data.setdefault("state", fields.get("System.State", "")) + work_item_data.setdefault("description", fields.get("System.Description", "")) + return work_item_data except requests.HTTPError as e: if e.response is not None and e.response.status_code == 404: return None diff --git a/src/specfact_cli/adapters/backlog_base.py b/src/specfact_cli/adapters/backlog_base.py index 25b94275..139372a8 100644 --- a/src/specfact_cli/adapters/backlog_base.py +++ b/src/specfact_cli/adapters/backlog_base.py @@ -11,6 +11,7 @@ from __future__ import annotations +import re import time from abc import ABC, abstractmethod from datetime import UTC, datetime @@ -238,6 +239,142 @@ def extract_change_proposal_data(self, item_data: dict[str, Any]) -> dict[str, A tool-specific data formats (GitHub issue body, ADO work item fields, etc.). """ + @beartype + @ensure(lambda result: isinstance(result, str), "Must return string") + def _slugify_imported_change_title(self, title: str) -> str: + """Return a stable kebab-case slug for imported backlog titles.""" + normalized = re.sub(r"[^a-z0-9]+", "-", title.lower()).strip("-") + normalized = re.sub(r"-{2,}", "-", normalized) + return normalized or "change" + + @beartype + @ensure(lambda result: isinstance(result, str) and len(result) > 0, "Must return non-empty change id") + def _normalize_imported_change_id( + self, + proposal_data: dict[str, Any], + item_data: dict[str, Any], + tool_name: str, + existing_proposals: dict[str, ChangeProposal] | None = None, + ) -> str: + """Normalize imported change IDs so title-based slugs win over numeric fallbacks.""" + raw_change_id = self._get_imported_change_id_seed(proposal_data, item_data) + title = str(proposal_data.get("title") or item_data.get("title") or "").strip() + candidate = self._prefer_imported_title_slug(raw_change_id, title) + proposals = existing_proposals or {} + return self._dedupe_imported_change_id(candidate, raw_change_id, item_data, tool_name, proposals) + + @beartype + @ensure(lambda result: isinstance(result, str) and len(result) > 0, "Must return non-empty seed") + def _get_imported_change_id_seed(self, proposal_data: dict[str, Any], item_data: dict[str, Any]) -> str: + return str(proposal_data.get("change_id") or item_data.get("id") or item_data.get("number") or "unknown") + + @beartype + @ensure(lambda result: isinstance(result, str) and len(result) > 0, "Must return non-empty candidate") + def _prefer_imported_title_slug(self, raw_change_id: str, title: str) -> str: + if raw_change_id and raw_change_id != "unknown" and not raw_change_id.isdigit(): + return raw_change_id + if not title: + return raw_change_id or "unknown" + return self._slugify_imported_change_title(title) + + @beartype + @ensure(lambda result: isinstance(result, str) and len(result) > 0, "Must return non-empty change id") + def _dedupe_imported_change_id( + self, + candidate: str, + raw_change_id: str, + item_data: dict[str, Any], + tool_name: str, + existing_proposals: dict[str, ChangeProposal], + ) -> str: + existing_proposal = existing_proposals.get(candidate) + if existing_proposal is None: + return candidate or raw_change_id or "unknown" + + if self._matches_existing_import_source(existing_proposal, item_data, tool_name): + return candidate + + source_id = item_data.get("id") or item_data.get("number") + if source_id is None: + return candidate or raw_change_id or "unknown" + + deduped_candidate = f"{candidate}-{source_id}" + existing_deduped = existing_proposals.get(deduped_candidate) + if existing_deduped and self._matches_existing_import_source(existing_deduped, item_data, tool_name): + return deduped_candidate + return deduped_candidate + + @beartype + @ensure(lambda result: isinstance(result, str), "Must return source URL string") + def _get_import_source_url(self, item_data: dict[str, Any]) -> str: + html_url = item_data.get("html_url") + if isinstance(html_url, str) and html_url: + return html_url + + url = item_data.get("url") + if isinstance(url, str) and url: + return url + + links = item_data.get("_links") + if not isinstance(links, dict): + return "" + html_link = links.get("html") + if not isinstance(html_link, dict): + return "" + href = html_link.get("href") + return href if isinstance(href, str) else "" + + @beartype + @ensure(lambda result: isinstance(result, bool), "Must return match flag") + def _matches_existing_import_source( + self, + proposal: ChangeProposal, + item_data: dict[str, Any], + tool_name: str, + ) -> bool: + source_tracking = proposal.source_tracking + if source_tracking is None or not isinstance(source_tracking.source_metadata, dict): + return False + + source_id = item_data.get("id") or item_data.get("number") + source_id_str = str(source_id) if source_id is not None else None + source_url = self._get_import_source_url(item_data) + source_type = tool_name.lower() + source_metadata = source_tracking.source_metadata + backlog_entries = source_metadata.get("backlog_entries") + if isinstance(backlog_entries, list): + for entry in backlog_entries: + if isinstance(entry, dict) and self._source_metadata_matches( + entry, source_type, source_id_str, source_url + ): + return True + + fallback_metadata = dict(source_metadata) + fallback_metadata.setdefault("source_type", source_tracking.tool) + return self._source_metadata_matches(fallback_metadata, source_type, source_id_str, source_url) + + @beartype + @ensure(lambda result: isinstance(result, bool), "Must return match flag") + def _source_metadata_matches( + self, + source_metadata: dict[str, Any], + source_type: str, + source_id: str | None, + source_url: str, + ) -> bool: + entry_type = str(source_metadata.get("source_type") or source_metadata.get("tool") or "").lower() + if entry_type and entry_type != source_type: + return False + + entry_url = source_metadata.get("source_url") + if source_url and isinstance(entry_url, str) and entry_url == source_url: + return True + + entry_id = source_metadata.get("source_id") + if source_id is None or entry_id is None: + return False + return str(entry_id) == source_id + @beartype @require(lambda item_data: isinstance(item_data, dict), "Item data must be dict") @require(lambda tool_name: isinstance(tool_name, str) and len(tool_name) > 0, "Tool name must be non-empty") @@ -265,20 +402,16 @@ def create_source_tracking( """ source_metadata: dict[str, Any] = {} - # Extract common fields (ID, URL) if present source_id = None if tool_name.lower() == "github": source_id = item_data.get("number") or item_data.get("id") - # GitHub: convert to string for consistency (GitHub issue numbers are strings) if source_id is not None: source_metadata["source_id"] = str(source_id) else: - # For ADO and other adapters: preserve original type - # ADO work item IDs are integers, so keep as int source_id = item_data.get("id") or item_data.get("number") if source_id is not None: source_metadata["source_id"] = source_id - # Prefer html_url (user-friendly) over url (API URL) + if "html_url" in item_data: source_metadata["source_url"] = item_data.get("html_url") elif "url" in item_data: @@ -291,7 +424,6 @@ def create_source_tracking( assignees = [item_data["assignee"]] if item_data["assignee"] else [] source_metadata["assignees"] = assignees - # Add cross-repo support if bridge_config has external_base_path if bridge_config and hasattr(bridge_config, "external_base_path") and bridge_config.external_base_path: source_metadata["external_base_path"] = str(bridge_config.external_base_path) @@ -303,7 +435,8 @@ def create_source_tracking( "Status must be non-empty", ) @require( - lambda backlog_status: isinstance(backlog_status, str) and len(backlog_status) > 0, "Status must be non-empty" + lambda backlog_status: isinstance(backlog_status, str) and len(backlog_status) > 0, + "Status must be non-empty", ) @ensure(lambda result: isinstance(result, str), "Must return conflict resolution strategy name") def resolve_status_conflict( @@ -335,7 +468,6 @@ def resolve_status_conflict( if strategy == "prefer_backlog": return backlog_status if strategy == "merge": - # Status priority: applied > in-progress > proposed > deprecated > discarded status_priority = { "applied": 5, "in-progress": 4, @@ -347,7 +479,6 @@ def resolve_status_conflict( backlog_priority = status_priority.get(backlog_status, 0) return openspec_status if openspec_priority >= backlog_priority else backlog_status - # Default: prefer OpenSpec return openspec_status @beartype @@ -355,7 +486,11 @@ def resolve_status_conflict( @require(lambda tool_name: isinstance(tool_name, str) and len(tool_name) > 0, "Tool name must be non-empty") @ensure(lambda result: isinstance(result, ChangeProposal) or result is None, "Must return ChangeProposal or None") def import_backlog_item_as_proposal( - self, item_data: dict[str, Any], tool_name: str, bridge_config: Any = None + self, + item_data: dict[str, Any], + tool_name: str, + bridge_config: Any = None, + existing_proposals: dict[str, ChangeProposal] | None = None, ) -> ChangeProposal | None: """ Import backlog item as OpenSpec change proposal (reusable pattern). @@ -370,6 +505,7 @@ def import_backlog_item_as_proposal( item_data: Backlog item data (tool-specific format) tool_name: Tool identifier (e.g., "github", "ado", "jira", "linear") bridge_config: Optional bridge configuration (for cross-repo support) + existing_proposals: Existing proposals used for collision-safe and idempotent imported IDs Returns: ChangeProposal instance if successful, None if data is invalid @@ -383,22 +519,16 @@ def import_backlog_item_as_proposal( and map_backlog_status_to_openspec(). """ try: - # Extract change proposal data (tool-specific parsing) proposal_data = self.extract_change_proposal_data(item_data) - # Get status from extracted data or map from backlog item if "status" in proposal_data: openspec_status = proposal_data["status"] else: - # Map backlog status to OpenSpec status backlog_status = item_data.get("state") or item_data.get("status") or "open" openspec_status = self.map_backlog_status_to_openspec(backlog_status) - # Create source tracking source_tracking = self.create_source_tracking(item_data, tool_name, bridge_config) - - # Create change proposal - change_id = proposal_data.get("change_id") or item_data.get("id") or item_data.get("number") or "unknown" + change_id = self._normalize_imported_change_id(proposal_data, item_data, tool_name, existing_proposals) return ChangeProposal( name=change_id, title=proposal_data.get("title", "Untitled Change Proposal"), diff --git a/src/specfact_cli/adapters/github.py b/src/specfact_cli/adapters/github.py index 9962b9f3..873e45ef 100644 --- a/src/specfact_cli/adapters/github.py +++ b/src/specfact_cli/adapters/github.py @@ -265,7 +265,7 @@ def extract_change_proposal_data(self, item_data: dict[str, Any]) -> dict[str, A Change ID extraction priority: 1. Body footer (legacy format): *OpenSpec Change Proposal: `id`* 2. Comments (new format): **Change ID**: `id` in OpenSpec Change Proposal Reference comment - 3. Issue number (fallback) + 3. Issue number (fallback, normalized during shared proposal import) """ if not isinstance(item_data, dict): msg = "GitHub issue data must be dict" @@ -554,7 +554,15 @@ def import_artifact( pass # Path operations will respect external_base_path in OpenSpec adapter # Import GitHub issue as change proposal using backlog adapter pattern - proposal = self.import_backlog_item_as_proposal(artifact_path, "github", bridge_config) + existing_proposals = ( + dict(project_bundle.change_tracking.proposals) if getattr(project_bundle, "change_tracking", None) else {} + ) + proposal = self.import_backlog_item_as_proposal( + artifact_path, + "github", + bridge_config, + existing_proposals=existing_proposals, + ) if not proposal: msg = "Failed to import GitHub issue as change proposal" diff --git a/tests/integration/sync/test_ado_backlog_sync.py b/tests/integration/sync/test_ado_backlog_sync.py index 128b71fc..9bdc9c28 100644 --- a/tests/integration/sync/test_ado_backlog_sync.py +++ b/tests/integration/sync/test_ado_backlog_sync.py @@ -124,8 +124,8 @@ def test_ado_to_openspec_import( project_bundle=project_bundle, ) - assert "123" in project_bundle.change_tracking.proposals - proposal = project_bundle.change_tracking.proposals["123"] + assert "add-feature-x" in project_bundle.change_tracking.proposals + proposal = project_bundle.change_tracking.proposals["add-feature-x"] assert proposal.title == "Add Feature X" assert proposal.status == "proposed" assert proposal.source_tracking is not None diff --git a/tests/integration/sync/test_backlog_sync.py b/tests/integration/sync/test_backlog_sync.py index fcd4257e..b4560cb6 100644 --- a/tests/integration/sync/test_backlog_sync.py +++ b/tests/integration/sync/test_backlog_sync.py @@ -100,8 +100,8 @@ def test_github_to_openspec_import(self, github_adapter: GitHubAdapter, tmp_path project_bundle=project_bundle, ) - assert "123" in project_bundle.change_tracking.proposals - proposal = project_bundle.change_tracking.proposals["123"] + assert "add-feature-x" in project_bundle.change_tracking.proposals + proposal = project_bundle.change_tracking.proposals["add-feature-x"] assert proposal.title == "Add Feature X" assert proposal.status == "proposed" assert proposal.source_tracking is not None diff --git a/tests/integration/sync/test_bridge_sync_import.py b/tests/integration/sync/test_bridge_sync_import.py new file mode 100644 index 00000000..bdc6555f --- /dev/null +++ b/tests/integration/sync/test_bridge_sync_import.py @@ -0,0 +1,66 @@ +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from beartype import beartype + +from specfact_cli.adapters.ado import AdoAdapter +from specfact_cli.models.bridge import BridgeConfig +from specfact_cli.models.plan import Product +from specfact_cli.models.project import BundleManifest, ProjectBundle +from specfact_cli.sync.bridge_sync import BridgeSync +from specfact_cli.utils.bundle_loader import save_project_bundle + + +def _create_sample_bundle(base_path: Path, bundle_name: str = "demo-bundle") -> Path: + projects_dir = base_path / ".specfact" / "projects" + projects_dir.mkdir(parents=True, exist_ok=True) + + bundle_dir = projects_dir / bundle_name + manifest = BundleManifest(schema_metadata=None, project_metadata=None) + bundle = ProjectBundle(manifest=manifest, bundle_name=bundle_name, product=Product(themes=["Testing"])) + save_project_bundle(bundle, bundle_dir, atomic=True) + return bundle_dir + + +class TestBridgeSyncImport: + @beartype + def test_import_selected_ado_backlog_item_uses_native_payload_for_import(self, tmp_path: Path) -> None: + _create_sample_bundle(tmp_path) + sync = BridgeSync(tmp_path, bridge_config=BridgeConfig.preset_openspec()) + adapter = AdoAdapter(org="test-org", project="test-project", api_token="test-token") + + work_item_payload = { + "id": 123, + "fields": { + "System.Title": "Add Feature X", + "System.Description": "## Why\n\nNeeded\n\n## What Changes\n\nImplement", + "System.State": "New", + "System.CreatedDate": "2025-01-01T10:00:00Z", + "System.WorkItemType": "User Story", + }, + "_links": { + "html": {"href": "https://dev.azure.com/test-org/test-project/_workitems/edit/123"}, + }, + } + mock_response = MagicMock() + mock_response.json.return_value = work_item_payload + mock_response.raise_for_status = MagicMock() + + with ( + patch.object(adapter, "_ado_get", return_value=mock_response), + patch.object(adapter, "generate_bridge_config", return_value=BridgeConfig.preset_ado()), + patch.object(adapter, "_get_work_item_comments", return_value=[]), + patch("specfact_cli.sync.bridge_sync.AdapterRegistry.get_adapter", return_value=adapter), + patch.object(sync, "_write_openspec_change_from_proposal", return_value=[]), + ): + result = sync.import_backlog_items_to_bundle( + adapter_type="ado", + bundle_name="demo-bundle", + backlog_items=["123"], + ) + + assert result.success is True + assert result.errors == [] + assert len(result.operations) == 1 diff --git a/tests/unit/adapters/test_ado.py b/tests/unit/adapters/test_ado.py index 917bc891..47c56f28 100644 --- a/tests/unit/adapters/test_ado.py +++ b/tests/unit/adapters/test_ado.py @@ -16,6 +16,8 @@ from specfact_cli.adapters.ado import AdoAdapter from specfact_cli.models.bridge import AdapterType, BridgeConfig +from specfact_cli.models.change import ChangeProposal, ChangeTracking +from specfact_cli.models.source_tracking import SourceTracking @pytest.fixture @@ -362,8 +364,8 @@ def test_import_artifact_ado_work_item(self, ado_adapter: AdoAdapter, tmp_path: project_bundle=project_bundle, ) - assert "123" in project_bundle.change_tracking.proposals - proposal = project_bundle.change_tracking.proposals["123"] + assert "add-feature-x" in project_bundle.change_tracking.proposals + proposal = project_bundle.change_tracking.proposals["add-feature-x"] assert proposal.title == "Add Feature X" assert proposal.status == "proposed" assert proposal.source_tracking is not None @@ -809,3 +811,157 @@ def test_verify_branch_exists(self, ado_adapter: AdoAdapter, tmp_path: Path) -> assert ado_adapter._verify_branch_exists("feature/test", tmp_path) is True assert ado_adapter._verify_branch_exists("nonexistent", tmp_path) is False + + @beartype + @patch.object(AdoAdapter, "_ado_get") + def test_fetch_backlog_item_preserves_native_payload( # pylint: disable=redefined-outer-name + self, + mock_ado_get: MagicMock, + ado_adapter: AdoAdapter, + ) -> None: + """Selective fetch should keep the native ADO payload for proposal import.""" + work_item_data = { + "id": 123, + "fields": { + "System.Title": "Add Feature X", + "System.Description": "

Implement feature X

", + "System.State": "New", + }, + "_links": { + "html": {"href": "https://dev.azure.com/test-org/test-project/_workitems/edit/123"}, + }, + } + mock_response = MagicMock() + mock_response.json.return_value = work_item_data + mock_response.raise_for_status = MagicMock() + mock_ado_get.return_value = mock_response + + result = ado_adapter.fetch_backlog_item("123") + + assert result["id"] == 123 + assert result["fields"]["System.Title"] == "Add Feature X" + assert result["title"] == "Add Feature X" + assert result["state"] == "New" + assert result["description"] == "

Implement feature X

" + + @beartype + @patch.object(AdoAdapter, "_get_work_item_comments", return_value=[]) + def test_import_artifact_ado_work_item_collision_uses_source_suffix( # pylint: disable=redefined-outer-name + self, + _mock_get_comments: MagicMock, + ado_adapter: AdoAdapter, + tmp_path: Path, + ) -> None: + """Duplicate imported slugs should keep the title and append the source ID.""" + project_bundle = MagicMock() + project_bundle.change_tracking = ChangeTracking( + proposals={ + "add-feature-x": ChangeProposal( + name="add-feature-x", + title="Existing Feature X", + description="Existing", + rationale="Existing rationale", + timeline=None, + owner=None, + status="proposed", + created_at="2025-01-01T10:00:00+00:00", + applied_at=None, + archived_at=None, + source_tracking=SourceTracking(tool="ado"), + ) + } + ) + project_bundle.bundle_dir = tmp_path + + work_item_data = { + "id": 123, + "fields": { + "System.Title": "Add Feature X", + "System.Description": "## Why\n\nNeeded\n\n## What Changes\n\nImplement", + "System.State": "New", + "System.CreatedDate": "2025-01-01T10:00:00Z", + "System.WorkItemType": "User Story", + }, + "_links": { + "html": {"href": "https://dev.azure.com/test-org/test-project/_workitems/edit/123"}, + }, + } + + ado_adapter.import_artifact( + artifact_key="ado_work_item", + artifact_path=work_item_data, + project_bundle=project_bundle, + ) + + assert "add-feature-x-123" in project_bundle.change_tracking.proposals + proposal = project_bundle.change_tracking.proposals["add-feature-x-123"] + assert proposal.title == "Add Feature X" + assert proposal.source_tracking is not None + assert proposal.source_tracking.source_metadata["backlog_entries"][0]["source_id"] == "123" + + @beartype + @patch.object(AdoAdapter, "_get_work_item_comments", return_value=[]) + def test_import_artifact_ado_work_item_reimport_keeps_original_slug( + self, + _mock_get_comments: MagicMock, + ado_adapter: AdoAdapter, + tmp_path: Path, + ) -> None: + """Re-importing the same work item should keep the original proposal name.""" + project_bundle = MagicMock() + project_bundle.change_tracking = ChangeTracking( + proposals={ + "add-feature-x": ChangeProposal( + name="add-feature-x", + title="Add Feature X", + description="Existing", + rationale="Existing rationale", + timeline=None, + owner=None, + status="proposed", + created_at="2025-01-01T10:00:00+00:00", + applied_at=None, + archived_at=None, + source_tracking=SourceTracking( + tool="ado", + source_metadata={ + "source_id": 123, + "source_url": "https://dev.azure.com/test-org/test-project/_workitems/edit/123", + "source_type": "ado", + "backlog_entries": [ + { + "source_id": "123", + "source_url": "https://dev.azure.com/test-org/test-project/_workitems/edit/123", + "source_type": "ado", + } + ], + }, + ), + ) + } + ) + project_bundle.bundle_dir = tmp_path + + work_item_data = { + "id": 123, + "fields": { + "System.Title": "Add Feature X", + "System.Description": "## Why\n\nNeeded\n\n## What Changes\n\nImplement", + "System.State": "New", + "System.CreatedDate": "2025-01-01T10:00:00Z", + "System.WorkItemType": "User Story", + }, + "_links": { + "html": {"href": "https://dev.azure.com/test-org/test-project/_workitems/edit/123"}, + }, + } + + ado_adapter.import_artifact( + artifact_key="ado_work_item", + artifact_path=work_item_data, + project_bundle=project_bundle, + ) + + assert "add-feature-x" in project_bundle.change_tracking.proposals + assert "add-feature-x-123" not in project_bundle.change_tracking.proposals + assert len(project_bundle.change_tracking.proposals) == 1 diff --git a/tests/unit/adapters/test_github.py b/tests/unit/adapters/test_github.py index 594cb6e1..709cdc67 100644 --- a/tests/unit/adapters/test_github.py +++ b/tests/unit/adapters/test_github.py @@ -657,8 +657,8 @@ def test_import_artifact_github_issue(self, github_adapter: GitHubAdapter, tmp_p project_bundle=project_bundle, ) - assert "123" in project_bundle.change_tracking.proposals - proposal = project_bundle.change_tracking.proposals["123"] + assert "add-feature-x" in project_bundle.change_tracking.proposals + proposal = project_bundle.change_tracking.proposals["add-feature-x"] assert proposal.title == "Add Feature X" assert proposal.status == "proposed" assert proposal.source_tracking is not None @@ -811,3 +811,30 @@ def test_create_source_tracking(self, github_adapter: GitHubAdapter) -> None: assert source_tracking.source_metadata["source_url"] == "https://github.com/test-owner/test-repo/issues/123" assert source_tracking.source_metadata["source_state"] == "open" assert len(source_tracking.source_metadata["assignees"]) == 1 + + @beartype + @patch("specfact_cli.adapters.github.requests.get") + def test_fetch_backlog_item_preserves_native_issue_payload( # pylint: disable=redefined-outer-name + self, + mock_get: MagicMock, + github_adapter: GitHubAdapter, + ) -> None: + """Similar selective fetch path should keep the native GitHub issue payload.""" + issue_data = { + "number": 123, + "title": "Add Feature X", + "body": "## Why\n\nNeeded\n\n## What Changes\n\nImplement", + "state": "open", + "html_url": "https://github.com/test-owner/test-repo/issues/123", + } + mock_response = MagicMock() + mock_response.json.return_value = issue_data + mock_response.raise_for_status = MagicMock() + mock_response.ok = True + mock_get.return_value = mock_response + + result = github_adapter.fetch_backlog_item("123") + + assert result == issue_data + assert result["title"] == "Add Feature X" + assert result["body"].startswith("## Why")