refactor(cli): derive relaunch flag table from argparse introspection
Pull the top-level + chat parser construction out of main() into hermes_cli/_parser.py so relaunch.py can introspect parser._actions to discover which flags exist and whether they take values, instead of maintaining a parallel hand-rolled (flag, takes_value) tuple list. - _parser.py: build_top_level_parser() returns (parser, subparsers, chat_parser); side-effect-free import. - main.py: ~290 lines of inline parser construction collapsed to a helper call. Other subparsers stay inline (dispatch is bound to module-level cmd_* functions). - _parser._inherited_flag(parser, ...): wraps parser.add_argument and sets action.inherit_on_relaunch = True. Used in place of parser.add_argument for the 25 flags (top-level + chat) that need to carry over. - _parser.PRE_ARGPARSE_INHERITED_FLAGS: holds --profile/-p, which isn't on argparse (consumed earlier by main._apply_profile_override). - relaunch.py: drops _CRITICAL_DESTS and _PRE_ARGPARSE_FLAGS; the table builder now filters by getattr(action, 'inherit_on_relaunch', False). - test_ignore_user_config_flags.py: brittle inspect.getsource grep replaced with proper parser introspection. - test_relaunch.py: introspection sanity tests added. Salvaged from PR #17549; added top-level -t/--toolsets flag to _parser.py so #17623 (fix(tui): honor launch toolsets) behavior is preserved on current main. Co-authored-by: ethernet <arilotter@gmail.com>
This commit is contained in:
@@ -224,22 +224,21 @@ class TestArgparseFlagsRegistered:
|
||||
assert args.ignore_rules is True
|
||||
|
||||
def test_main_py_registers_both_flags(self):
|
||||
"""E2E: the real hermes_cli/main.py parser accepts both flags.
|
||||
"""E2E: the real hermes parser accepts both flags."""
|
||||
from hermes_cli._parser import build_top_level_parser
|
||||
|
||||
We invoke the real argparse tree builder from hermes_cli.main.
|
||||
"""
|
||||
import hermes_cli.main as hm
|
||||
parser, _subparsers, chat_parser = build_top_level_parser()
|
||||
|
||||
top_dests = {a.dest for a in parser._actions}
|
||||
chat_dests = {a.dest for a in chat_parser._actions}
|
||||
assert "ignore_user_config" in top_dests
|
||||
assert "ignore_rules" in top_dests
|
||||
assert "ignore_user_config" in chat_dests
|
||||
assert "ignore_rules" in chat_dests
|
||||
|
||||
# hm has a helper that builds the argparse tree inside main().
|
||||
# We can extract it by catching the SystemExit on --help.
|
||||
# Simpler: just grep the source for the flag strings. Both approaches
|
||||
# are brittle; we use a combined test.
|
||||
import inspect
|
||||
src = inspect.getsource(hm)
|
||||
assert '"--ignore-user-config"' in src, \
|
||||
"chat subparser must register --ignore-user-config"
|
||||
assert '"--ignore-rules"' in src, \
|
||||
"chat subparser must register --ignore-rules"
|
||||
# And the cmd_chat env-var wiring must be present
|
||||
import inspect
|
||||
import hermes_cli.main as hm
|
||||
src = inspect.getsource(hm)
|
||||
assert "HERMES_IGNORE_USER_CONFIG" in src
|
||||
assert "HERMES_IGNORE_RULES" in src
|
||||
|
||||
@@ -38,37 +38,69 @@ class TestResolveHermesBin:
|
||||
assert relaunch_mod.resolve_hermes_bin() is None
|
||||
|
||||
|
||||
class TestExtractCriticalFlags:
|
||||
class TestExtractInheritedFlags:
|
||||
def test_extracts_tui_and_dev(self):
|
||||
argv = ["--tui", "--dev", "chat"]
|
||||
assert relaunch_mod._extract_critical_flags(argv) == ["--tui", "--dev"]
|
||||
assert relaunch_mod._extract_inherited_flags(argv) == ["--tui", "--dev"]
|
||||
|
||||
def test_extracts_profile_with_value(self):
|
||||
argv = ["--profile", "work", "chat"]
|
||||
assert relaunch_mod._extract_critical_flags(argv) == ["--profile", "work"]
|
||||
assert relaunch_mod._extract_inherited_flags(argv) == ["--profile", "work"]
|
||||
|
||||
def test_extracts_short_p_with_value(self):
|
||||
argv = ["-p", "work"]
|
||||
assert relaunch_mod._extract_critical_flags(argv) == ["-p", "work"]
|
||||
assert relaunch_mod._extract_inherited_flags(argv) == ["-p", "work"]
|
||||
|
||||
def test_extracts_equals_form(self):
|
||||
argv = ["--profile=work", "--model=anthropic/claude-sonnet-4"]
|
||||
assert relaunch_mod._extract_critical_flags(argv) == [
|
||||
assert relaunch_mod._extract_inherited_flags(argv) == [
|
||||
"--profile=work",
|
||||
"--model=anthropic/claude-sonnet-4",
|
||||
]
|
||||
|
||||
def test_skips_unknown_flags(self):
|
||||
argv = ["--foo", "bar", "--tui"]
|
||||
assert relaunch_mod._extract_critical_flags(argv) == ["--tui"]
|
||||
assert relaunch_mod._extract_inherited_flags(argv) == ["--tui"]
|
||||
|
||||
def test_does_not_consume_flag_like_value(self):
|
||||
argv = ["--tui", "--resume", "abc123"]
|
||||
assert relaunch_mod._extract_critical_flags(argv) == ["--tui"]
|
||||
assert relaunch_mod._extract_inherited_flags(argv) == ["--tui"]
|
||||
|
||||
def test_preserves_multiple_skills(self):
|
||||
argv = ["-s", "foo", "-s", "bar", "--tui"]
|
||||
assert relaunch_mod._extract_critical_flags(argv) == ["-s", "foo", "-s", "bar", "--tui"]
|
||||
assert relaunch_mod._extract_inherited_flags(argv) == ["-s", "foo", "-s", "bar", "--tui"]
|
||||
|
||||
|
||||
class TestInheritedFlagTable:
|
||||
"""Sanity-check the argparse-introspected table that drives extraction."""
|
||||
|
||||
def test_short_and_long_aliases_are_paired(self):
|
||||
table = dict(relaunch_mod._INHERITED_FLAGS_TABLE)
|
||||
# Each pair declared together in the parser shares takes_value.
|
||||
for short, long_ in [
|
||||
("-p", "--profile"),
|
||||
("-m", "--model"),
|
||||
("-s", "--skills"),
|
||||
]:
|
||||
assert table[short] == table[long_], f"{short}/{long_} disagree"
|
||||
|
||||
def test_store_true_flags_do_not_take_value(self):
|
||||
table = dict(relaunch_mod._INHERITED_FLAGS_TABLE)
|
||||
for flag in ["--tui", "--dev", "--yolo", "--ignore-user-config", "--ignore-rules"]:
|
||||
assert table[flag] is False, f"{flag} should not take a value"
|
||||
|
||||
def test_value_flags_take_value(self):
|
||||
table = dict(relaunch_mod._INHERITED_FLAGS_TABLE)
|
||||
for flag in ["--profile", "--model", "--provider", "--skills"]:
|
||||
assert table[flag] is True, f"{flag} should take a value"
|
||||
|
||||
def test_excluded_flags_are_not_inherited(self):
|
||||
table = dict(relaunch_mod._INHERITED_FLAGS_TABLE)
|
||||
# --worktree creates a new worktree per process; inheriting would
|
||||
# orphan the parent's. Chat-only flags (--quiet/-Q, --verbose/-v,
|
||||
# --source) can't be in argv at the existing relaunch callsites.
|
||||
for flag in ["-w", "--worktree", "-Q", "--quiet", "-v", "--verbose", "--source"]:
|
||||
assert flag not in table, f"{flag} should not be inherited"
|
||||
|
||||
|
||||
class TestBuildRelaunchArgv:
|
||||
@@ -82,7 +114,7 @@ class TestBuildRelaunchArgv:
|
||||
argv = relaunch_mod.build_relaunch_argv(["--resume", "abc"])
|
||||
assert argv == [sys.executable, "-m", "hermes_cli.main", "--resume", "abc"]
|
||||
|
||||
def test_preserves_critical_flags(self, monkeypatch):
|
||||
def test_preserves_inherited_flags(self, monkeypatch):
|
||||
monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: "/usr/bin/hermes")
|
||||
original = ["--tui", "--dev", "--profile", "work", "sessions", "browse"]
|
||||
argv = relaunch_mod.build_relaunch_argv(["--resume", "abc"], original_argv=original)
|
||||
@@ -100,7 +132,7 @@ class TestBuildRelaunchArgv:
|
||||
monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: "/usr/bin/hermes")
|
||||
original = ["--tui", "chat"]
|
||||
argv = relaunch_mod.build_relaunch_argv(
|
||||
["--resume", "abc"], preserve_critical=False, original_argv=original
|
||||
["--resume", "abc"], preserve_inherited=False, original_argv=original
|
||||
)
|
||||
assert "--tui" not in argv
|
||||
assert argv == ["/usr/bin/hermes", "--resume", "abc"]
|
||||
|
||||
Reference in New Issue
Block a user