Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions planfile/sync/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()}
Expand All @@ -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.<integration>.id)
if sections:
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions project/TICKETS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) | - | - | - | - | - |
<!-- AUTO:TICKET_INDEX:END -->
25 changes: 25 additions & 0 deletions project/ticket-254/README.md
Original file line number Diff line number Diff line change
@@ -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.
75 changes: 75 additions & 0 deletions project/ticket-254/intent.json
Original file line number Diff line number Diff line change
@@ -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."
}
]
}
}
80 changes: 80 additions & 0 deletions tests/test_sync_selected_provider_reads.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading