Skip to content

Staged 2c3b33832330 1780719283#692

Merged
JoeOakhartNava merged 4 commits into
mainfrom
staged-2c3b33832330-1780719283
Jun 6, 2026
Merged

Staged 2c3b33832330 1780719283#692
JoeOakhartNava merged 4 commits into
mainfrom
staged-2c3b33832330-1780719283

Conversation

@JoeOakhartNava

@JoeOakhartNava JoeOakhartNava commented Jun 6, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added deduplication logic to prevent duplicate local tickets when syncing with existing Jira keys.
  • Bug Fixes

    • Improved script compatibility across different operating systems.
  • Tests

    • Enhanced test coverage for deduplication functionality and dependency management.

Test and others added 4 commits June 5, 2026 21:31
…apply() tests (549c)

test_apply_honours_suppress_pair_drops_subsequent_inbound{,_via_computed_form}
called applier.apply() without stubbing the lazy module loaders, so apply()
loaded the real acli-integration.py whose 'from dso_reconciler.adf import ...'
is unresolvable under this file's spec_from_file_location scheme (modules live
under plugins.dso.scripts.dso_reconciler, not a bare dso_reconciler package) —
raising ModuleNotFoundError before any inbound dispatch. Mirror test_e2e_dedup_pass:
add _patch_apply_deps() stubbing _load_acli (offline client) and _load_concurrency
(no git in tmp_path). The suppress_pair LOGIC was already correct — file now 24/24.
A test-harness gap, not the inbound-dispatch defect originally suspected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…re (1577)

The snapshot differ stands down for an already-bound issue by recognising its
dso-id:<local_id> label (bug 4354) — the primary production dedup. This adds a
cheap applier-level safety net for the narrow transient where the fetched
snapshot predates the label write-back but bindings.json already holds the
binding: the differ then mis-emits an inbound CREATE that would materialise a
phantom local ticket. _apply_inbound_create now consults binding_store
(reverse-index get_local_id(target), no Jira round-trip); on a hit it writes the
mapping and returns dedup_skipped without materialising. binding_store is
threaded through _apply_typed (both apply() call sites); leaves opt in by
declaring the param, so other leaves are unaffected. RED->GREEN test added;
inbound_leaf_bodies 25/25, differ/dedup/comment-bootstrap suites green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…7af)

e2e_field_validation_probe.sh run_reconciler used mktemp -t recon-probe.XXXXXX.log;
the -t flag has divergent macOS/GNU semantics and is prohibited by CLAUDE.md
rule:mktemp-tmp. Switch to the template form mktemp "/tmp/recon-probe.XXXXXX.log"
(production/orchestrator context — no per-test TMPDIR isolation contract).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
staged: fix/reconciler-followups-20260606 -> staged-2c3b33832330-1780719283
Copilot AI review requested due to automatic review settings June 6, 2026 05:11
@JoeOakhartNava JoeOakhartNava merged commit b4c162a into main Jun 6, 2026
19 of 20 checks passed
@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes.

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dc8bc85f-06dd-420a-9671-1358fe87eaf5

📥 Commits

Reviewing files that changed from the base of the PR and between 70e1102 and ca015ac.

📒 Files selected for processing (3)
  • plugins/dso/scripts/dso_reconciler/applier.py
  • plugins/dso/scripts/dso_reconciler/e2e_field_validation_probe.sh
  • tests/unit/dso_reconciler/test_inbound_leaf_bodies.py

Walkthrough

This PR extends the DSO reconciler's applier to deduplicate inbound Jira CREATE mutations by consulting a binding store before materializing local tickets. The typed dispatcher was enhanced to inspect handler signatures and selectively pass context parameters, and the deduplication behavior is validated by new tests and regression coverage.

Changes

Inbound CREATE deduplication via binding_store

Layer / File(s) Summary
Inbound CREATE dedup guard and signature
plugins/dso/scripts/dso_reconciler/applier.py
Inbound CREATE leaf accepts optional binding_store parameter. When the store indicates a Jira key is already bound, the leaf writes the mapping to bridge_state/mapping.json atomically and returns dedup_skipped instead of materializing a duplicate ticket.
Dispatcher signature inspection and binding_store threading
plugins/dso/scripts/dso_reconciler/applier.py
_apply_typed now inspects handler signatures to determine if they accept binding_store and/or repo_root (including **kwargs support), then passes only supported arguments. Both the typed-single-mutation apply() path and the inbound typed dispatch loop now forward binding_store to the dispatcher.
Test infrastructure and dedup regression coverage
tests/unit/dso_reconciler/test_inbound_leaf_bodies.py
Introduces _patch_apply_deps() to stub applier dependency loaders (_load_acli, _load_concurrency) for offline test execution. Two existing suppress-pair tests updated to use the stub. New regression test verifies inbound CREATE dedupes pre-bound keys, writes the mapping atomically, and does not create phantom ticket directories; also confirms unbound keys still materialize.

Shell test infrastructure mktemp fix

Layer / File(s) Summary
mktemp platform-portable fix
plugins/dso/scripts/dso_reconciler/e2e_field_validation_probe.sh
run_reconciler() now allocates the reconciler log side-car using mktemp /tmp/recon-probe.XXXXXX.log with comments on macOS/GNU -t flag semantics, replacing the previous -t form.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch staged-2c3b33832330-1780719283

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a defense-in-depth deduplication guard for inbound Jira CREATE mutations when the Jira key is already bound in bindings.json, preventing accidental duplicate local ticket materialization during a narrow snapshot/label write-back transient. It also updates unit tests to run applier.apply() in offline/all-inbound contexts by stubbing lazy loaders, and improves mktemp portability in an e2e probe script.

Changes:

  • Plumb binding_store through typed-mutation dispatch and add an inbound-create early-exit that records mapping.json and skips local materialization when the Jira key is already bound.
  • Add unit coverage for the bound-key inbound CREATE dedup behavior and patch unit tests to stub _load_acli() / _load_concurrency() for offline typed-inbound apply runs.
  • Replace mktemp -t usage with an explicit /tmp/...XXXXXX... template to avoid macOS/GNU semantic divergence.

Security overlay: Not warranted by this diff (no new auth/trust-boundary changes; mostly local state writes).
Performance overlay: Not warranted by this diff (adds a single local binding-store lookup and an atomic mapping write on an early-return path).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/unit/dso_reconciler/test_inbound_leaf_bodies.py Adds offline stubs for apply() dependencies and new test asserting inbound CREATE dedups when binding exists.
plugins/dso/scripts/dso_reconciler/e2e_field_validation_probe.sh Updates mktemp invocation to a portable template form.
plugins/dso/scripts/dso_reconciler/applier.py Adds binding-store-aware inbound CREATE dedup guard and threads binding_store through typed dispatch/apply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants