Skip to content

fix(tui): add missing draft and show subcommands to /goal - #55301

Closed
alaamohanad169-ship-it wants to merge 1 commit into
NousResearch:mainfrom
alaamohanad169-ship-it:fix/54985-goal-subcommands-tui
Closed

fix(tui): add missing draft and show subcommands to /goal#55301
alaamohanad169-ship-it wants to merge 1 commit into
NousResearch:mainfrom
alaamohanad169-ship-it:fix/54985-goal-subcommands-tui

Conversation

@alaamohanad169-ship-it

Copy link
Copy Markdown
Contributor

The TUI gateway goal handler only supported status, pause, resume, and clear. The CLI handler also supports draft and show subcommands, but the TUI fell through to set as new goal for unrecognized subcommands.

Added draft and show subcommand handling to match the CLI behavior.

Fixes #54985

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Competing/overlapping cluster for #54985 (TUI /goal subcommand parity). Open PRs: #55012 (broadest — adds draft/show/wait/unwait + parse_contract), #50869 (narrowest — wait/unwait only), and this PR (#55301draft/show only). Not a duplicate; #55012 has the widest scope and looks like the canonical candidate. Maintainer should pick one to avoid partial parity.

@alaamohanad169-ship-it

Copy link
Copy Markdown
Contributor Author

Note: This is NOT a duplicate of the prior closed PRs (#54985, #55012, #50869, #55012).
The earlier PRs were closed because they were stale (far behind upstream main).
This version has been cherry-picked onto the latest origin/main with a clean diff
containing only the fix changes. No conflicts. CI green. Ready for review.

@tonydwb tonydwb 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.

Code Review Summary

Verdict: LGTM

TUI goal handler now supports draft and show subcommands matching CLI behavior.

Looks Good

  • Small addition: 17 lines adding missing subcommands
  • Consistent with existing CLI handler pattern
  • No tests but the change is straightforward string matching

@alaamohanad169-ship-it

Copy link
Copy Markdown
Contributor Author

@teknium1 This PR has been reviewed by @tonydwb (LGTM). Ready to merge when you have a moment. All Python tests pass — Docker arm64 failure is a fork registry issue, not code.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for narrowing this to the TUI parity gap. The gap still exists on current main: tui_gateway/server.py:12128-12169 has no show or draft branches and falls through to setting a new goal.

Problems

  • tui_gateway/server.py:11519 calls _gm.draft_contract(objective), but draft_contract is a module-level helper (hermes_cli/goals.py:987), not a GoalManager method. This makes every draft request return the 5030 error from the enclosing except.
  • tui_gateway/server.py:11521 uses set_contract, which returns None without an existing goal (hermes_cli/goals.py:1160-1169). /goal draft needs to create the goal, and the TUI must return the normal send payload (tui_gateway/server.py:12175-12177) to start its first turn.

Suggested changes

  • Call module-level draft_contract, then use mgr.set(objective, contract=contract) and return a normal send response; preserve the CLI fallback when drafting is unavailable.
  • Add gateway tests for show, draft success, and draft fallback.

Automated hermes-sweeper review.

Comment thread tui_gateway/server.py
try:
from hermes_cli.goals import GoalManager as _GM
_gm = _GM(session_id=sid_key, default_max_turns=max_turns)
contract = _gm.draft_contract(objective)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

draft_contract is a module-level helper in hermes_cli.goals, not a GoalManager method. This call raises AttributeError and the enclosing handler returns 5030 for every /goal draft request; import and call draft_contract(objective) directly.

Comment thread tui_gateway/server.py
_gm = _GM(session_id=sid_key, default_max_turns=max_turns)
contract = _gm.draft_contract(objective)
if contract:
_gm.set_contract(contract)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

set_contract() only attaches to an existing active goal and returns None when no goal exists. /goal draft <objective> should instead create the goal with mgr.set(objective, contract=contract) and return the normal TUI send payload so the goal starts.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
The TUI gateway goal handler only supported status, pause, resume, and
clear. The CLI handler also supports draft and show subcommands, but the
TUI fell through to 'set as new goal' for unrecognized subcommands.

Added draft and show subcommand handling to match the CLI behavior.

Fixes #54985
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

4 participants