Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,29 @@ and versions are tracked in the repo-root `VERSION` file.

## [Unreleased]

### Added

- Add `ConfigurationError` so consumer profiles can explicitly mark
user-correctable configuration messages as safe usage errors.

### Changed

- Normalize command returns, Click errors, aborts, interrupts, `SystemExit`, and
unexpected exceptions through one core outcome model and clean `run_app()`
process boundary.
- Treat plain profile-callback exceptions as private internal failures. Profiles
that used `ValueError` for expected configuration problems should raise
`ConfigurationError` instead.

### Fixed

- Finalize core-owned run metadata for successful, failed, aborted, interrupted,
and unexpected command outcomes without letting secondary persistence
failures replace the command result.
- Roll back partially constructed command contexts without leaking handlers,
temporary directories, or incomplete run bundles.
- Preserve exception tracebacks in persistent logs and show them on stderr only
when debug output is enabled.
- Make history persistence best-effort so secondary failures cannot mask the
command outcome or skip cleanup, context reset, and logger shutdown.
- Allow finished history records to omit `log_path` when file logging is
Expand Down
55 changes: 53 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ command result meanings:
prevented successful completion.
- `ExitCode.USAGE_ERROR` (`2`): the command could not proceed because user
input, configuration, or environment setup was invalid or incomplete.
- `ExitCode.INTERRUPTED` (`130`): the user interrupted the command with
<kbd>Ctrl</kbd>+<kbd>C</kbd>.

Existing commands can keep returning integers. New code should prefer the named
constants when it makes intent clearer:
Expand All @@ -285,6 +287,37 @@ if ctx.project_root is None:
return base_cli.ExitCode.USAGE_ERROR
```

`run_app()` is the process boundary for production entry points. It preserves
Click's messages and exit codes for usage and application errors, reports an
explicit abort as `1`, and reports <kbd>Ctrl</kbd>+<kbd>C</kbd> during startup or
command execution as `130` without a traceback. After the command outcome has
settled, history, metadata, and cleanup are best-effort teardown: even a second
interrupt there cannot replace the primary result.

An unexpected exception returns `1` with a stable, detail-free message. The run
ID and diagnostic-log path are included when context and file logging are
available. The traceback is kept in the persistent log when enabled and is
shown on stderr with an effective `--debug` setting. A failure before option
parsing can provide a traceback only when `--debug` is an unambiguous leading
flag; otherwise the message says that diagnostic context was unavailable.
Tests or embedding code that need the original exception can pass
`reraise_unexpected=True`.

| Command result or exception | `outcome` | Exit code | Default message |
| --- | --- | ---: | --- |
| `None` or returned `0` | `success` | 0 | none |
| returned `2` | `usage_error` | 2 | none |
| another returned nonzero integer | `nonzero_return` | returned value | none |
| `click.UsageError` | `usage_error` | exception code | Click usage error |
| another `click.ClickException` | `click_error` | exception code | Click error |
| `click.Abort` | `aborted` | 1 | `Aborted!` |
| <kbd>Ctrl</kbd>+<kbd>C</kbd> | `interrupted` | 130 | `Interrupted.` |
| `SystemExit` | `system_exit` | normalized payload | string payload, if any |
| another unexpected exception | `unexpected_error` | 1 | stable internal-error message |

For `SystemExit`, a missing payload becomes `0`, an integer payload is preserved,
and any other payload is printed and normalized to `1`.

## Context

`Context` is the object command code should pass around instead of rediscovering
Expand Down Expand Up @@ -454,8 +487,26 @@ Windows uses `%LOCALAPPDATA%` (falling back to `~/AppData/Local`). Set
`BASE_CLI_CACHE_DIR` to override the default on any platform. The generic
profile does not prescribe a product-wide cache name or cleanup command.

Each invocation is a run bundle containing a private `run.json`, `logs/`, and
`tmp/`, while persistent component caches live in the bundle's cache directory.
Each lifecycle-owned invocation is a run bundle containing a private
`run.json`, `logs/`, and `tmp/`, while persistent component caches live in the
owner's cache directory. When persistence succeeds, `run.json` begins with
`status: "running"` and is finalized with `status`, `outcome`, `exit_code`,
`ended_at`, and `duration_ms`, including command failures and interruptions.
The stable outcome values are `success`, `usage_error`, `nonzero_return`,
`click_error`, `aborted`, `interrupted`, `system_exit`, and
`unexpected_error`. Terminal-write failures are warnings and cannot change the
process result; base-cli then removes a matching or corrupt owned record on a
best-effort basis so history data cannot masquerade as authoritative core data.

Parsing errors, help, and version requests occur before the command lifecycle
owns a bundle and therefore do not create one. Neither do inherited runtimes,
`log_to_file=False`, or dry-run invocations; an explicit log path can still
receive diagnostics in the latter two modes. Context startup is transactional:
if directory creation, logger setup, or retention fails, base-cli closes
partially installed handlers and removes new bundle-local temp/log artifacts
and empty directories. Pre-existing content, persistent component caches, and
parent-runtime data are preserved.

On POSIX, base-cli enforces owner-only `0600`/`0700` modes. On Windows, the
default user-local cache root relies on inherited user-profile ACLs; consumers
using a custom cache root must provide the appropriate ACL themselves.
Expand Down
23 changes: 23 additions & 0 deletions docs/cache-ownership-and-layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,29 @@ Each invocation has a private run bundle containing:
- `logs/` for diagnostic logs; and
- `tmp/` for temporary command data.

The core lifecycle, rather than an optional history adapter, owns `run.json`.
Once command context construction succeeds, the file is written with
`status: "running"`. When persistence succeeds, the core writes a terminal
snapshot containing `status`, `outcome`, `exit_code`, `ended_at`, and
`duration_ms`. Terminal status is `ok` only for exit code zero; all other exit
codes use `error`. The outcome discriminator is one of `success`,
`usage_error`, `nonzero_return`, `click_error`, `aborted`, `interrupted`,
`system_exit`, or `unexpected_error`.

History may enrich a matching record with consumer fields, but the core writes
the canonical lifecycle fields last. If terminal persistence fails, the
process keeps its primary result and the framework best-effort removes its
matching or corrupt record rather than leave history data or `running` state
looking authoritative. Writes are not yet promised to be atomic.

The ownership boundary intentionally excludes parser failures, help and version
requests, inherited runtime bindings, `log_to_file=False`, and dry-run mode.
Those invocations do not create or finalize a bundle. If context construction
fails after creating artifacts, rollback closes partial logging handlers and
removes new bundle-local temp/log artifacts and empty directories. It does not
delete pre-existing content, persistent component caches, paths outside the
selected run root, or a parent runtime's metadata.

Persistent component caches live under the owner's `cache/components/` path.
On POSIX systems, runtime directories are owner-only (`0700`) and runtime files
are owner-only (`0600`). On Windows, the default `%LOCALAPPDATA%` root relies
Expand Down
14 changes: 14 additions & 0 deletions docs/consumer-profiles.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,20 @@ translate internal entry-point names into user-facing labels. The generic
default only replaces underscores with hyphens; it does not know any product's
command aliases.

## Safe profile errors

Plain exceptions from profile callbacks are treated as unexpected internal
errors: production output hides their details, while `--debug` exposes the
traceback after option parsing. This prevents a programming error or a private
value in a callback from becoming user-facing output by accident.

For a user-correctable configuration problem whose message is safe to show,
raise `base_cli.ConfigurationError`; `run_app()` renders it as a Click usage
error with exit code `2`. A callback may raise `click.UsageError` or another
`click.ClickException` when it needs Click's standard rendering or a custom
exit code. Consumers that previously raised plain `ValueError` for expected
configuration failures should migrate those sites to `ConfigurationError`.

## Consumer-owned adapters

`App()` uses `CliProfile.generic()` when no profile is supplied. This keeps the
Expand Down
2 changes: 2 additions & 0 deletions lib/python/base_cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def _resolve_version() -> str:
register_record_schema,
)
from .context import Context, get_current_context
from .errors import ConfigurationError
from .exit_codes import ExitCode
from .inspection import inspection_envelope, render_inspection_json
from .logging import configure_logger, log_critical, log_debug, log_error, log_info, log_warning
Expand All @@ -66,6 +67,7 @@ def _resolve_version() -> str:
"CliProfile",
"CommandFilterNormalizer",
"CommandProtocolError",
"ConfigurationError",
"Context",
"ExitCode",
"FieldSpec",
Expand Down
165 changes: 165 additions & 0 deletions lib/python/base_cli/_lifecycle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
from __future__ import annotations

import json
from dataclasses import dataclass
from datetime import datetime
from typing import Any

from ._private_files import write_private_json
from .context import Context
from .exit_codes import ExitCode
from .history import format_timestamp


@dataclass(frozen=True)
class InvocationOutcome:
"""Private normalized result of one lifecycle-owned invocation."""

kind: str
status: str
exit_code: int


@dataclass(frozen=True)
class RunRecorder:
"""Write core-owned lifecycle snapshots for one Context."""

context: Context
started_at: datetime
started_monotonic_ns: int

def start(self) -> None:
if self.context._run_metadata_path is None:
return
write_private_json(
self.context._run_metadata_path,
self._metadata(status="running"),
)

def finish(
self,
outcome: InvocationOutcome,
*,
ended_at: datetime,
ended_monotonic_ns: int,
) -> None:
if self.context._run_metadata_path is None:
return
elapsed_ns = max(0, ended_monotonic_ns - self.started_monotonic_ns)
metadata = self._existing_metadata()
metadata.update(self._metadata(status=outcome.status))
metadata.update(
{
"outcome": outcome.kind,
"exit_code": outcome.exit_code,
"ended_at": format_timestamp(ended_at),
"duration_ms": round(elapsed_ns / 1_000_000),
}
)
write_private_json(self.context._run_metadata_path, metadata)

def _existing_metadata(self) -> dict[str, Any]:
path = self.context._run_metadata_path
if path is None:
return {}
try:
payload = json.loads(path.read_text(encoding="utf-8"))
except (OSError, UnicodeDecodeError, json.JSONDecodeError):
return {}
if not isinstance(payload, dict) or payload.get("run_id") != self.context.run_id:
return {}
return dict(payload)

def discard_owned_record(self) -> None:
"""Remove our misleading record after terminal persistence fails."""
path = self.context._run_metadata_path
if path is None:
return
try:
payload = json.loads(path.read_text(encoding="utf-8"))
if (
isinstance(payload, dict)
and payload.get("run_id") == self.context.run_id
):
path.unlink()
except (UnicodeDecodeError, json.JSONDecodeError):
try:
path.unlink()
except OSError:
pass
except OSError:
pass

def _metadata(self, *, status: str) -> dict[str, Any]:
context = self.context
return {
"schema_version": 1,
"run_id": context.run_id,
"owner": context.runtime_owner,
"cli": context.cli_name,
"status": status,
"started_at": format_timestamp(self.started_at),
"project": context.project_name,
"project_root": str(context.project_root) if context.project_root else None,
"manifest": str(context.manifest_path) if context.manifest_path else None,
"workspace_root": str(context.workspace_root) if context.workspace_root else None,
}


def outcome_from_exit_code(exit_code: int) -> InvocationOutcome:
if exit_code == ExitCode.SUCCESS:
return InvocationOutcome("success", "ok", exit_code)
if exit_code == ExitCode.USAGE_ERROR:
return InvocationOutcome("usage_error", "error", exit_code)
if exit_code == ExitCode.INTERRUPTED:
return InvocationOutcome("interrupted", "error", exit_code)
return InvocationOutcome("nonzero_return", "error", exit_code)


def outcome_from_exception(click: Any, exc: BaseException) -> InvocationOutcome:
if isinstance(exc, KeyboardInterrupt):
return InvocationOutcome("interrupted", "error", ExitCode.INTERRUPTED)
if isinstance(exc, EOFError):
return InvocationOutcome("aborted", "error", ExitCode.FAILURE)
if isinstance(exc, click.Abort):
if isinstance(exc.__cause__, KeyboardInterrupt):
return InvocationOutcome("interrupted", "error", ExitCode.INTERRUPTED)
return InvocationOutcome("aborted", "error", ExitCode.FAILURE)
if isinstance(exc, click.exceptions.Exit):
exit_code = _click_exception_exit_code(exc)
if exit_code is None:
return InvocationOutcome("unexpected_error", "error", ExitCode.FAILURE)
return outcome_from_exit_code(exit_code)
if isinstance(exc, click.UsageError):
exit_code = _click_exception_exit_code(exc)
if exit_code is None:
return InvocationOutcome("unexpected_error", "error", ExitCode.FAILURE)
return InvocationOutcome("usage_error", _status_for_exit_code(exit_code), exit_code)
if isinstance(exc, click.ClickException):
exit_code = _click_exception_exit_code(exc)
if exit_code is None:
return InvocationOutcome("unexpected_error", "error", ExitCode.FAILURE)
return InvocationOutcome("click_error", _status_for_exit_code(exit_code), exit_code)
if isinstance(exc, SystemExit):
exit_code = system_exit_code(exc)
return InvocationOutcome("system_exit", _status_for_exit_code(exit_code), exit_code)
return InvocationOutcome("unexpected_error", "error", ExitCode.FAILURE)


def system_exit_code(exc: SystemExit) -> int:
if exc.code is None:
return ExitCode.SUCCESS
if isinstance(exc.code, int):
return int(exc.code)
return ExitCode.FAILURE


def _status_for_exit_code(exit_code: int) -> str:
return "ok" if exit_code == ExitCode.SUCCESS else "error"


def _click_exception_exit_code(exc: Any) -> int | None:
try:
return int(exc.exit_code)
except BaseException: # pylint: disable=broad-exception-caught
return None
6 changes: 5 additions & 1 deletion lib/python/base_cli/_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ class RuntimeLayout:
_LOG_INDEX_NAME = ".base-cli-log-index.json"


class RuntimeDirectoryError(RuntimeError):
"""An actionable failure to create a framework-owned runtime directory."""


# pylint: disable=too-many-arguments
def runtime_layout(
cache_root: Path,
Expand Down Expand Up @@ -62,7 +66,7 @@ def create_runtime_directory(path: Path, cache_root: Path) -> None:
for directory in [path, *missing]:
restrict_directory(directory)
except OSError as exc:
raise RuntimeError(_runtime_directory_error(path, cache_root, exc)) from exc
raise RuntimeDirectoryError(_runtime_directory_error(path, cache_root, exc)) from exc


def runtime_namespace_root(cache_root: Path, namespace: str) -> Path:
Expand Down
Loading