Skip to content

test(kanban): preserve non-zero exit for delegated-child refusals - #70667

Merged
teknium1 merged 2 commits into
NousResearch:mainfrom
vadelma-agent:fix/kanban-cli-exit-status
Aug 18, 2026
Merged

test(kanban): preserve non-zero exit for delegated-child refusals#70667
teknium1 merged 2 commits into
NousResearch:mainfrom
vadelma-agent:fix/kanban-cli-exit-status

Conversation

@vadelma-agent

@vadelma-agent vadelma-agent commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Add regression coverage for the Kanban CLI process boundary so a delegated-child refusal remains a non-zero process exit instead of being interpreted as successful automation.

Observed behavior and root cause

The production fix for propagating non-zero command-handler results is already on upstream main. The remaining contribution is test coverage at the subprocess boundary: the CLI test must observe a refused delegated-child operation as a failure, not merely inspect an in-process return value.

Without this regression coverage, an automation caller could treat a rejected Kanban mutation as success even though the user-facing command had refused the operation.

Changes Made

  • Added coverage in tests/hermes_cli/test_kanban_cli_exit_status.py for delegated-child refusal exit status.
  • Kept the PR test-only: no production behavior, dependency, configuration, or platform adapter changes.
  • Bound the subprocess helper with an explicit timeout so a broken CLI path cannot hang the regression test indefinitely.
  • The current series contains two focused test commits and one changed test file.

Related Issue

No standalone issue. The production behavior is already present on upstream main; this PR preserves the missing regression contract.

How to Test

Current exact-head verification

Published head: 7a22b9036f3aab34833e8fba5d025883750517a4
Rebased base: 8911e2e0edf750b104edbdc106d63d6cdac88524

The repository's canonical isolated test wrapper was run from a clean checkout of that exact head:

scripts/run_tests.sh -q tests/hermes_cli/test_kanban_cli_exit_status.py
1 passed, 0 failed

git diff --check 8911e2e0edf750b104edbdc106d63d6cdac88524 7a22b9036f3aab34833e8fba5d025883750517a4
passed

This covers the delegated-child Kanban CLI process-exit regression. The candidate checkout remained clean after the run. The full repository suite was not run and is not claimed as passing.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (test(scope):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this regression coverage
  • I've run pytest tests/ -q and all tests pass — focused canonical coverage was run instead
  • I've added tests or improved test coverage
  • I've tested on Linux

Documentation & Housekeeping

  • No user documentation change is required
  • No config keys changed; cli-config.yaml.example is unaffected
  • No architecture or workflow change; CONTRIBUTING.md and AGENTS.md are unaffected
  • The change is test-only and has no additional cross-platform behavior
  • No tool descriptions or schemas changed

Provenance

  • Published head: 2416fee97f9c2d8950ffa674148841b2595b73c0
  • PR base: main at de0abc0617794a8c4ae661d2f67f4b23d4ab51fe
  • Source fork: vadelma-agent/hermes-agent

@vadelma-agent
vadelma-agent force-pushed the fix/kanban-cli-exit-status branch from e5bc232 to 6fcd4b7 Compare July 24, 2026 09:14
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jul 24, 2026
@vadelma-agent

vadelma-agent commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@teknium1 Could you please review the current test-only regression coverage for this Kanban CLI bug?

The production fix is now on upstream main (commit 397e9fc1e4): the top-level CLI propagates non-zero command-handler results, so a delegated-child Kanban mutation refusal exits with status 1 instead of 0. This PR was rebased to remove the now-redundant production change and retain process-boundary regression coverage.

PR #70667
Current head: 53d6cf2478bcd1eaae822d566b26b1a5385ef8f0

Test evidence on the current PR head:

  • process-boundary regression: 1 passed
  • Kanban CLI overlap suite: 47 passed
  • delegated-isolation suite: 16 passed
  • py_compile and git diff --check passed

The PR is currently test-only and awaits the fork workflow/CI gate.

@vadelma-agent
vadelma-agent force-pushed the fix/kanban-cli-exit-status branch 2 times, most recently from 8746434 to 53d6cf2 Compare July 24, 2026 17:50
@vadelma-agent vadelma-agent changed the title fix(kanban): preserve CLI mutation failure status test(kanban): cover delegated CLI refusal exit status Jul 24, 2026
@vadelma-agent
vadelma-agent force-pushed the fix/kanban-cli-exit-status branch 7 times, most recently from 4373670 to d9bbf56 Compare July 29, 2026 14:17

@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 preserving process-boundary coverage after the production behavior landed.

Problems

  • tests/hermes_cli/test_kanban_cli_exit_status.py:30 invokes subprocess.run() without a timeout. A CLI startup or dispatch regression can therefore hang this test worker indefinitely. The existing Kanban CLI helper sets timeout=30 at tests/hermes_cli/test_kanban_boards.py:300-306.

Suggested changes

  • Add a finite timeout (for example, timeout=30) to _run_hermes().

Current main already returns 1 from the Kanban delegated-child guard (hermes_cli/kanban.py:980-987) and forwards non-zero handler returns to the process exit code (hermes_cli/main.py:12406-12414). This test remains valuable because it exercises that complete CLI boundary.

Automated hermes-sweeper review.

env["HERMES_DELEGATED_CHILD_CONTEXT"] = "1"
else:
env.pop("HERMES_DELEGATED_CHILD_CONTEXT", None)
return subprocess.run(

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.

Please add a finite timeout (for example, timeout=30) here. The comparable Kanban CLI subprocess helper uses one at tests/hermes_cli/test_kanban_boards.py:300-306; without it, a startup or dispatch regression can hang the test worker indefinitely.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
@vadelma-agent
vadelma-agent force-pushed the fix/kanban-cli-exit-status branch from d9bbf56 to 133e48c Compare July 30, 2026 21:05
@vadelma-agent

Copy link
Copy Markdown
Contributor Author

Review follow-up on exact head 133e48c84adb67fb0b58976dd87ded5d19096de8 (rebased onto 4a798f4bce29302c9e981c877753e083f80fe533): I added timeout=30 to the shared _run_hermes() subprocess helper, so CLI startup/dispatch regressions cannot hang this test worker. Exact-head verification: the process-boundary regression, Kanban CLI overlap suite, delegated-isolation suite, py_compile, and git diff --check all pass. The updated PR body marks the previous SHA evidence as superseded.

@vadelma-agent
vadelma-agent force-pushed the fix/kanban-cli-exit-status branch 11 times, most recently from e915fa7 to 2bd7d95 Compare August 4, 2026 01:30
@vadelma-agent
vadelma-agent force-pushed the fix/kanban-cli-exit-status branch 27 times, most recently from 73362d9 to 7f8bcfc Compare August 9, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants