Skip to content

fix(warehouse): surface out-of-scope dirty file count in status/contribute#148

Merged
Shadowsong27 merged 2 commits into
mainfrom
feat/per-159-warehouse-cli-untracked-notice
May 18, 2026
Merged

fix(warehouse): surface out-of-scope dirty file count in status/contribute#148
Shadowsong27 merged 2 commits into
mainfrom
feat/per-159-warehouse-cli-untracked-notice

Conversation

@Shadowsong27
Copy link
Copy Markdown
Owner

Summary

abc warehouse status and abc warehouse contribute filter git status through the project's beacon.yaml-tracked set — so warehouse files outside that scope were silently invisible. Users saw "✓ Working tree is clean." / "No uncommitted changes to contribute." while git status in the warehouse showed dirty files, and concluded the CLI was broken.

This PR adds a dirty_outside_scope_count field to StatusResult and ContributeResult, computes it via line-count subtraction of unfiltered vs filtered git status --porcelain, and surfaces it in both CLI handlers when the in-scope set is clean.

Closes PER-159.

Behavior matrix

State Before After
Clean warehouse ✓ Working tree is clean. unchanged
In-scope dirty files lists them unchanged
Out-of-scope dirty files only ✓ Working tree is clean. (misleading) Note: N dirty file(s) ... outside this project's beacon.yaml scope.
Mixed in-scope + out-of-scope lists in-scope only unchanged (in-scope ones still shown; user sees --all to inspect the rest)
--all mode shows everything unchanged (notice not added — already comprehensive)
abc warehouse status <path> single-file diff unchanged
abc warehouse contribute --paths <p> (out-of-scope path) explicit error unchanged

Test plan

  • uv run pytest libs/beacon/tests/unit/warehouse/test_status.py libs/beacon/tests/unit/warehouse/test_contribute.py -q — 29 passed
  • uv run pytest libs/beacon/tests/unit -q — 1047 passed, 2 skipped
  • Live smoke against ~/Code/knowledge/hl-knowledge-market warehouse — both commands now show the count
  • Clean-tree smoke — still ✓ Working tree is clean.
  • --all smoke — still surfaces everything
  • CI green
  • opencode-review bot clean

🤖 Generated with Claude Code

…ibute

Both `abc warehouse status` (no-arg) and `abc warehouse contribute` filter
git status through the project's beacon.yaml-tracked set, so warehouse files
outside that scope were invisible. Users saw "clean" / "no changes" while
`git status` inside the warehouse showed dirty files, and concluded the CLI
was broken.

Add a `dirty_outside_scope_count` to `StatusResult` and `ContributeResult`
and surface it in both CLI handlers when the in-scope set is clean but the
warehouse has out-of-scope dirty files. Existing in-scope behavior unchanged.

Closes PER-159.
@github-actions
Copy link
Copy Markdown
Contributor

OpenCode Review

Model: openai/gpt-5.5
Reviewed commit: 76d09857078b677fdbb08b2463960df72843c397
Workflow run: https://github.com/Shadowsong27/agentic-beacon/actions/runs/26025508017

Findings

  • [Medium] Default CLI path filtering may suppress the new warning - libs/beacon/src/beacon/domains/warehouse/contribute.py:78
    dirty_outside_scope_count is only computed when paths is None, but Click commonly passes no variadic args as an empty tuple. If the CLI forwards that tuple, default abc warehouse contribute will return 0 and never show the intended out-of-scope warning. Normalize empty paths to None at the CLI/domain boundary or test the actual CLI default path.

  • [Low] Tests don't cover CLI output for the new UX - libs/beacon/tests/unit/warehouse/test_status.py:183
    The changed behavior is user-visible in warehouse.py, but the added tests only assert domain result fields. Add CLI-level coverage for abc warehouse status and abc warehouse contribute showing the note when scoped results are clean but the warehouse has out-of-scope dirty files.

Open Questions

  • None.

Notes

  • Review limited to the attached diff.

Token Usage

Input tokens: 12462
Output tokens: 217
Reasoning tokens: 516
Cache read tokens: 1536
Cache write tokens: 0
Total tokens: 14731

@Shadowsong27
Copy link
Copy Markdown
Owner Author

Supervisor triage — bot round 1

Bot review: 76d09857078b677fdbb08b2463960df72843c397 (review run 26025508017)

Findings

  • [Medium] Default CLI path filtering may suppress the new warninglibs/beacon/src/beacon/domains/warehouse/contribute.py:78

    • Decision: acknowledged, false alarm — see evidence.
    • Evidence: the CLI handler (libs/beacon/src/beacon/cli/warehouse.py:415) explicitly normalizes Click's multiple=True empty tuple to None before calling the domain:
      result = contribute(
          project_root=Path.cwd(),
          message=message,
          push=push,
          paths=tuple(paths) or None,   # ← () is falsy, becomes None
      )
      The domain layer ALSO defensively rejects paths=() with an explicit ValueError (contribute.py:67-70), so the "empty-tuple passed through" code path the bot worried about cannot reach the dirty_outside_scope_count branch.
    • Verified live during deep review:
      $ abc warehouse contribute -m "smoke"
      Note: 1 dirty file(s) in warehouse outside this project's beacon.yaml scope.
      Run 'abc warehouse status --all' to see them, or contribute from a project that tracks them.
      
      (Documented in the PR body's "Test plan" check.)
  • [Low] Tests don't cover CLI output for the new UXlibs/beacon/tests/unit/warehouse/test_status.py:183

    • Decision: filed as PER-192 (Low). Domain-level tests + the live smoke (PR body) cover correctness; CLI-level coverage is a regression-net upgrade we can layer later.

Notes / residual risks (bot's "Notes" section)

  • "Review limited to the attached diff." — acked; no additional context the bot missed.

No fix-up needed. Handing off to UI merge.

@Shadowsong27 Shadowsong27 merged commit 96ce01c into main May 18, 2026
9 checks passed
@Shadowsong27 Shadowsong27 deleted the feat/per-159-warehouse-cli-untracked-notice branch May 18, 2026 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant