feat(sync): ADR-023 phase 4a — conflict resolution, v2 API and MCP tools#593
Merged
Conversation
…, v2 API, MCP tools) Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…ates (ADR-023 §6) Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…R-023 §6) Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…(ADR-023 §5) Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…cts (ADR-023 §6) Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
… docs Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…, license header Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The Phase 4a implementation plan served its purpose; durable decisions live in docs/adr/ADR-023-jira-worklog-bidirectional-sync.md and docs/worklog-sync.md. Spec-Cleanup: captured-in-ADR-023 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #593 +/- ##
============================================
+ Coverage 80.67% 80.93% +0.26%
- Complexity 4068 4203 +135
============================================
Files 319 336 +17
Lines 11228 11618 +390
============================================
+ Hits 9058 9403 +345
- Misses 2170 2215 +45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Implements ADR-023 Phase 4a by exposing the Jira worklog bidirectional sync engine via (1) conflict/orphan resolution, (2) v2 REST endpoints guarded by sync:read/sync:write, (3) MCP tools providing the same programmable surface, plus (4) admin configuration of sync user/default activity. This rounds out the non-UI “programmable surfaces” layer that Phase 4b (SPA) will build on.
Changes:
- Added conflict/orphan resolution flow (
ConflictResolutionService) including local force-push and remote live-pull/delete behavior. - Introduced v2 API actions + request/response DTOs and MCP tools for triggering runs, reading run reports, listing conflicts, and resolving conflicts.
- Extended repositories and admin ticket-system save handling to support sync-surface queries and sync configuration.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Service/Sync/WorklogWriteServiceTest.php | Adds coverage for forcePush() behavior (skip lease compare, empty ticket skip). |
| tests/Service/Sync/ConflictResolutionServiceTest.php | New unit tests covering conflict resolution paths (local/remote, orphaned, failure). |
| tests/Repository/SyncSurfaceQueriesTest.php | Integration tests for run listing and parked-state repository queries. |
| tests/Mcp/WorklogSyncToolsTest.php | Integration tests for new MCP sync tools and their auth/ownership semantics. |
| tests/Mcp/McpToolsTest.php | Extends global MCP tool shape guard to include new sync tools + fixtures. |
| tests/Dto/SyncResponseDtosTest.php | Unit tests for v2/MCP response DTO JSON shapes (run/run item/conflict). |
| tests/Controller/SaveTicketSystemPreserveSecretsTest.php | Adds tests ensuring sync-user/default-activity config is persisted/cleared/validated. |
| tests/Controller/Api/V2/WorklogSyncRunActionsTest.php | Functional tests for POST /runs and GET /runs/{id} including auth matrix and validation. |
| tests/Controller/Api/V2/WorklogSyncConflictActionsTest.php | Functional tests for conflict listing and conflict resolution endpoints. |
| src/ValueObject/Sync/ResolutionResult.php | New value object for conflict-resolution outcomes. |
| src/Service/Sync/WorklogWriteService.php | Adds forcePush() to support conflict-resolution “local wins” writes without lease compare. |
| src/Service/Sync/SyncRunRequestMapper.php | Shared parsing/dispatch for REST+MCP run triggers (range, since, service dispatch). |
| src/Service/Sync/SyncRunAuthorization.php | Centralizes ADR-023 §6 authorization matrix for runs/conflicts (shared by REST+MCP). |
| src/Service/Sync/ConflictResolutionService.php | Implements conflict/orphan resolution workflow, including live remote refetch and day recalculation. |
| src/Repository/WorklogSyncStateRepository.php | Adds findParked() and findParkedById() for conflict/orphan surfacing. |
| src/Repository/SyncRunRepository.php | Adds findLatest() for run listing (ordered, optional ticket-system filter). |
| src/Mcp/Tool/SyncJiraWorklogsTool.php | MCP entrypoint to trigger verify/import/sync runs with validation and auth checks. |
| src/Mcp/Tool/ResolveSyncConflictTool.php | MCP tool to resolve a parked conflict state with winner selection. |
| src/Mcp/Tool/ListSyncConflictsTool.php | MCP tool to list parked conflicts/orphans (admin filter vs forced self). |
| src/Mcp/Tool/GetSyncRunTool.php | MCP tool to fetch a run report with ownership/admin enforcement. |
| src/Mcp/ScopeGuard.php | Adds isAdmin() helper for tools needing “narrow behavior” vs hard-fail. |
| src/Dto/WorklogSyncRunDto.php | Request DTO for v2 run creation (and shared validation for MCP mapper). |
| src/Dto/TicketSystemSaveDto.php | Extends admin save DTO with syncUserId/syncDefaultActivityId (manual relation resolution). |
| src/Dto/Response/SyncRunItemDto.php | Response DTO for run items (kind/reason/payload, etc.). |
| src/Dto/Response/SyncRunDto.php | Response DTO for sync runs (optionally includes items). |
| src/Dto/Response/SyncConflictDto.php | Response DTO for parked conflicts/orphans (entry + base + remote snapshot). |
| src/Dto/ResolveConflictDto.php | Request DTO for resolve endpoint (winner). |
| src/Controller/Api/V2/ResolveWorklogSyncConflictAction.php | Adds v2 endpoint to resolve a parked conflict/orphan. |
| src/Controller/Api/V2/ListWorklogSyncConflictsAction.php | Adds v2 endpoint to list parked conflicts/orphans (admin filter). |
| src/Controller/Api/V2/GetWorklogSyncRunAction.php | Adds v2 endpoint to fetch a run report (ownership/admin). |
| src/Controller/Api/V2/CreateWorklogSyncRunAction.php | Adds v2 endpoint to trigger verify/import/sync runs with shared mapper + auth matrix. |
| src/Controller/Admin/SaveTicketSystemAction.php | Accepts/persists sync-user/default-activity config with 422 on unknown IDs. |
| docs/worklog-sync.md | Documents v2 API + MCP surfaces, scopes, and examples. |
| docs/adr/README.md | Updates ADR-023 progress note to Phase 4a complete. |
| docs/adr/ADR-023-jira-worklog-bidirectional-sync.md | Adds Phase 4a implementation notes and updates ADR status line. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Phase 4a of the Jira Worklog Import and Bidirectional Sync (ADR-023), introducing v2 REST API endpoints and MCP tools for triggering sync runs, listing parked conflicts, and resolving conflicts. It also includes the ConflictResolutionService to handle local and remote conflict resolution, along with comprehensive test coverage. The reviewer suggested trimming the from and to date strings in SyncRunRequestMapper to prevent whitespace-only inputs from causing errors when instantiated as DateTimeImmutable.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
… dedupe sync test fixtures - sql/full.sql gains the three ADR-023 sync tables and the seven missing doctrine_migration_versions seed rows — CI provisions its integration database from this file, which is why every SyncSurfaceQueriesTest case failed with 'Table sync_runs doesn't exist' while local runs (real migrations) passed. - ConflictResolutionService no longer reports winner=local success when forcePush wrote but the base refresh no-oped (post-write 404 race or missing worklog id); the conflict stays parked with an explicit reason. - The three sync surface test classes share CreatesWorklogSyncFixtures instead of three copies of the same setUp and builders (SonarCloud new-code duplication gate). Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…s cleanly on EM closure Found by importing real Jira worklogs: a worklog comment longer than the entries.description varchar(255) column threw 'Data too long' under strict SQL mode, which closed the EntityManager mid-run — and executeRun's catch block then compounded it with an opaque EntityManagerClosed from the retry flush, hiding the real cause. - ImportWorklogsService::createEntry and EntryPullApplier cap the description at Entry::DESCRIPTION_MAX_LENGTH (255) before persisting. - AbstractSyncRunService::executeRun detects a closed manager after a failed run body and rethrows with the original error message instead of the useless EntityManagerClosed. Pure-mock unit tests never hit a real column constraint; only a run against live Jira surfaced it. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…ack in_sync Found by re-importing real Jira worklogs (originally pushed by prod TT, so their comments already carried the '#<id>: <activity>:' wrapper): imported entries came back local_dirty because the projector re-wrapped the already- prefixed comment and compared it against the raw remote, so a later sync or save would push TT's double-prefixed comment over the original. Reconciliation now compares the bare description on both sides — the projector emits entry.description, the normalizer decodes the remote comment — which aligns with the ADR (activity is a TT-only field that never participates in the diff) and makes TT-pushed and Jira-native worklogs compare cleanly. Import seeds the sync base from the entry's own projection so truncated descriptions also stay in_sync. Verified against live Jira: 12 imported worklogs read back in_sync on the next verify (previously all 12 were local_dirty). Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…mpty Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
9 tasks
CybotTM
added a commit
that referenced
this pull request
Jul 10, 2026
## Description Phase 4b of [ADR-023](docs/adr/ADR-023-jira-worklog-bidirectional-sync.md) — the final phase: the SolidJS worklog-sync UI, driving the Phase 4a v2 endpoints. **This completes ADR-023 (Phases 1–4).** Builds on #590, #591, #592, #593. ## Type of Change - [x] New feature (non-breaking change that adds functionality) ## Changes Made - **Self-service import** (Settings): ticket-system + date-range + default-activity form with a dry-run preview → execute flow, scoped to the current user. Surfaces the run summary (counters + parked items). - **Admin sync area** (ROLE_ADMIN): trigger verify/import/sync runs, browsable run history, and per-run reports — plus the embedded conflict list. - **Conflict resolution screen**: side-by-side local vs remote (from `conflict_remote`), accessible per-conflict "keep local / keep remote" buttons, resolve + refetch. - **Ticket-system admin form**: new sync-user and default-import-activity selects wired into the existing `ticketsystems` descriptor. - **Backend**: `GET /api/v2/worklog-sync/runs` list endpoint (backs the run history; the `findLatest` query existed but was unexposed), owner-scoped for non-admins under `sync:read`. - Frontend API module (`src/api/worklogSync.ts`) with hand-written DTO interfaces and query factories; shared `SyncRunSummary` component; `worklogsync_*` i18n keys added identically to all five catalogs (en/de/es/fr/ru). ## Testing - [x] Frontend: `bun run typecheck` (proves all five i18n catalogs are key-consistent), `bun run test` (full suite green), `bun run lint` - [x] Backend: the list endpoint's functional tests (19 cases incl. authorization matrix) pass on `db_unittest` - [x] Component tests assert `axe` has no violations (WCAG 2.2 AA); writes use the `postJson` + `invalidateQueries` idiom (no `useMutation`); no Tailwind utilities (hand-written CSS with the `is-<state>` convention) - [ ] e2e: a smoke spec (`e2e/worklog-sync.spec.ts`) covers admin gating + control presence; runs against the CI e2e stack (no Jira) Note: `composer test:controller` fails in a local checkout that has the `.env.test.local` dev-DB trap with accumulated sync-run rows; on a fresh `db_unittest` (and in CI) all tests pass. ## Code Quality - [x] Code follows project coding standards (SolidJS conventions per frontend/AGENTS.md) - [x] Self-review completed (per-task adversarial review during implementation) - [x] Documentation updated (ADR-023 marked fully implemented; docs/worklog-sync.md UI section) - [x] No breaking changes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Phase 4a of ADR-023: the programmable surfaces over the sync engine — conflict resolution, four v2 REST endpoints under PAT scopes, four MCP tools (the agentic worklog-hub interface), and admin configuration of the ticket-system sync fields. Builds on Phases 1–3 (#590, #591, #592). The SPA UI follows as Phase 4b.
Type of Change
Changes Made
ConflictResolutionService+WorklogWriteService::forcePush): winner=local force-pushes (recreating the worklog for orphans); winner=remote pulls the live remote (re-fetched — the stored snapshot is display material) or, for orphans, accepts the deletion by removing the local entry. Base refreshed, conflict cleared, day classes recalculated.sync:read/sync:writescopes — the ADR's proposedworklog-sync:*scopes were reconciled to the ADR-021 taxonomy, recorded in the ADR):POST /api/v2/worklog-sync/runs— trigger verify/import/sync with dry-run; authorization matrix: non-admins may verify and self-import only; sync triggers and foreign imports needROLE_ADMIN.GET /api/v2/worklog-sync/runs/{id}— run report (own runs; admins see all).GET /api/v2/worklog-sync/conflicts— parked conflicts/orphans (own entries; admins may filter by user).POST /api/v2/worklog-sync/conflicts/{id}/resolve—{winner: local|remote}(owner or admin).sync_jira_worklogs,get_sync_run,list_sync_conflicts,resolve_sync_conflict— same authorization viaScopeGuard; parsing/dispatch shared with the v2 actions throughSyncRunRequestMapperandSyncRunAuthorization(single source for the matrix).TicketSystemSaveDto+ save action acceptsyncUserId/syncDefaultActivityId(cursor stays read-only; reset viatt:sync-worklogs --since).SyncRunDto,SyncRunItemDto,SyncConflictDto), repository queries for run listing and parked states, API/MCP section indocs/worklog-sync.md.Testing
composer test:fast)composer check:all+ Rector dry-run clean)Code Quality