Skip to content

fix(kanban): preserve native card during webhook convergence - #16

Merged
solovision24 merged 2 commits into
dev/hermes-upgrade-t_16bbffadfrom
dev/kanban-webhook-native-convergence-t-9de88053
Aug 2, 2026
Merged

fix(kanban): preserve native card during webhook convergence#16
solovision24 merged 2 commits into
dev/hermes-upgrade-t_16bbffadfrom
dev/kanban-webhook-native-convergence-t-9de88053

Conversation

@solovision24

@solovision24 solovision24 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Preserve the native implementation task as the canonical card when a GitHub webhook card arrived first.
  • Finalize a claimed webhook reviewer run during reconciliation so no live orphaned run is moved to the native card.
  • Update reconciliation documentation and add claimed-webhook race coverage.

Verification

  • scripts/run_tests.sh tests/hermes_cli/test_kanban_review_lifecycle.py tests/hermes_cli/test_kanban_cli.py — 30 passed
  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py tests/tools/test_kanban_tools.py tests/gateway/test_kanban_notifier.py tests/gateway/test_kanban_notifier_watcher_dispatch_gate.py tests/hermes_cli/test_kanban_count_notify_subs.py — 77 passed
  • git diff --check and python3 -m compileall -q hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_review_lifecycle.py

Kanban Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f80c96a3c7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

Comment thread hermes_cli/kanban_db.py
Comment on lines +4663 to +4666
_merge_task_history(
conn,
str(duplicate["id"]),
canonical_id,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Clear the archived webhook card's idempotency key

When the webhook card arrived first, this fold archives it but leaves its github-pr:... idempotency key intact before assigning that same key to the native card. On the next webhook replay, ingest_pull_request() orders matching rows oldest-first, selects the archived webhook row as same_head, archives the active native card as superseded, and creates a third card even though the SHA is unchanged. Clear the source key during reconciliation or make ingestion prefer the active same-head row so webhook retries do not undo native-card preservation.

AGENTS.md reference: AGENTS.md:L54-L57

Useful? React with 👍 / 👎.

Comment thread hermes_cli/kanban_db.py Outdated
conn,
str(duplicate["id"]),
canonical_id,
source_run_id=None,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Terminate an in-flight webhook review run before folding

When the webhook-created card has already been claimed by a reviewer, passing source_run_id=None migrates that run onto the native task without ending it. The native implementation run is then ended and current_run_id cleared, leaving an unreferenced task_runs row with status='running', its claim lock, and no ended_at; meanwhile the existing reviewer worker still targets the now-archived webhook task and cannot complete its review. Reconciliation must explicitly close or otherwise safely transfer the duplicate card's active run.

AGENTS.md reference: AGENTS.md:L54-L57

Useful? React with 👍 / 👎.

@solovision24

Copy link
Copy Markdown
Owner Author

Orion Review — changes required

Reviewed immutable head f80c96a3c7e24d559ff70e8cbfaebfff73cfcb58 directly and reproduced one lifecycle defect.

Blocking findings

  1. PR fix(kanban): preserve native card during webhook convergence #16 is not a reviewable/mergeable artifact. It targets main, where GitHub reports CONFLICTING / DIRTY, 8,100 changed files, +2,126,631/-176,748, and no checks. The approved convergence base is dev/hermes-upgrade-t_16bbffad at af8e57b3e0e42262c976165da066572ded7236c6 (PR fix(kanban): reconcile webhook and native review handoffs #13 merge). Rebuild the branch from that exact base, apply only the intended remediation, update the PR base, and return a new immutable head with a clean, focused diff and green checks.

  2. hermes_cli/kanban_db.py:4663 leaves an orphaned live reviewer run when the webhook-first card has already been claimed. Reproduction: claim the webhook Review card, then submit the native implementation card. Submission returns True and preserves the native task, but _merge_task_history(..., source_run_id=None) moves the webhook reviewer run onto the native card without ending it; the native card ends with current_run_id=None while that moved run remains status='running', ended_at=NULL. The original reviewer worker is simultaneously operating against an archived source task. Reconcile/terminate that run deterministically and add coverage for the claimed-webhook race, not only the unclaimed case.

  3. Same-card documentation is still stale. _merge_task_history at hermes_cli/kanban_db.py:4014-4018 says it moves an implementation card onto a PR card, and the workspace comment at 4027 assumes a webhook canonical destination. The function now runs in the reverse direction for webhook-first native submission. Update the helper contract/comments so they accurately describe both directions or the native-card invariant.

Verification performed

  • Focused lifecycle/CLI: 29 passed.
  • Kanban DB/tools/notifier/count suite: 77 passed.
  • git diff --check and compileall: passed.
  • Claimed-webhook race: reproduced the orphaned running run described above.

Do not merge this head. Return one focused, mergeable, green immutable head on the approved fork base.

@solovision24
solovision24 changed the base branch from main to dev/hermes-upgrade-t_16bbffad August 2, 2026 17:08
@solovision24
solovision24 force-pushed the dev/kanban-webhook-native-convergence-t-9de88053 branch from 9497754 to cca6f8f Compare August 2, 2026 17:09
@solovision24

Copy link
Copy Markdown
Owner Author

Orion re-review — changes required

Reviewed immutable head cca6f8fd3d5ec2ed55dd9548935a4a10d8e40fd3 directly. The artifact is now focused and mergeable against the approved base, and the claimed-webhook run finalization plus subscription reconciliation pass focused verification. One requested correction is still incomplete, and required CI is red.

Blocking findings

  1. The old canonical direction remains documented at hermes_cli/kanban_db.py:4645-4648. The comment still says to “fold the implementation card into the already-created webhook card,” while the implementation immediately below correctly preserves task_id as canonical and folds the webhook card into it. Update this comment to state the native-card invariant so the helper/call-site documentation is internally consistent.

  2. Required CI is not green on this immutable head. Python tests / Run tests slice 1/8 and therefore All required checks pass failed. The failing test is tests/hermes_cli/test_update_eol_churn.py::test_churn_across_more_files_than_fit_in_one_argv, outside this two-file diff; return a new head with required checks green and do not absorb unrelated EOL implementation changes into this PR.

Verification performed

  • PR base/head/mergeability: dev/hermes-upgrade-t_16bbffad at af8e57b3e0e42262c976165da066572ded7236c6; head cca6f8fd3d5ec2ed55dd9548935a4a10d8e40fd3; MERGEABLE.
  • Complete diff reviewed: 2 files, +115/-39.
  • Focused Kanban lifecycle/CLI/DB/tools/notifier suite: 107 passed.
  • git diff --check: passed.
  • The CI EOL test is independently failing locally on the approved base/worktree as a broader pre-existing issue (5 failures locally), not caused by the two-file remediation diff.

Acceptance: correct the stale call-site comment, push one new immutable head on the same focused branch, and return with required checks green or with explicit proof that the repository gate has accepted/rerun the unrelated failure.

@solovision24

Copy link
Copy Markdown
Owner Author

ORION ACCEPTED — exact immutable head 5d9ea5e3e4fadf41a09b09714136ecbb9f47bb4f.

Reviewed the complete two-file diff from base af8e57b3e0e42262c976165da066572ded7236c6, including the final comment-only synchronize delta from cca6f8fd3d5ec2ed55dd9548935a4a10d8e40fd3. Native implementation-card canonicality, claimed webhook-run finalization, duplicate subscription convergence, same-card lifecycle, and corrected documentation are consistent. Independent exact-head focused suite passed 107/107; compile and diff checks passed; GitHub required aggregate, all 8 Python slices, contributor attribution, lints, security, and Desktop E2E are green. Same-account Orion governance acceptance is recorded here even though GitHub cannot record a native APPROVED review.

@solovision24
solovision24 merged commit 09aed9b into dev/hermes-upgrade-t_16bbffad Aug 2, 2026
37 checks passed
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.

1 participant