rebase fork patch queue onto upstream v2026.6.19 (incl. #44338 hand-port) - #7
Conversation
cwest
left a comment
cwest
left a comment
There was a problem hiding this comment.
Reviewed as a rebase, not a line diff — the 237k/50k churn is the v2026.6.5 → v2026.6.19 bump, and the real question is whether the 12-patch queue replayed correctly and the two conflicts were resolved right.
Verified against the branch:
git cherry v2026.6.19 topic/rebase-v2026.6.19shows exactly the 12 commits the description lists, all+. Upstream has absorbed none. Each carries its(cherry picked from commit …)provenance line;7c75a22correctly shows two (the squash of the original pair).- Conflict 1 (
79dc66c, scripts/release.py author map): the upstream additions are all preserved (AUTHOR_MAP intact) andcasey@geeknest.com → cwestis appended at the tail. Matches the description. - Conflict 2 (
926ecb4, dispatcher executor): the change landed in its new home, gateway/kanban_watchers.py, after upstream's extraction out of gateway/run.py. A private single-thread ThreadPoolExecutor backs an_offloadhelper, and all four dispatcher offloads (zombie reaper, auto-decompose, _tick_once, _ready_nonempty) route through it — none through the sharedasyncio.to_thread. The singleton-lock release on cancel is preserved and the executor is shut down (wait=False) in the finally. The corrupt-board tests were retargeted from gateway.run.* to gateway.kanban_watchers.* so they patch the code actually under test. This is the right resolution.
Ran the affected suites in a throwaway clone (uv sync, py3.11): dispatcher executor / starvation / corrupt-board 18 passed 1 skipped; external_dirs skill-count (#6) 4 passed; review respawn-guard bypass (NousResearch#46549) 66 passed; webhook/cron-fire 278 passed. Green.
The one genuinely new (non-replayed) code path is the webhook action allow-list (7a77d4b). The guard allowed_actions and action and action not in allowed_actions is correct and additive — empty/absent actions accepts everything. The fail-open on a missing action field is deliberate and called out in a code comment rather than left silent, which is the right call for GitHub payloads that always carry one.
One discrepancy, non-blocking: the 1a222c5 commit message says the VoiceMixer voice tests stay red until a companion T3 change lands, but on this branch they pass (19 passed) — the mocked-AudioSource base from e16fd02 is what makes them green. The note in the commit body is stale relative to where the branch ended up; worth a one-line correction in card B's PATCHES.md refresh so the manifest doesn't carry a wrong status.
Build/test was deferred to card B per scope, so this is a structural + targeted-suite review, not a full-suite run. As a draft this is in good shape — the queue is faithful and both conflicts are resolved correctly.
cwest
changed the title
cwest
left a comment
cwest
left a comment
There was a problem hiding this comment.
Re-review of the current head (650f403) after the synchronize from 926ecb4. The delta is a clean fast-forward: three new commits, nothing dropped or re-ordered, and the diff touches exactly five files. The earlier findings on the rebase queue (both conflict resolutions, the webhook allow-list, provenance) are unchanged because none of those files moved.
The only new code is 91dd0a8, the hand-port of the NousResearch#44338 notifier fix into gateway/kanban_watchers.py. It replaces the drop-after-N-failures behavior with: rewind the pre-send claim on every delivery failure so the terminal event stays unseen, keep the subscription, and after MAX_SEND_FAILURES enter per-subscription exponential backoff gated before the cursor claim. I traced it end to end:
- SendResult(success=False) now raises into the same failure path as an exception, so a non-raising adapter (matrix) no longer advances the cursor on a failed send.
- The backoff gate at line 245 builds its lookup key inline as (task_id, platform, chat_id, thread_id or ""), identical to sub_key at the write site, so the read and write hit the same dict entry. Both sides read the same monotonic clock.
- _kanban_unsub is no longer reachable from the failure path; the only remaining unsub is the success path on done/archived, which is correct.
- The four new tests cover false-result delivery, keep-alive across repeated failures, backoff-window suppression, and recovery-after-backoff. I ran the notifier suite plus the two helper tests the rename touched: 25 passed.
The PATCHES.md reconciliation is accurate. I checked every PR-state claim against live upstream: NousResearch#44023 and NousResearch#46549 are OPEN, NousResearch#44338 is CLOSED, NousResearch#45940 and NousResearch#46443 are OPEN. The behavior-keyed retire rule for the NousResearch#44338 row follows from that: the PR was closed administratively rather than merged, so the default PR-merge retire trigger can never fire, and NousResearch#45940 adds only SendResult detection without the keep-alive or backoff, so dropping the carry on its merge would regress. The per-row override note in the auto-retire section guards a future rebaser against applying the merge rule here.
Still a draft, so this is a COMMENT, not an approve. One non-blocking note inline. Merge stays Casey's call.
| send_result = await adapter.send( | ||
| sub["chat_id"], msg, metadata=metadata, | ||
| ) | ||
| if getattr(send_result, "success", True) is False: |
There was a problem hiding this comment.
getattr(send_result, "success", True) treats a None return (an adapter that returns nothing) as success and skips the raise. That is the right back-compat default for adapters that signal delivery by not raising, and the matrix case this fixes returns an explicit SendResult, so it is covered. Worth a one-line comment naming the None-means-success assumption so a future adapter author does not start returning None on failure and silently reintroduce the dropped-notification bug.
1 participant
Rebase the fork patch queue onto upstream
v2026.6.19Re-establishes the integration line as upstream tag
v2026.6.19+ our patchqueue replayed atomically on top, replacing the previous base (
v2026.6.5+170 commits of drift). Each carried patch is preserved as its own atomic commit
via
git cherry-pick -x(origin refs recorded in each commit trailer).Kanban card:
t_ec2b5f16.Patches replayed (12, in order)
1a222c5988c5fce16fd022045862c890de8915a4335866d9b7c75a222ec7e607a77d4b79dc66c926ecb4git cherry v2026.6.19 topic/rebase-v2026.6.19shows exactly these 12 (all+)— upstream has absorbed none of them.
Excluded by design
70bb7153ffix(gateway): retry failed kanban notifications (NousResearch#44338 hand-port)is not in this branch — it is carried separately as its own atomic step in
card A2 (already decided KEEP).
Conflicts resolved (2 of 12)
79dc66cskill-count fix — conflict inscripts/release.pyauthor-mapdict; upstream added many new author entries after the old tail line.
Resolution: kept every upstream entry and appended our
casey@geeknest.com → cwestmapping at the end of the dict.926ecb4dispatcher executor — code-move conflict: upstreamv2026.6.19 extracted the kanban watcher/dispatcher loops out of
gateway/run.pyinto the newGatewayKanbanWatchersMixin(
gateway/kanban_watchers.py, god-file Phase 3). Took HEAD forgateway/run.py(the extraction) and re-applied the dedicated-executorchange to its new home in
gateway/kanban_watchers.py(preserving theupstream singleton-lock release logic). The two stubbed dispatcher tests had
their monkeypatch targets retargeted from
gateway.run.*togateway.kanban_watchers.*so they patch the code under test; the newstarvation regression test drives
_kanban_dispatcher_watcher()directly andneeded no module change.
The other 10 patches applied cleanly (a few benign auto-merges, no markers).
Notes
Draft — do not mark ready or merge.
Update — card A2 carried onto this branch (
t_74112775)The NousResearch#44338 kanban-notifier hand-port and its manifest retire-trigger fix are
now included on this branch (they were excluded from the original rebase as
their own atomic step; A2 places them here per the card plan). Two new commits:
91dd0a818f39b6NousResearch#44338 hand-port — re-targeted after the god-file refactor
git cherry-pick -x 70bb7153fcollided with upstream's Phase 3 extraction: theoriginal carry modified the notifier loop in
gateway/run.py, but v2026.6.19moved that loop into
GatewayKanbanWatchersMixin(gateway/kanban_watchers.py).Resolution:
gateway/run.pytaken--ours(the extraction stands; no orphanloop re-introduced), and the four behaviors re-applied by hand to the loop's new
home in
gateway/kanban_watchers.py:SendResult(success=False)fromadapter.sendas a delivery failure (raise).The cherry-picked test changes (
_kanban_sub_fail_counts->_kanban_sub_fail_statesplus 4 new behavior tests) applied cleanly. All 25 notifier tests pass locally
(
tests/gateway/test_kanban_notifier.py,tests/gateway/test_kanban_notifier_watcher_dispatch_gate.py,tests/hermes_cli/test_kanban_notify.py).Manifest retire-trigger fix (
PATCHES.md)The NousResearch#44338 row's old trigger ("auto-retire when NousResearch#44338 merges upstream") can
never fire — NousResearch#44338 was closed administratively, not merged. Rewrote it to be
behavior-keyed: retire only when upstream
gateway/kanban_watchers.pyimplementsALL of (i) SendResult failure-detection, (ii) keep-alive-on-permanent-failure,
and (iii) bounded exponential backoff. Explicitly do not drop on NousResearch#45940 merge
alone (NousResearch#45940 is detection-only and would regress behaviors ii + iv). Watch
NousResearch#45940 and NousResearch#46443. Bumped the row's base-tag to v2026.6.19 and added a per-row
override caveat to the global Auto-retire rule.
Signed commits (ssh), Conventional Commits, no AI attribution. Still a draft.
Update — card B: build + targeted tests + PATCHES.md reconciled (
t_a8821d26)Built and tested the branch in the worktree's own
.venv(Python 3.12.13,pytest 9.0.2;
uv sync --extra dev,--extra voiceadded for the VoiceMixernumpy path). The live install at
~/.hermes/hermes-agentwas not touched. Onenew commit:
650f403Targeted regression tests — all green
test_kanban_core_functionality.py -k "dispatcher or corrupt or executor or starv"external_dirs(fork PR #6)tests/tools/test_skills_tool.py::TestCountProfileSkillstest_kanban_notifier.py,test_kanban_notifier_watcher_dispatch_gate.py,test_kanban_notify.pytest_kanban_db.py -k respawn(incl. the 4 review-bypass tests)tests/gateway/test_discord_voice_mixer.pyvoiceextra'snumpy)test_webhook_cli.py -k "allow or filter or action"Combined targeted run (executor + skill-count + 3 notifier files + 4
respawn-bypass tests): 35 passed, 0 failed. No failures across any carried
patch.
hermes doctor(worktree binary, code under test)Reports v0.17.0 (2026.6.19), local
650f403. All code/environment-integritychecks clean: Python Environment ✓ (version files consistent 0.17.0), Required
Packages ✓, SSL/CA ✓, Security Advisories ✓, MCP Security ✓, Directory
Structure ✓, Profiles ✓. The only warnings are runtime-config/credential items
for this isolated environment (missing API keys, optional telegram/discord
libs, config v27→v30 migration available) — none are code defects from the
rebase.
PATCHES.md reconciled
v2026.6.5(3c231eb) →v2026.6.19(681cd638d).base-tagis now v2026.6.19; PR statuses verified live viagh:fix(discord): inherit AudioSource in VoiceMixer NousResearch/hermes-agent#44023 OPEN, fix(kanban): review tasks bypass dup-PR respawn guards (active_pr/recent_success) NousResearch/hermes-agent#46549 OPEN, fix(gateway): retry failed kanban notifications NousResearch/hermes-agent#44338 CLOSED (behavior-keyed retire,
unchanged), fork PR fix(gateway): run in-process kanban dispatcher on a dedicated executor #4 dispatcher OPEN, fork PR 🐛 fix(profiles): count external_dirs skills so dashboard matches CLI #6 skill-count
MERGED.
dispatcher (fork PR fix(gateway): run in-process kanban dispatcher on a dedicated executor #4, upstream-pending — no upstream PR# yet, so a
behavior-keyed retire trigger), skill-count (fork PR 🐛 fix(profiles): count external_dirs skills so dashboard matches CLI #6, permanent-local),
webhook allow-list, the two CI workflows, and PATCHES.md itself
(permanent-local).
git cherry v2026.6.19 topic/rebase-v2026.6.19shows allcarried patches as
+(upstream absorbed none).Signed commit (ssh), Conventional Commits, no AI attribution. Still a draft —
do not mark ready or merge.