fix: reconcile Linear publication recovery identities - #624
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
kmbroai
left a comment
There was a problem hiding this comment.
Critical review
Reviewed head 532a03df06cf6e92e4fae3ef12397f81d9fedab5, including comparison with #482 and the cancellation follow-up #625.
Recommendation: keep this recovery correction and use it to supersede #482. No additional blocking defect found in the reviewed paths. The important feature is not broader identity inference; it is refusing to turn contradictory mutation evidence into an authoritative success while preserving independently verified siblings.
Correctness
I ran the same actual publishScanInternal fixture on #482 and this head: an exact completed call reported SYNTH-1 in structured output and SYNTH-2 in JSON text, while the handoff chose SYNTH-1. #482 returned and recorded success. This head retained an indeterminate receipt/evidence, threw, and recorded no success. The explicit absent/conflicting/resolved distinction fixes a real problem.
The reconciliation path also separates human keys from entity UUIDs, requires corroboration before combining independent claims, and applies cross-owner claim collisions after per-finding reconciliation. Exact argument validation remains shared with payload production. Delaying terminal success progress until reconciliation is necessary; the new handoff_recorded event correctly reports persistence of evidence, not proof of a created issue.
The direct-API cancellation change deliberately lets in-flight mutations settle after the first signal so their outcomes can be recorded. That trades immediate exit for recoverability; #625 supplies the separate connected-process-tree escalation fix and should remain a small follow-up.
Simplification
Keep one pure evidence reconciliation path rather than backporting individual branches into #482's parallel maps. The current parser/reconciler split is useful. One concrete follow-up opportunity is indexing expected mutation arguments once: matchPublicationIssue currently scans all issues for exact matches, then scans again for identifier fallback on each event. Avoid adding another generic state-machine framework; the existing transition tables already express the meaningful cases.
Do not describe these checks as remote readback or retry idempotency. They validate captured mutation arguments and identity evidence; a retained uncertain handoff still requires reconciliation before another publish.
Verification
Ran the four publisher/event/integration/CLI-publication suites: 139 passed, 0 failed, plus the paired conflict probe. Linux, Bun 1.3.14 / Node 22.13.1, cached dependencies. No live Linear mutation, native Windows run, or clean package installation was performed.
faizan-oai
left a comment
There was a problem hiding this comment.
Reviewed the reconciliation and recovery changes. All 139 focused publication tests pass, and current-head CI is green. No blocking findings.
Preserve Cloud scan selection and cancellation alongside direct Linear recovery. Cover repeated Cloud cancellation with a configured Linear API key.
There was a problem hiding this comment.
Reviewed the merge integration at c64fcbd against main 1898775. Linear recovery and Cloud publication behavior are preserved, including the repeated-signal regression check. Local suite: 1,740 passed, 30 skips; typecheck and formatting passed. All 25 current-head CI jobs passed, including the three required checks. No blocking findings.
Summary
Linear publication could treat incomplete or conflicting identity evidence as
determinate. This change verifies exact mutation arguments, reconciles human
issue keys, entity UUIDs, and canonical issue URLs across connector events and
durable handoffs, and preserves uncertain outcomes for recovery instead of
recording them as published.
Changes
recovery validation, matching exact arguments before the finding and
occurrence fallback.
nested result carriers iteratively, and distinguish human issue keys from
entity UUIDs while canonicalizing equivalent Linear issue URLs.
independent evidence, and reserve recognized claims in one global normalized
identity namespace.
diagnostics. Retain handoffs, connector-event evidence, and partial receipts
for indeterminate outcomes while persisting independently verified siblings
in publication order.
issue key, and unexpected post-mutation child termination as possible
mutations. Emit per-finding terminal progress only after reconciliation.
handoff_recordedprogress only after each direct-API resultis durably appended. The CLI reports saved publication evidence without
claiming that an issue was created.
suppress a duplicate delivery of the same signal for 500 ms, then allow a
later repeated or different terminal signal to force-stop after warning that
retained evidence must be reconciled. Connected-app recovery continues to
wait for durable evidence.
readback or retry deduplication.
Testing
InternalLinearErrorregression: 0 passed, 1 failed, 2assertions.
failed, 56 assertions; presenter regression: 0 passed, 1 failed, 1 assertion.
publication-events,publication-integration,publish, andcli-publishsuites: 139 passed, 0 failed, 1,385 assertions in 8.60s.git diff --check: passed.SHA-256
737c10a53be8833d8ad878ace1cf7960af0a5a3130971b7dc857ecb4f0ab88e3.Installed public import, NodeNext types, CLI, 113 bundled plugin files,
bundled Codex version, and nested worker validations passed.
1,696 passed, 28 skipped, 1 failed, and 22,300 assertions. The exact failing
test passed immediately in isolation with the same seed: 1 passed, 0 failed,
and 9 assertions.
12345: 1,697 passed, 28 skipped, 0 failed, and22,307 assertions across 1,725 tests and 92 files in 292.34s.
gpt-5.6-solpasses at maximum reasoning completed on the committed head.All three repeated one receipt-diagnostic candidate; independent validation
rejected it as speculative because production receipts serialize only
PublishScanResult, while the environment is used only to resolve the statedirectory. No actionable P0-P3 finding remains.
Risk and rollout
Reconciliation is intentionally stricter: ambiguous, mismatched, disjoint, or
globally reused claims now produce an indeterminate recovery error instead of
being recorded as successful publication. Independently verified siblings may
already be persisted, so retained evidence must be reconciled before retrying.
PublishScanProgressgains ahandoff_recordedvariant. It reports only thata direct-API result was durably saved;
issue_completedremains the soleper-finding terminal outcome after global reconciliation. Exhaustive consumers
of the progress union must handle or ignore the new variant.
A later terminal signal can force-stop direct API publication after graceful
cancellation was requested. A remote mutation may have completed without a
local handoff, so the CLI warns users to reconcile retained publication evidence
before retrying. Connected-app repeat-signal behavior is unchanged.
No remote readback or automatic retry deduplication is added.
Public disclosure review
Reviewed the branch name, proposed title and description, two commits, ten
changed files, documentation, comments, diagnostics, fixtures, identifiers,
and links. Fixtures use explicit synthetic keys,
SEC-*andSYNTH-*identifiers, patterned UUIDs, and
linear.app/exampleURLs. No screenshots orattachments are included. All disclosure attestations were checked after the
final exact-head review.