fix(kanban): reject unknown assignee at create time (no silent dispatcher drop) - #49
Conversation
…cher drop) The dispatcher only spawns a ready task whose assignee names a real profile on disk; a typo'd or never-built assignee was accepted onto the board and then silently never spawned. Validate the assignee at create time so a bad route fails loud immediately and nobody has to pre-probe the lane list. - New shared validator kanban_db.validate_assignee() reuses the existing list_profiles_on_disk() (no new enumeration). Unassigned sentinels (None/empty/none/-/null) stay valid; a named assignee that is neither a known profile nor a sentinel returns an error that names the bad value and lists the valid profiles so the model self-corrects. - Tool path (tools/kanban_tools.py::_handle_create) and CLI path (hermes_cli/kanban.py::_cmd_create) both call the one shared validator, so the two surfaces cannot drift. No row is created on rejection. - Scope: assignee-only. No other field validation, no dispatcher change, no surrounding refactor. Tests: tests/hermes_cli/test_kanban_validate_assignee.py (validator unit cases, tool + CLI create happy/reject paths, board-unchanged-on-reject). Existing kanban tool/CLI suites updated to seed profile dirs for fake assignees. Patch note: ~/.hermes/plans/hermes-patches/kanban-validate-assignee.md
🔎 Lint report:
|
| Rule | Count |
|---|---|
PLW1514 |
1 |
First entries
gateway/run.py:5594: [PLW1514] `open` in text mode without explicit `encoding` argument
✅ Fixed issues (1):
| Rule | Count |
|---|---|
PLW1514 |
1 |
First entries
../../../../../tmp/lint-base/gateway/run.py:5594: [PLW1514] `open` in text mode without explicit `encoding` argument
Unchanged: 0 pre-existing issues carried over.
ty (type checker)
Total: 11573 on HEAD, 11574 on base (✅ -1)
🆕 New issues (2):
| Rule | Count |
|---|---|
unresolved-import |
1 |
invalid-assignment |
1 |
First entries
tests/hermes_cli/test_kanban_validate_assignee.py:20: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
✅ Fixed issues (2):
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
run_agent.py:3026: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
Unchanged: 6092 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces assignee validation during Kanban task creation across both the CLI and tool interfaces, preventing tasks with invalid assignees from being silently ignored by the dispatcher. The validation checks the assignee against known profiles on disk while allowing unassigned sentinels. Feedback suggests enhancing this validation by consistently normalizing both input and disk profile names to prevent case-sensitivity mismatches, and adding defensive exception handling when reading profiles from disk.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| try: | ||
| from hermes_cli.profiles import normalize_profile_name | ||
|
|
||
| canon = normalize_profile_name(text) | ||
| except Exception: | ||
| canon = text | ||
| known = set(list_profiles_on_disk()) | ||
| if canon in known: |
There was a problem hiding this comment.
To ensure robust and defensive validation, we should address two potential issues here:
- Case-sensitivity / Normalization Mismatch: If a profile directory on disk contains uppercase letters or is not fully normalized, but
normalize_profile_namenormalizes the inputassignee, a direct membership check (canon in known) might fail. Normalizing both the input and the disk profile names using the same function guarantees consistent matching. - Robust Exception Handling: If
list_profiles_on_disk()raises an exception (e.g., due to permission issues or missing directories on an uninitialized system), the function will crash. Wrapping it in atry-exceptblock ensures defensive fallback to an empty list.
try:
raw_known = list_profiles_on_disk()
except Exception:
raw_known = []
try:
from hermes_cli.profiles import normalize_profile_name
canon = normalize_profile_name(text)
known = {normalize_profile_name(p) for p in raw_known}
except Exception:
canon = text
known = set(raw_known)
if canon in known:![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66cbfa704b
ℹ️ 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".
| known = set(list_profiles_on_disk()) | ||
| if canon in known: | ||
| return None |
There was a problem hiding this comment.
This makes list_profiles_on_disk() a hard allow-list, but dispatch_once intentionally treats non-profile assignees as control-plane/terminal lanes (for example orion-cc) that are skipped by the dispatcher and claimed directly via claim_task. In those setups, /kanban create --assignee orion-cc and the kanban_create tool now fail before a row is created, breaking the documented external/terminal lane workflow rather than just catching typos. Please keep an explicit path for intentional non-profile lanes instead of rejecting every assignee absent from the profile list.
Useful? React with 👍 / 👎.
| assignee_error = kb.validate_assignee(args.assignee) | ||
| if assignee_error: | ||
| print(f"kanban: {assignee_error}", file=sys.stderr) | ||
| return 2 |
There was a problem hiding this comment.
When a create request is retried with an --idempotency-key for an existing non-archived task whose assignee profile was later renamed or deleted, this preflight rejects the retry before kb.create_task can reach its existing idempotency lookup and return the original task id. That turns an idempotent retry into a hard failure; the tool path has the same ordering, so webhook/orchestrator retries can break after a profile cleanup even though no new row would be inserted.
Useful? React with 👍 / 👎.
| if not text or text.casefold() in _UNASSIGNED_SENTINELS: | ||
| return None |
There was a problem hiding this comment.
This new validator accepts and the error text recommends none as an unassigned sentinel, but both create callers still pass the original string into create_task. When a user or model follows that hint (--assignee none / assignee: "none"), the row is stored with literal assignee none, so the dispatcher treats it as skipped_nonspawnable instead of unassigned (and kanban.default_assignee will not apply). The sentinel needs to be converted to None before insertion, or not advertised as creating an unassigned card.
Useful? React with 👍 / 👎.
1 participant
Problem
The dispatcher only spawns a ready task whose
assigneenames a real profile on disk. A card created with a typo'd or never-built assignee (e.g.equity-analyzerinstead ofequity-analyst) is accepted onto the board and then silently never spawns — it sits inreadyforever with no error. This recently made an orchestrator waste a run shelling out tohermes profile listto pre-verify lanes (no terminal → crashed).Fix
Validate the assignee at create time so a bad route fails loud immediately.
kanban_db.validate_assignee(assignee) -> Optional[str], sitting next to and reusing the existinglist_profiles_on_disk()(no new profile-enumeration path per spec).None, empty,none/-/null, matching the tool's existing_normalize_profile) → valid. A deliberately-unassigned/triage card stays legal.normalize_profile_name(same canonicalizationcreate_taskalready applies) and checked againstset(list_profiles_on_disk()). Unknown → error naming the bad value and listing valid profiles so the model self-corrects.tools/kanban_tools.py::_handle_create) and CLI path (hermes_cli/kanban.py::_cmd_create) both call the one shared validator → cannot drift. No row created on rejection.Scope (deliberately narrow)
Assignee-only. No other field validation, no dispatcher change, no surrounding refactor.
Known divergence (flagged for review)
The dispatcher's runtime gate is
profile_exists()(keys on the profile dir), while this validator useslist_profiles_on_disk()(requires aconfig.yaml). They agree for the canonical "lane doesn't exist" case — the only case this guard targets. A dir-without-config.yaml(a half-built profile) would be rejected at create time though the dispatcher would attempt to spawn it. The spec explicitly namedlist_profiles_on_disk()as the helper to reuse; documented in the patch note. Flagging in case you'd prefer the validator key on the same gate as the dispatcher.Tests — fail-before / pass-after
tests/hermes_cli/test_kanban_validate_assignee.py(16 cases): validator unit cases (real/unknown/sentinels/default/case-normalization), tool_handle_create(real OK, unknown → tool_error + board row count unchanged,nonesentinel OK), CLI_cmd_create(unknown--assignee→ no row, error surfaced).Fail-before (source changes stashed — validator absent / row created today):
Pass-after (new test file):
16 passed in 11.13sFull regression (tool + CLI + core kanban suites + new file):
319 passed, 1 skipped.ruff checkclean on all touched files.Existing kanban tool/CLI test fixtures were updated to seed profile dirs for the fake assignees they route to (those assignees now must exist on disk).
Patch note:
~/.hermes/plans/hermes-patches/kanban-validate-assignee.md