fix(kanban): deliver actionable active-profile notifications - #62380
fix(kanban): deliver actionable active-profile notifications#62380salimhamed wants to merge 1 commit into
Conversation
|
Thanks for the focused regression fix. The premise is confirmed on current main: The blocked-event formatter also truncates the reason at Automated hermes-sweeper review. |
49fc71e to
5037514
Compare
|
Independently confirmed this exact failure in a real non-multiplex gateway running under a named active profile. Observed chain:
I also reproduced the resolver/notifier behavior independently with an ephemeral database and recording adapter: I checked current remote I did not check out or modify the PR branch because my local Hermes checkout contains unrelated concurrent work. This comment confirms the bug and the proposed routing direction, not a full branch test or formal approval. Environment of the operational reproduction: macOS arm64, Python 3.11, Hermes Agent 0.18.2. Private chat IDs, task IDs, and local paths intentionally omitted. |
KoryakovDmitry
left a comment
There was a problem hiding this comment.
I independently reproduced the named-primary notifier failure, but the current resolver condition leaves one cross-profile routing hole:
if profile_name and profile_name not in {"default", active_profile}:When the active gateway profile is named (for example job-search), an explicit stamped profile="default" still falls through to self.adapters, i.e. the named primary bot. That does not preserve the documented fail-closed invariant for every non-primary stamp.
The safer condition is to resolve exactly one primary identity:
active = getattr(self, "_active_profile_name", None)
primary_profile = active() if callable(active) else "default"
primary_profile = str(primary_profile or "default").strip() or "default"
if profile_name and profile_name != primary_profile:
# secondary lookup, then fail closedA regression contract that fails on the current PR implementation:
runner._active_profile_name = lambda: "job-search"
runner._profile_adapters = {}
assert runner._authorization_adapter(Platform.WECOM, profile="default") is NoneI verified the stricter form against the notifier + multiplex/upstream authz suites: 30 passed; Ruff and ty pass. The PR is also currently conflicting with main, so it needs a rebase before this can land.
A gateway running under a named active profile (e.g. `hermes -p main gateway`) stamps kanban auto-subscriptions with notifier_profile=main, but _authorization_adapter() treated any name other than the literal "default" as a multiplex secondary and consulted only _profile_adapters — empty on standalone gateway-per-profile deployments. The helper failed closed, the notifier rewound the claim, and the notification was silently retried forever (#71340). Recognize the gateway's own active profile name as primary so its stamped subscriptions resolve via self.adapters; genuinely secondary profiles keep the fail-closed lookup. Salvaged from PR #62380 (the unrelated blocked-reason truncation change is intentionally not taken).
A gateway running under a named active profile (e.g. `hermes -p main gateway`) stamps kanban auto-subscriptions with notifier_profile=main, but _authorization_adapter() treated any name other than the literal "default" as a multiplex secondary and consulted only _profile_adapters — empty on standalone gateway-per-profile deployments. The helper failed closed, the notifier rewound the claim, and the notification was silently retried forever (#71340). Recognize the gateway's own active profile name as primary so its stamped subscriptions resolve via self.adapters; genuinely secondary profiles keep the fail-closed lookup. Salvaged from PR #62380 (the unrelated blocked-reason truncation change is intentionally not taken).
A gateway running under a named active profile (e.g. `hermes -p main gateway`) stamps kanban auto-subscriptions with notifier_profile=main, but _authorization_adapter() treated any name other than the literal "default" as a multiplex secondary and consulted only _profile_adapters — empty on standalone gateway-per-profile deployments. The helper failed closed, the notifier rewound the claim, and the notification was silently retried forever (#71340). Recognize the gateway's own active profile name as primary so its stamped subscriptions resolve via self.adapters; genuinely secondary profiles keep the fail-closed lookup. Salvaged from PR #62380 (the unrelated blocked-reason truncation change is intentionally not taken).
|
Partially merged via PR #72241 — your adapter-routing fix (active named profile resolves through the active adapter map, killing the #71340 rewind-forever loop) was cherry-picked with your authorship preserved. The 160→3000-char blocked-reason truncation change was out of scope for that cluster and was not taken — feel free to resubmit it as a focused PR if you still want it. Thanks! |
A gateway running under a named active profile (e.g. `hermes -p main gateway`) stamps kanban auto-subscriptions with notifier_profile=main, but _authorization_adapter() treated any name other than the literal "default" as a multiplex secondary and consulted only _profile_adapters — empty on standalone gateway-per-profile deployments. The helper failed closed, the notifier rewound the claim, and the notification was silently retried forever (NousResearch#71340). Recognize the gateway's own active profile name as primary so its stamped subscriptions resolve via self.adapters; genuinely secondary profiles keep the fail-closed lookup. Salvaged from PR NousResearch#62380 (the unrelated blocked-reason truncation change is intentionally not taken).
Summary
Reproduction
A gateway running profile
mainwith a Kanban subscription stampednotifier_profile=maintreatedmainas a secondary profile._authorization_adapter()returnedNone, so the notifier rewound each claim and never delivered. Separately, a 413-character approval reason was truncated before itsApprove/Request changeschoices.Tests
uv run --with pytest --with pytest-asyncio pytest -q tests/gateway/test_kanban_notifier.py tests/gateway/test_multiplex_profile_authz.py tests/gateway/test_kanban_notifier_watcher_dispatch_gate.py18 passed.