Skip to content

Address Claude review findings in base-cli #7

Description

@codeforester

Summary

Claude's review of base-cli identified two confirmed correctness bugs, one runtime-permission issue, test-fixture behavior that can invalidate realistic command tests, documentation gaps, duplicated security/dependency logic, portability/performance concerns, and several framework-scope decisions.

Correctness and security

  • Fix version resolution in lib/python/base_cli/__init__.py so an unrelated ancestor VERSION file cannot override the installed package version. Preserve checkout behavior and add an installed/nested-project regression test.
  • Validate command return values in app.py: accept only None or an integer/ExitCode, and raise a clear framework error for invalid values. Cover both command wrappers and run_app().
  • Create runtime directories with mode 0700, including when created under a permissive umask. Add permission tests.

Tests and documentation

  • Make base_cli.testing.invoke(cwd=...) change the process working directory as well as the framework override, restoring it reliably afterward. Test Path.cwd() and relative file access.
  • Repair the dead README links to docs/local-config.md and docs/cache-ownership-and-layout.md, or restore the missing documentation.
  • Add regression coverage for the above fixes and a packaged-README/link validation check.

Cleanup and maintainability

  • Centralize the repeated PyYAML dependency guard in a shared _require_yaml() helper.
  • Centralize private JSON writes in a helper that always enforces 0600, then replace the duplicated write/chmod patterns.

Portability and performance

  • Decide how to handle history-file locking on Windows: implement an msvcrt fallback or document the concurrency limitation explicitly.
  • Evaluate and optimize prune_log_files, which rescans the full historical run tree on every invocation.

Framework scope and adoption

  • Decide whether command_protocol should support downstream record schemas via a public registration API, or be documented as Base-specific.
  • Remove or isolate Base-specific ide_schema.py, IDE config/export behavior, and _BASE_DISPLAY_COMMANDS if this package is intended to be generic.
  • Improve color defaults so interactive TTYs are colored by default while honoring NO_COLOR and explicit opt-out.
  • Add or document a shell-completion workflow.

The review found the core lifecycle, context, logging, output, and redaction machinery solid; this issue tracks the remaining correctness, extraction, and adoption work.

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething is not working

Type

No type

Projects

  • Status
    Done

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions