diff --git a/almanac/architecture/lifecycle/operation-runner.md b/almanac/architecture/lifecycle/operation-runner.md index 42d3be4a..b5508007 100644 --- a/almanac/architecture/lifecycle/operation-runner.md +++ b/almanac/architecture/lifecycle/operation-runner.md @@ -10,6 +10,10 @@ sources: type: file path: src/codealmanac/workflows/operations/harness.py note: Harness result validation and event classification. + - id: operation-failure + type: file + path: src/codealmanac/workflows/operations/failure.py + note: Terminal failed-run transition for operations. - id: operation-models type: file path: src/codealmanac/workflows/operations/models.py @@ -44,7 +48,9 @@ If a harness returns no event stream, the harness helper creates a terminal fall After recording harness output, the runner validates harness success [@operation-service] [@operation-harness]. A failed harness therefore leaves its transcript and output in the run log before the run is marked failed. -The runner then refreshes the index, validates the wiki through `HealthService.ensure_valid`, and finishes the run as `DONE` with the harness summary or operation success summary [@operation-service]. Each step runs inside an explicit failure phase. A failed readiness check becomes `harness_readiness`; an adapter exception or normalized failed result becomes `provider_execution`; index refresh becomes `indexing`; final health validation becomes `wiki_validation`; and event-sink or run machinery failures become `internal_error` [@operation-service]. `fail` computes a non-throwing one-line summary, then best-efforts the readable error event and authoritative terminal transition independently. It records the supplied phase rather than inferring durable analytics from exception classes or traceback modules. +The runner then refreshes the index, validates the wiki through `HealthService.ensure_valid`, and finishes the run as `DONE` with the harness summary or operation success summary [@operation-service]. Each step runs inside an explicit failure phase. A failed readiness check becomes `harness_readiness`; an adapter exception or normalized failed result becomes `provider_execution`; index refresh becomes `indexing`; final health validation becomes `wiki_validation`; and event-sink or run machinery failures become `internal_error` [@operation-service]. `fail` computes a non-throwing one-line summary, best-efforts the readable error event, then writes the authoritative terminal transition. It records the supplied phase rather than inferring durable analytics from exception classes or traceback modules. + +The two writes are not equally optional. A failed error-event write is diagnostic noise and never blocks the terminal transition, but a failed transition raises `ExecutionFailed` naming the run, the original failure, and the bookkeeping error [@operation-failure]. A run left non-terminal with no record of why is worse than a noisy failure. The readiness and invocation calls are the stage distinction: missing, unavailable, or failed readiness checks are `harness_readiness`; once readiness passes, adapter exceptions are `provider_execution` regardless of their exception class. `HarnessEventSinkFailed` preserves the one exception to that stage rule by identifying caller-owned event persistence as `internal_error` [@operation-service]. Ingest source resolution and runtime inspection occur before shared execution, so the ingest workflow marks those scopes as `source_preparation` while still delegating the actual failure write to `OperationRunner`. diff --git a/almanac/architecture/wiki/health-and-validation.md b/almanac/architecture/wiki/health-and-validation.md index b7a094ee..8cbe153c 100644 --- a/almanac/architecture/wiki/health-and-validation.md +++ b/almanac/architecture/wiki/health-and-validation.md @@ -26,6 +26,14 @@ sources: type: file path: src/codealmanac/services/health/runtime.py note: Runtime-state leak checks. + - id: health-topics + type: file + path: src/codealmanac/services/health/topics.py + note: topics.yaml readability check. + - id: index-sources + type: file + path: src/codealmanac/services/index/sources.py + note: Index source loading and the page route collision error. - id: validate-tests type: file path: tests/test_validate.py @@ -34,7 +42,7 @@ sources: # Health And Validation -Health and validation are the wiki's safety checks. `health` reports problems found in the indexed page graph, while `validate` turns those checks into a pass/fail boundary for commands and lifecycle runs [@health-service]. Validation also checks source frontmatter shape and runtime-state leaks before relying on the index-backed health report [@health-service] [@health-sources] [@health-runtime]. +Health and validation are the wiki's safety checks. `health` reports problems found in the indexed page graph, while `validate` turns those checks into a pass/fail boundary for commands and lifecycle runs [@health-service]. Validation also checks source frontmatter shape, `topics.yaml` readability, and runtime-state leaks before relying on the index-backed health report [@health-service] [@health-sources] [@health-topics] [@health-runtime]. This matters because the committed `almanac/` tree is source. A lifecycle agent can write Markdown, but the run is not successful until the wiki can be indexed and validated [@validate-tests]. The same validation path is available to users before they commit wiki edits. @@ -42,7 +50,7 @@ This matters because the committed `almanac/` tree is source. A lifecycle agent `HealthService.check` selects a repository and returns the index service's health report [@health-service]. It is a read-side diagnostic, consistent with the read/write split described in [Service boundaries](../service-boundaries). -`HealthService.validate` is stricter. It gathers source shape issues and runtime-state issues, then calls `index.ensure_fresh`. If index refresh fails, the validation result records a `page_routes` issue and stops before running index health queries [@health-service]. If refresh succeeds, validation adds issues derived from the health report [@health-service]. +`HealthService.validate` is stricter. It gathers source shape issues, `topics.yaml` issues, and runtime-state issues, then calls `index.ensure_fresh`. If index refresh fails, the validation result records the failure and stops before running index health queries: a `PageRouteCollision` becomes a `page_routes` issue and every other `ValidationFailed` becomes an `index_sources` issue [@health-service] [@index-sources]. If refresh succeeds, validation adds issues derived from the health report [@health-service]. `ensure_valid` wraps the same validation path and raises `ValidationFailed` when any issue exists [@health-service]. `OperationRunner` uses that method after a lifecycle agent writes pages, so invalid wiki output fails the run instead of being marked done. @@ -58,6 +66,8 @@ Source hygiene comes from a separate index view. Citation IDs are parsed from in Some checks happen before or outside the indexed report. Source shape validation opens every page and verifies that `sources:` is a list and that each source entry has an `id`, a supported `type`, and a target field such as `path` for file evidence [@health-sources]. +Topic-file validation reads `topics.yaml` through the same lenient reader the index uses. Read commands keep working when that file is malformed or unreadable, so the reader returns no topics and carries the parse error instead of raising; validation turns that error into a `topics_file` issue so the dropped topics are never silent [@health-topics]. + Runtime-state validation rejects local state files and directories inside `almanac/`. The blocked names are `index.db`, `index.db-wal`, `index.db-shm`, `jobs`, and `runs`; the message points back to `~/.codealmanac/` as the correct runtime home [@health-runtime]. Route collisions are handled through index refresh. If two files map to the same page route, validation records a `page_routes` issue instead of leaking a traceback [@validate-tests]. diff --git a/almanac/architecture/wiki/topics-dag.md b/almanac/architecture/wiki/topics-dag.md index ca9994cc..c2c36307 100644 --- a/almanac/architecture/wiki/topics-dag.md +++ b/almanac/architecture/wiki/topics-dag.md @@ -18,6 +18,10 @@ sources: type: file path: src/codealmanac/services/wiki/topic_file.py note: Round-trip topics.yaml mutation path. + - id: topic-read + type: file + path: src/codealmanac/services/wiki/topic_read.py + note: Lenient topics.yaml reader that carries its parse error. - id: topic-tests type: file path: tests/test_topics_mutation.py @@ -38,6 +42,8 @@ This area is split into read and write responsibilities. `TopicsService` is the Topic reads come from the index. `list` selects the repository and returns indexed topic summaries, while `show` kebab-cases the requested slug and asks the index for a `TopicDetail` [@topics-service]. This means callers do not parse `topics.yaml` directly during normal reads. +The reader behind the index is deliberately lenient. A malformed or unreadable `topics.yaml` yields no topic definitions instead of breaking every read command, and the parse error travels with that empty result so `validate` can report it as a `topics_file` issue [@topic-read]. Mutation commands keep the stricter contract and refuse to rewrite a file they could not parse [@topic-file]. + The index can see two kinds of topic evidence. A topic can be defined in `topics.yaml`, or it can appear only in page frontmatter. Mutation commands call the index for existing topic slugs before deciding whether a topic exists, which lets commands promote page-only topics into explicit YAML entries when needed [@topic-mutations] [@topic-tests]. ## Mutation Flow diff --git a/src/codealmanac/services/health/service.py b/src/codealmanac/services/health/service.py index 4b84ce24..942cb9c3 100644 --- a/src/codealmanac/services/health/service.py +++ b/src/codealmanac/services/health/service.py @@ -9,8 +9,10 @@ from codealmanac.services.health.requests import HealthCheckRequest, ValidateWikiRequest from codealmanac.services.health.runtime import runtime_state_issues from codealmanac.services.health.sources import source_shape_issues +from codealmanac.services.health.topics import topics_file_issues from codealmanac.services.index.models import HealthReport, IndexRefreshResult from codealmanac.services.index.service import IndexService +from codealmanac.services.index.sources import PageRouteCollision from codealmanac.services.repositories.models import Repository, RepositoryName from codealmanac.services.repositories.service import RepositoriesService @@ -31,6 +33,7 @@ def validate(self, request: ValidateWikiRequest) -> ValidationResult: def validate_repository(self, repository: Repository) -> ValidationResult: issues = [ *source_shape_issues(repository.almanac_path), + *topics_file_issues(repository.almanac_path), *runtime_state_issues(repository.almanac_path), ] index = None @@ -39,7 +42,7 @@ def validate_repository(self, repository: Repository) -> ValidationResult: except ValidationFailed as error: issues.append( ValidationIssue( - category="page_routes", + category=index_issue_category(error), message=str(error), path=repository.almanac_root.as_posix(), ) @@ -64,6 +67,12 @@ def select_repository( return self.repositories.select_for_read(cwd, repository_name) +def index_issue_category(error: ValidationFailed) -> str: + if isinstance(error, PageRouteCollision): + return "page_routes" + return "index_sources" + + def validation_result( repository: Repository, index: IndexRefreshResult | None, diff --git a/src/codealmanac/services/health/topics.py b/src/codealmanac/services/health/topics.py new file mode 100644 index 00000000..97722c20 --- /dev/null +++ b/src/codealmanac/services/health/topics.py @@ -0,0 +1,17 @@ +from pathlib import Path + +from codealmanac.services.health.models import ValidationIssue +from codealmanac.services.wiki.topics import read_topics_yaml + + +def topics_file_issues(almanac_path: Path) -> tuple[ValidationIssue, ...]: + error = read_topics_yaml(almanac_path).error + if error is None: + return () + return ( + ValidationIssue( + category="topics_file", + message=f"topics.yaml could not be read: {error}", + path="topics.yaml", + ), + ) diff --git a/src/codealmanac/services/index/sources.py b/src/codealmanac/services/index/sources.py index 986d5716..906f3e3e 100644 --- a/src/codealmanac/services/index/sources.py +++ b/src/codealmanac/services/index/sources.py @@ -13,6 +13,10 @@ from codealmanac.services.wiki.topics import TopicDefinition, load_topics_yaml +class PageRouteCollision(ValidationFailed): + """Two page files claim the same wiki route.""" + + class LoadedIndexSources(CodeAlmanacModel): documents: tuple[PageDocument, ...] topics: tuple[TopicDefinition, ...] @@ -61,7 +65,7 @@ def load_documents(almanac_path: Path) -> tuple[list[PageDocument], int, int]: continue if document.slug in seen_slugs: first = seen_slugs[document.slug] - raise ValidationFailed( + raise PageRouteCollision( "page route collision: " f"{document.slug} maps to both {first} and {page_path}" ) @@ -75,5 +79,7 @@ def file_hash(path: Path) -> str: return sha256(b"").hexdigest() try: return sha256(path.read_bytes()).hexdigest() - except OSError: - return sha256(b"").hexdigest() + except OSError as error: + # An unreadable file must not hash like a missing one, or the index + # would report itself fresh while the file's content stays unread. + return sha256(f"unreadable:{error.__class__.__name__}".encode()).hexdigest() diff --git a/src/codealmanac/services/wiki/topic_read.py b/src/codealmanac/services/wiki/topic_read.py index c723ccb2..7c66a954 100644 --- a/src/codealmanac/services/wiki/topic_read.py +++ b/src/codealmanac/services/wiki/topic_read.py @@ -4,21 +4,34 @@ from pydantic import ValidationError from yaml import YAMLError +from codealmanac.core.errors import error_summary +from codealmanac.core.models import CodeAlmanacModel from codealmanac.services.wiki.topic_models import TopicDefinition, TopicsYaml -def load_topics_yaml(almanac_path: Path) -> tuple[TopicDefinition, ...]: +class TopicsYamlRead(CodeAlmanacModel): + """Reads stay lenient; the parse error travels with the empty result.""" + + topics: tuple[TopicDefinition, ...] = () + error: str | None = None + + +def read_topics_yaml(almanac_path: Path) -> TopicsYamlRead: path = almanac_path / "topics.yaml" if not path.is_file(): - return () + return TopicsYamlRead() try: parsed = pyyaml.safe_load(path.read_text(encoding="utf-8")) or {} - except (OSError, YAMLError): - return () + except (OSError, YAMLError) as error: + return TopicsYamlRead(error=error_summary(error)) if not isinstance(parsed, dict): - return () + return TopicsYamlRead(error="topics.yaml must be a YAML mapping") try: model = TopicsYaml.model_validate(parsed) - except ValidationError: - return () - return tuple(topic for topic in model.topics if topic.slug) + except ValidationError as error: + return TopicsYamlRead(error=error_summary(error)) + return TopicsYamlRead(topics=tuple(topic for topic in model.topics if topic.slug)) + + +def load_topics_yaml(almanac_path: Path) -> tuple[TopicDefinition, ...]: + return read_topics_yaml(almanac_path).topics diff --git a/src/codealmanac/services/wiki/topics.py b/src/codealmanac/services/wiki/topics.py index 5a0e2b99..cbdc1c56 100644 --- a/src/codealmanac/services/wiki/topics.py +++ b/src/codealmanac/services/wiki/topics.py @@ -4,13 +4,19 @@ TopicsYaml, title_for_slug, ) -from codealmanac.services.wiki.topic_read import load_topics_yaml +from codealmanac.services.wiki.topic_read import ( + TopicsYamlRead, + load_topics_yaml, + read_topics_yaml, +) __all__ = [ "TopicDefinition", "TopicsYaml", "TopicsYamlFile", + "TopicsYamlRead", "load_topics_file", "load_topics_yaml", + "read_topics_yaml", "title_for_slug", ] diff --git a/src/codealmanac/workflows/operations/failure.py b/src/codealmanac/workflows/operations/failure.py new file mode 100644 index 00000000..d691c7f2 --- /dev/null +++ b/src/codealmanac/workflows/operations/failure.py @@ -0,0 +1,26 @@ +from codealmanac.core.errors import ExecutionFailed, error_summary +from codealmanac.services.runs.models import RunFailureCategory, RunStatus +from codealmanac.services.runs.requests import FinishRunRequest +from codealmanac.services.runs.service import RunsService + + +def finish_failed_run( + runs: RunsService, + run_id: str, + message: str, + category: RunFailureCategory, +) -> None: + try: + runs.finish( + FinishRunRequest( + run_id=run_id, + status=RunStatus.FAILED, + error=message, + failure_category=category, + ) + ) + except Exception as error: + raise ExecutionFailed( + f"run {run_id} failed ({message}) and could not be marked failed: " + f"{error_summary(error)}" + ) from error diff --git a/src/codealmanac/workflows/operations/service.py b/src/codealmanac/workflows/operations/service.py index 7a3b54ff..b3b48bb3 100644 --- a/src/codealmanac/workflows/operations/service.py +++ b/src/codealmanac/workflows/operations/service.py @@ -25,6 +25,7 @@ RecordRunHarnessTranscriptRequest, ) from codealmanac.services.runs.service import RunsService +from codealmanac.workflows.operations.failure import finish_failed_run from codealmanac.workflows.operations.harness import ( harness_events, harness_run_event_kind, @@ -174,6 +175,8 @@ def fail( category: RunFailureCategory, ) -> None: message = error_summary(error) + # The event log is a diagnostic; the terminal transition carries the + # same message, so a failed event write must not block it. with suppress(Exception): self.record( RecordOperationEventRequest( @@ -182,15 +185,7 @@ def fail( message=message, ) ) - with suppress(Exception): - self.runs.finish( - FinishRunRequest( - run_id=context.run_id, - status=RunStatus.FAILED, - error=message, - failure_category=category, - ) - ) + finish_failed_run(self.runs, context.run_id, message, category) @contextmanager def failure_phase( diff --git a/tests/test_ingest_workflow.py b/tests/test_ingest_workflow.py index 032c8dc0..e7192d6c 100644 --- a/tests/test_ingest_workflow.py +++ b/tests/test_ingest_workflow.py @@ -709,6 +709,44 @@ def test_operation_failure_finishes_when_error_event_write_fails( assert stored.error == "invalid source" +def test_operation_failure_surfaces_unrecorded_terminal_transition( + tmp_path: Path, + isolated_home: Path, + monkeypatch, +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + (repo / "note.md").write_text("auth decision\n", encoding="utf-8") + app = create_app( + AppConfig(database_path=isolated_home / ".codealmanac/codealmanac.db") + ) + initialize_repository(app, path=repo) + operations, context, queued = begin_ingest_operation( + app, + IngestRequest( + cwd=repo, + inputs=("note.md",), + harness=HarnessKind.CODEX, + model="gpt-5.5", + ), + ) + monkeypatch.setattr( + operations.runs, + "finish", + lambda _request: (_ for _ in ()).throw(OSError("database is locked")), + ) + + with pytest.raises(ExecutionFailed, match="could not be marked failed"): + operations.fail( + context, + ValidationFailed("invalid source"), + RunFailureCategory.SOURCE_PREPARATION, + ) + + stored = app.runs.show(ShowRunRequest(run_id=queued.run_id)) + assert stored.status == RunStatus.RUNNING + + def test_operation_failure_handles_broken_exception_stringification( tmp_path: Path, isolated_home: Path, diff --git a/tests/test_read_model.py b/tests/test_read_model.py index 4a0e07e9..58e32282 100644 --- a/tests/test_read_model.py +++ b/tests/test_read_model.py @@ -7,6 +7,7 @@ from codealmanac.app import create_app from codealmanac.core.errors import NotFoundError, ValidationFailed from codealmanac.services.index.requests import ReindexRequest +from codealmanac.services.index.sources import file_hash from codealmanac.services.pages.requests import ShowPageRequest from codealmanac.services.repositories.requests import ( RegisterRepositoryRequest, @@ -572,6 +573,26 @@ def test_ensure_fresh_skips_unchanged_projection_and_refreshes_edits( assert [row.slug for row in rows] == ["note"] +def test_unreadable_source_file_does_not_hash_like_a_missing_one( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + present = tmp_path / "topics.yaml" + present.write_text("topics: []\n", encoding="utf-8") + readable_hash = file_hash(present) + missing_hash = file_hash(tmp_path / "absent.yaml") + monkeypatch.setattr( + Path, + "read_bytes", + lambda _self: (_ for _ in ()).throw(PermissionError("denied")), + ) + + unreadable_hash = file_hash(present) + + assert unreadable_hash != missing_hash + assert unreadable_hash != readable_hash + + def test_reindex_forces_projection_rebuild_when_index_is_fresh( tmp_path: Path, isolated_home: Path, diff --git a/tests/test_validate.py b/tests/test_validate.py index d5441220..be312df4 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -106,6 +106,26 @@ def test_validate_reports_invalid_source_shape( } +def test_validate_reports_unreadable_topics_file( + tmp_path: Path, + isolated_home: Path, +): + repo = initialized_repo(tmp_path, isolated_home) + (repo / "almanac/topics.yaml").write_text("topics: [", encoding="utf-8") + app = create_app( + AppConfig(database_path=isolated_home / ".codealmanac/codealmanac.db") + ) + + result = app.health.validate(ValidateWikiRequest(cwd=repo)) + + assert result.ok is False + topics_issues = [ + issue for issue in result.issues if issue.category == "topics_file" + ] + assert [issue.path for issue in topics_issues] == ["topics.yaml"] + assert topics_issues[0].message.startswith("topics.yaml could not be read:") + + def test_validate_reports_runtime_state_leaks( tmp_path: Path, isolated_home: Path,