chore: improve unstract maintenance path

[lphuc2250gma]

Summary:

• Add or tighten focused edge-case tests or type assertions in tests/rig/cli.py, tests/conftest.py related to Python typing, tests, CLI ergonomics, observability; avoid docs-only changes and broad refactors.
• Keep the change narrow so it is straightforward to review.

Notes:

• I kept this scoped to the relevant implementation and tests.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d7953a84-9019-4a52-9b0e-e70a171b3a30

📥 Commits

Reviewing files that changed from the base of the PR and between 2877bd8 and 9015501.

📒 Files selected for processing (2)

• tests/rig/cli.py
• tests/rig/tests/test_cli.py

---

Summary by CodeRabbit

• Bug Fixes
  • Fixed handling of the --timeout 0 command-line argument to correctly override the group's default timeout setting instead of being replaced by it.

Walkthrough

The PR fixes a bug where cmd_run incorrectly treated --timeout 0 as a falsy value and fell back to group defaults. The fix replaces the args.timeout or group.timeout_seconds logic with an explicit None check, so zero is honored as a valid override. A new test validates the fix.

Changes

Timeout Zero Override Fix

Layer / File(s)	Summary
Explicit None check for timeout argument tests/rig/cli.py, tests/rig/tests/test_cli.py	cmd_run now uses explicit None check (group.timeout_seconds if args.timeout is None else args.timeout) when passing timeout to _execute_group, respecting zero as a valid override. New test test_timeout_zero_is_respected validates --timeout 0 is honored by capturing the timeout kwarg and verifying it equals 0.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description provides high-level objectives but lacks specific details required by the template, such as What, Why, How, impact analysis, and testing notes.	Fill in the required template sections (What, Why, How, impact analysis, testing notes) with concrete details about the timeout fix and test addition.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title is vague and generic, using non-descriptive terms like 'chore' and 'maintenance path' that don't clearly convey the specific changes being made.	Replace with a specific, descriptive title that highlights the main change, e.g., 'fix: handle zero timeout value correctly in CLI group execution' or 'test: add test for zero timeout handling'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[greptile-apps]

Greptile Summary

This PR fixes a bug where --timeout 0 was silently overridden by the group's default timeout because the previous args.timeout or group.timeout_seconds expression treats 0 as falsy. The fix replaces it with an explicit is None sentinel check, and adds a focused regression test that monkeypatches _execute_group to confirm the value is forwarded unchanged.

• tests/rig/cli.py: Single-line change replacing args.timeout or group.timeout_seconds with group.timeout_seconds if args.timeout is None else args.timeout, matching argparse's None-when-absent contract for type=int arguments.
• tests/rig/tests/test_cli.py: Adds test_timeout_zero_is_respected — patches the manifest, registry, and _execute_group; asserts captured["timeout"] == 0 after a cmd_run invocation with --timeout 0.

Confidence Score: 5/5

Safe to merge — the change is a one-line sentinel fix with no side effects on the normal path and a new test that directly exercises the previously broken case.

The fix is minimal and surgical: it corrects one expression, the argparse default=None contract is well-established, and the new test reproduces the exact failure mode (zero being swallowed by the or) and asserts the repaired behavior end-to-end through cmd_run. No other code paths are touched.

No files require special attention.

Important Files Changed

Filename	Overview
tests/rig/cli.py	Replaces falsy or-based timeout fallback with an explicit is None check, correctly allowing --timeout 0 to propagate to _execute_group.
tests/rig/tests/test_cli.py	Adds test_timeout_zero_is_respected which monkeypatches _execute_group to verify --timeout 0 is forwarded as 0 and not silently overridden by the group default.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CLI: parse_args()"] --> B{"args.timeout is None?"}
    B -- "Yes (--timeout not supplied)" --> C["use group.timeout_seconds"]
    B -- "No (--timeout 0 or N)" --> D["use args.timeout (0 or N)"]
    C --> E["_execute_group(..., timeout=group.timeout_seconds)"]
    D --> E
    E --> F["_spawn(cmd, timeout=T)\nsubprocess.run(..., timeout=T+30)"]
    E --> G["pytest --timeout=T"]

Loading

_{Reviews (1): Last reviewed commit: "chore: improve unstract maintenance path" | Re-trigger Greptile}