diff --git a/planfile/sync/operations.py b/planfile/sync/operations.py index 3daafa6..13b288f 100644 --- a/planfile/sync/operations.py +++ b/planfile/sync/operations.py @@ -466,8 +466,10 @@ def _resolve_selected_remote_ids( sections: dict[str, dict] | None, sync_state, integration_name: str, + *, + strict_local: bool = False, ) -> set[str] | None: - """Pre-resolve candidate remote IDs from ticket filters to avoid ambiguous lookups.""" + """Resolve filters, optionally giving local identities precedence over aliases.""" if not ticket_ids: return None normalized = {str(value).strip() for value in ticket_ids if str(value).strip()} @@ -476,20 +478,24 @@ def _resolve_selected_remote_ids( remote_ids: set[str] = set() for tid in normalized: - clean = tid.lstrip("#") - if clean.isdigit(): - remote_ids.add(clean) - remote_ids.add(tid) - remote_ids.add(clean) + # Explicit GitHub references always name remote issues. A bare numeric + # selector can instead be a real local ID, mapped to a different issue. + if strict_local and "#" in tid: + remote_ids.add(tid) + continue + mapped_ids: set[str] = set() + local_found = False # Look up in sync_state (local -> remote mapping) if sync_state is not None and hasattr(sync_state, "get_remote_id"): try: mapped_remote = sync_state.get_remote_id(tid) if mapped_remote: - remote_ids.add(str(mapped_remote).strip()) + mapped_ids.add(str(mapped_remote).strip()) except Exception as exc: logger.debug("sync_state remote id lookup failed for ticket %s: %s", tid, exc) + if strict_local: + raise # Look up in sections (section tickets -> sync..id) if sections: @@ -498,11 +504,22 @@ def _resolve_selected_remote_ids( continue ticket = (section.get("tickets") or {}).get(tid) if isinstance(ticket, dict): + local_found = True ref = (ticket.get("sync") or {}).get(integration_name) or {} if isinstance(ref, dict): rid = ref.get("id") if rid is not None and str(rid).strip(): - remote_ids.add(str(rid).strip()) + mapped_ids.add(str(rid).strip()) + + if strict_local: + if len(mapped_ids) > 1: + raise ValueError(f"conflicting selected remote mapping for local ticket {tid}") + if local_found or mapped_ids: + remote_ids.update(mapped_ids) + continue + clean = tid.lstrip("#") + remote_ids.update((tid, clean)) + remote_ids.update(mapped_ids) return remote_ids @@ -649,7 +666,8 @@ def sync_from_external( if local_id in normalized_ticket_ids: _validate_ticket_binding(ticket, integration_name, backend) selected_remote_ids = _resolve_selected_remote_ids( - normalized_ticket_ids, lookup_sections, sync_state, integration_name + normalized_ticket_ids, lookup_sections, sync_state, integration_name, + strict_local=integration_name == "github", ) external_tickets = _fetch_external_tickets( backend, integration_name, remote_labels, selected_remote_ids diff --git a/project/TICKETS.md b/project/TICKETS.md index 63d4ea8..3579fe2 100644 --- a/project/TICKETS.md +++ b/project/TICKETS.md @@ -156,4 +156,5 @@ This file indexes governance tickets without taking ownership of | **ticket-251** | [`README.md`](./ticket-251/README.md) | - | - | - | - | - | | **ticket-252** | [`README.md`](./ticket-252/README.md) | - | - | - | - | - | | **ticket-253** | [`README.md`](./ticket-253/README.md) | - | - | - | - | - | +| **ticket-254** | [`README.md`](./ticket-254/README.md) | - | - | - | - | - | diff --git a/project/ticket-254/README.md b/project/ticket-254/README.md new file mode 100644 index 0000000..2b4f138 --- /dev/null +++ b/project/ticket-254/README.md @@ -0,0 +1,25 @@ +# Ticket 254: Disambiguate numeric local selectors + +- **ID**: ticket-254 +- **Owner**: agent:codex +- **Status**: IN_PROGRESS +- **Workflow state**: PUBLICATION +- **Created**: 2026-10-28 + +## Goal and scope + +SESSION_EXECUTION_AUTHORIZATION: continue user-requested Heyasa autonomy repair, tests and independent publication. Follow up confirmed selector collision after semcod/planfile PR312, within existing owner issue256 / local PLF-17304. No duplicate intake issue. + +The public SDK accepts a numeric local ticket ID553 mapped to issue999. Selecting that local ID currently also reads issue553 and closes an unrelated mapped ticket. Resolve one meaning per selector and reject conflicting selected local mappings before remote reads. Explicit #553 / fixture/repo#553 remain remote selectors; retain unaffected provider, CAS, execution, sprint and label behavior. + +## Acceptance criteria + +- [x] AC-01: Numeric local selector changes only its mapped issue; explicit remote selectors remain available and unrelated snapshots stay identical. +- [x] AC-02: Conflicting local/ledger mappings fail before provider I/O; selected sync compatibility and governance pass. +- [ ] AC-03: Required current-head/base checks and independent publication succeed. + +Confirmed failing probe: external planfile256-20261028/numeric-selector-probe.json. PR312 was independently merged at959224a9cc95fe0a73a0d26b285c6b17b7c55561 before the additional probe was evaluated. Its own lease has been retired by exact owner CAS after trusted merge readback; this material correction uses a separate native allocation. + +## Validation + +Six regressions failed on the merged PR312 source; eight control cases passed. After correction all76selected-read cases and683affected sync tests pass (199.17s for the full affected suite). Ruff and native governance pass. Required exact-head/base protected verification and independent publication remain pending. No provider credentials, runtime dependency or protected policy changed. diff --git a/project/ticket-254/intent.json b/project/ticket-254/intent.json new file mode 100644 index 0000000..b1f2aac --- /dev/null +++ b/project/ticket-254/intent.json @@ -0,0 +1,75 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-254", + "summary": "Disambiguate local numeric selectors from GitHub issue IDs", + "workstream": "application", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "regression" + }, + "allowedPaths": [ + "project/ticket-254/**", + "TODO.md", + "project/TICKETS.md", + "planfile/sync/operations.py", + "tests/test_sync_selected_provider_reads.py" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "outcome": "Prevent a selected numeric local ticket ID from reading or updating an unrelated GitHub issue; reject conflicting local remote mappings before provider I/O.", + "targetBranch": "main", + "acceptedBaseSha": "959224a9cc95fe0a73a0d26b285c6b17b7c55561", + "complexity": "S", + "estimatedMinutes": 35, + "budgets": { + "maxImplementationFiles": 2, + "maxAffectedComponents": 1, + "maxPublicInterfaceChanges": 0, + "maxRuntimeDependencies": 0 + }, + "nonGoals": [ + "No provider credential, lease policy, runtime promotion or whole-operation deadline changes." + ], + "architecture": { + "status": "accepted", + "decision": "Resolve local selectors before remote aliases; preserve explicit remote references and non-GitHub compatibility, fail closed on conflicting selected mappings.", + "responsibilityChanges": false, + "components": [ + { + "name": "inbound-selection", + "paths": [ + "planfile/sync/operations.py", + "tests/test_sync_selected_provider_reads.py" + ] + } + ], + "interfaceChanges": [], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Protected revert of the correction without changing authoritative ticket data." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-01", + "commands": [ + "python -m pytest -q tests/test_sync_selected_provider_reads.py", + "python -m pytest -q tests/test_sync*.py", + "./project/governance-check.sh" + ], + "evidence": "Numeric local selector collision regression must fail on PR312 and pass after correction, preserving unrelated snapshots and explicit remote selections." + } + ] + } +} diff --git a/tests/test_sync_selected_provider_reads.py b/tests/test_sync_selected_provider_reads.py index 8fc86df..37ae866 100644 --- a/tests/test_sync_selected_provider_reads.py +++ b/tests/test_sync_selected_provider_reads.py @@ -323,3 +323,83 @@ def test_unknown_sprint_does_not_read_selected_issue(sample): before = unchanged(store) pull(sample, sprint_ids=["missing-sprint"]) assert not calls and unchanged(store) == before + + +@pytest.mark.parametrize( + "selectors,expected_reads,expected_local", + [ + (["554"], [999], {"554"}), + (["#554"], [554], {"PLF-002"}), + (["fixture/repo#554"], [554], {"PLF-002"}), + (["554", "#554"], [554, 999], {"554", "PLF-002"}), + ], +) +def test_numeric_local_selector_has_one_meaning(sample, selectors, expected_reads, expected_local): + store, _, calls, _ = sample + store.create_ticket( + Ticket( + id="554", + name="numeric local", + status="open", + sprint="backlog", + integration=["github"], + sync={"github": {"id": "999", "repository": "fixture/repo"}}, + ) + ) + before = { + tid: store.get_ticket(tid).model_dump(mode="json") for tid in ["PLF-001", "PLF-002", "554"] + } + pull(sample, selectors) + assert calls == [("get", number) for number in expected_reads] + for tid, snapshot in before.items(): + current = store.get_ticket(tid) + if tid in expected_local: + assert current.status == "done" + else: + assert current.model_dump(mode="json") == snapshot + + +def test_unmapped_numeric_local_selector_does_not_select_a_remote_issue(sample): + store, _, calls, _ = sample + store.create_ticket(Ticket(id="554", name="unmapped local", status="open", sprint="backlog")) + before = unchanged(store) + numeric_before = store.get_ticket("554").model_dump(mode="json") + pull(sample, ["554"]) + assert not calls + assert unchanged(store) == before + assert store.get_ticket("554").model_dump(mode="json") == numeric_before + + +@pytest.mark.parametrize("state_id", ["553", "554"]) +def test_selected_local_mapping_conflict_is_rejected_before_fetch(sample, state_id): + from planfile.sync.state import SyncState + + store, _, calls, _ = sample + state = SyncState(store.base_dir, "github", repository="fixture/repo") + state.save_sync({"PLF-001": state_id}) + before = unchanged(store) + state_before = state.state_file.read_bytes() + if state_id == "553": + pull(sample) + assert calls == [("get", 553)] + assert store.get_ticket("PLF-001").status == "done" + else: + with pytest.raises(ValueError, match="conflicting.*mapping"): + pull(sample) + assert not calls and unchanged(store) == before + assert state.state_file.read_bytes() == state_before + + +def test_selected_state_lookup_failure_cannot_fall_back_to_a_remote_alias(sample, monkeypatch): + from planfile.sync.state import SyncState + + store, _, calls, _ = sample + before = unchanged(store) + + def failed_lookup(self, local_id): + raise RuntimeError("mapping store unavailable") + + monkeypatch.setattr(SyncState, "get_remote_id", failed_lookup) + with pytest.raises(RuntimeError, match="mapping store unavailable"): + pull(sample) + assert not calls and unchanged(store) == before