Repository navigation
Resolve Azure field conflicts only at reviewed revisions - #41
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThis PR introduces revision-bound Azure field conflict resolution through portable typed plans, explicit choices, fenced audited application, and immutable idempotent receipts, integrates it into the non-git CLI/SDK workflow, updates synchronization helpers and dependency floors, and adds extensive synthetic acceptance coverage and documentation. Sequence diagram for revision-bound Azure conflict resolutionsequenceDiagram
actor Agent
participant CLI as ado CLI
participant Service as AdoResolutionService
participant PM as Core SDK
participant Azure as Azure REST API
Agent->>CLI: ado resolve-plan --item id
CLI->>Service: plan(id)
Service->>PM: listAllComplete()
Service->>PM: readLocatedItem()
Service->>Azure: getWorkItems()
Azure-->>Service: hydrated item and revision
Service-->>CLI: ResolutionPlan with field digests
CLI-->>Agent: review choices
Agent->>CLI: ado resolve-apply --artifact file
CLI->>Service: apply(plan)
Service->>PM: validateResolutionPlan()
Service->>PM: mutateItemWithHistoryContext()
PM->>Service: read local document and history
Service->>Azure: getWorkItems()
Azure-->>Service: current revision and snapshot
Service->>PM: previewItemMutations()
Service->>Azure: updateWorkItem(id, expectedRevision, fields)
Azure-->>Service: updated item
Service->>PM: record receipt and immutable history
Service-->>CLI: ResolutionReceipt
Entity relationship diagram for resolution artifacts and receiptserDiagram
CORE_PLAN {
string plan_id PK
string resolution_artifact_body
}
RESOLUTION_PLAN {
string schema
string local_revision
int remote_revision
string remote_digest
string mapping_digest
}
RESOLUTION_FIELD {
string field
string base_digest
string local_digest
string remote_digest
boolean conflict
}
RESOLUTION_CHOICE {
string field
string take
}
RESOLUTION_RECEIPT {
string decision PK
string pm_id
boolean replayed
}
CORE_PLAN ||--|| RESOLUTION_PLAN : stores
RESOLUTION_PLAN ||--|{ RESOLUTION_FIELD : contains
RESOLUTION_PLAN ||--o{ RESOLUTION_CHOICE : has
RESOLUTION_PLAN ||--o| RESOLUTION_RECEIPT : produces
State diagram for an Azure resolution planstateDiagram-v2
[*] --> Planned: plan(id)
Planned --> Reviewed: add explicit local/remote choices
Reviewed --> Refused: invalid choices
Reviewed --> Refused: local digest changed
Reviewed --> Refused: Azure revision or snapshot changed
Reviewed --> Applying: apply(plan)
Applying --> Applied: revision checks pass and PATCH succeeds
Applied --> Replayed: same decision receipt found
Replayed --> [*]
Applied --> [*]
Refused --> [*]: generate fresh plan
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
unbraind
left a comment
There was a problem hiding this comment.
VERDICT: BLOCK
FINDINGS:
- [blocking] resolution.ts:252 — Azure is PATCHed before the local commit without compensation — a subsequent lease expiry or tracker write failure leaves Azure changed, pm unchanged, and no successful replay receipt.
- [blocking] resolution.ts:200 — Plans contain the raw organization URL, which
--savepersists in the tracked Plan body — configured organization details therefore leak into tracker files and history.
SUMMARY: The PR adds revision-bound conflict plans, explicit field choices, and replay receipts. Tests cover stale revisions, invalid choices, replay, and non-git usage, but omit post-PATCH local failures and organization-URL privacy assertions; tests were inspected, not executed.
Review by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator; the PR returns to its author agent for the fixes.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
unbraind
left a comment
There was a problem hiding this comment.
Orchestrator review (built-in Claude review agent, verified by the orchestrator; bots are not waited for since 2026-10-22):
VERDICT: MERGE
FINDINGS:
- none
SUMMARY: The fixes add durable, fenced resolution recovery and opaque HMAC scope references without persisting credentials in saved plans. Inspected regression tests cover lease expiry, tracker write failure, single-PATCH recovery, revision checks and Plan privacy; I did not rerun them.
Re-review of the fix commits (e12ab27..195f7eb) by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator. Both earlier blocking findings are resolved: durable intent before the Azure PATCH with fenced settlement on resume (single PATCH, lease-expiry and tracker-write-failure paths tested), and opaque HMAC scope references instead of the organization URL in saved plans. CI npm + bun 215/215, coverage 100%.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
left a comment
unbraind
left a comment
There was a problem hiding this comment.
Orchestrator review (built-in Claude review agent, verified by the orchestrator; bots are not waited for since 2026-10-22):
VERDICT: MERGE
FINDINGS:
- none
SUMMARY: The fixes add durable, fenced resolution recovery and opaque HMAC scope references without persisting credentials in saved plans. Inspected regression tests cover lease expiry, tracker write failure, single-PATCH recovery, revision checks and Plan privacy; I did not rerun them.
Re-review of the fix commits (e12ab27..195f7eb) by a read-only codex review agent (gpt-6.1-sol), posted by the orchestrator. Both earlier blocking findings are resolved: durable intent before the Azure PATCH with fenced settlement on resume (single PATCH, lease-expiry and tracker-write-failure paths tested), and opaque HMAC scope references instead of the organization URL in saved plans. CI npm + bun 215/215, coverage 100%.
An interrupted resolution could previously PATCH Azure and then lose its local lease or tracker write, leaving no resumable receipt. Saved plans also persisted organization and project configuration. This change makes reviewed conflict decisions resumable and removes identifying scope values from portable/saved artifacts.
ado resolve-planproduces version 2 artifacts with eight field digest rows, explicit choices and an opaque credential-keyed organization/project reference.--savestill uses ordinary core Plan storage. Organization URL, project name, token and Basic encoding are absent from Plan bodies and history. Version 1 artifacts refuse rather than silently retaining their raw scope.ado resolve-applynow performs two audited SDK mutations. Before Azure is called, prepare commits the canonical plan/choice ID, original local revision, Azure revision/digest, exact PATCH and expected post-PATCH revision into the target item's durable intent/history. Settlement holds a fenced item lease, sends the saved PATCH with its leadingtest /revand a unique System.History decision marker, then atomically commits the pm fields, successful receipt and intent removal. The pm CLI 2026.10.23 pin supplies expired-lease fencing.On retry, the same intent plus Azure's expected revision, decision marker and mapped result prove a successful prior PATCH. The adapter completes pm settlement without patching again. If Azure is provably unchanged, the exact saved revision-checked PATCH can be attempted. Changed local context, conflicting intents, unexpected Azure revisions/results and unreadable recovery data return typed
ado_partial_applywith the intent coordinate, expected revision and same-plan recovery instructions. No blind rollback or overwrite of a newer Azure revision occurs.Independent behavioral revert proofs: removing only the durable prepare-field write fails lease recovery 1/1; disabling applied-PATCH recognition fails tracker-write recovery 1/1; replacing only opaque scope generation with raw configuration fails saved Plan/history privacy 1/1. Removing only pending-intent retry classification also fails the real held-lock regression 1/1 (
apply_failedinstead ofado_partial_apply). All source was restored. Targeted suite and PM-linked run: 18/18, zero skipped. Targeted resolution.ts measurement: 100% V8 lines, branches and functions. Source/test typechecks, 79-declaration docstring lint, changelog generation/check and diff checks pass locally. The saturated host did not run a full local release gate.The original acceptance remains covered: stale saved local/Azure revisions refuse before prepare writes; invalid or conflicting choices refuse before REST; successful immediate and historical replay add no PATCH or history event. Actual CLI JSON/TOON and core Plan persistence work in non-git trackers with git absent from PATH. Scalar plans preserve the existing relation adapter and legacy-base behavior; revisions-feed reconciliation remains excluded.
Public-data validation uses an unchanged non-git archive of the public unbraind/pm-ado tracker at
1fe0a306e13417115e66de9b48ac2e50deaa25d1: 28 certified items, 224 field comparisons, 28 synthetic conflicts, 28 valid plans and 28 incomplete-choice refusals; version 2 plans average 2,576 JSON bytes. All overlays remain in memory, copied source fingerprints are unchanged, and Azure writes are zero. Azure response fixtures follow Work Item Tracking REST 7.1; their README cites the revision and update APIs. Live Azure permissions and custom process behavior remain unverified because no live credentials were used.CI evidence for the follow-up head: run 39939836334 passed on exact head
195f7eb3da9d6521b28c43cf9917e6447da4ce07. Bothnpm run release:checkandbun run release:checkpassed 215/215 tests in each V8 and independent Istanbul run, with zero failures or skips. All seven configured sources measured 100% lines/branches/functions and 1,228/1,228 independently instrumented statements (100%), preserving every threshold. Docstring lint documented 79 declarations across 11 files; production audit reported 0 vulnerabilities. Typechecks, package contents, changelog, publish-attestation and version-date checks passed. Node 22 and Node 26 CI jobs also passed. The existing workflow was dispatched explicitly because automatic synchronization runs were not created for the latest pushes; all evidence is bound to the pushed head above.PM evidence: pm-ado-4x3u, including both review fixes and all revert proofs. The implementation claim is released; the item is unclosed. Prior issue #33 ownership evidence remains in pm-ado-github-5532dc4b9a65-33: delivery selection is owned by pm-github, not this package's closeout configuration.
No review-thread replies, mentions, merge or package release were performed.
Summary by Sourcery
Implement resumable, revision-bound Azure field conflict resolution with privacy-preserving artifacts and durable recovery state.
New Features:
ado resolve-planandado resolve-applycommands for reviewing and applying explicit Azure field conflict decisions.Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: