DeusData · umi008 · Aug 21, 2026
@@ -0,0 +1,45 @@
+## Context
+
+See proposal.md. Verified state:
+
+- `export_after_publish` (src/pipeline/pipeline.c:2492) runs AFTER the DB publish/rename (pipeline.c:2602-2613): the graph DB is already live at `final_path` when the artifact export runs. On failure it logs `pipeline.err` (phase `artifact_export`, err = `cbm_artifact_export_last_error()`) and returns the non-zero rc, which `cbm_pipeline_run` propagates (pipeline.c:2615-2618).
+- `handle_index_repository` (src/mcp/mcp.c:8335-8351): `rc != 0` ⇒ `status: "error"` + the fixed hint "Pipeline failed. Check repo_path exists and contains source files. Try mode='fast'..." — written regardless of where in the pipeline the failure occurred.
+- The fail-hard policy is an in-tree, cited decision ("A failed persistence export intentionally fails the run; this used to be ignored.", pipeline.c; commit 44d51745 "fix: address review feedback on artifact export error surfacing"). This change does NOT touch that policy.
+- `cbm_artifact_export_last_error()` (src/pipeline/artifact.c:67) is global state — it can carry a PREVIOUS run's error, so the response must not consult it directly; the pipeline must snapshot the error of the run it just executed.
+
+## Goals / Non-Goals
+
+**Goals:**
+- When a run fails post-publish specifically at the artifact export, the `index_repository` error hint names the real cause (artifact_export, the directory, the remedy: writable checkout or `--persistence false`), and notes the index was published.
+- All other failures keep the existing hint byte-for-byte.
+- The error surfaced is from the current run (snapshot on the pipeline), never stale global state.
+
+**Non-Goals:**
+- Changing the fail-hard semantics (non-zero rc on export failure stays).
+- Changing the success path, `artifact_present`, or any non-error surface.
+- Moving the artifact target elsewhere (a cache-dir fallback would be a maintainer-level design decision; not taken).
+
+## Decisions
+
+**D1 — Snapshot on the pipeline struct.**
+`cbm_pipeline_t` (defined in pipeline_internal.h) gains `char export_error[CBM_SZ_1K]`, zeroed at `cbm_pipeline_run` start; `export_after_publish` writes `cbm_artifact_export_last_error()` (or "unknown") into it on failure. Accessor `cbm_pipeline_export_error()` declared in pipeline.h returns NULL-safe `export_error`. Rationale: the MCP handler already holds `p`; no global-state races (pipeline runs happen in workers or in-process).
+
+**D2 — MCP hint selection by rc + snapshot only.**
+In `handle_index_repository`'s error branch: if `cbm_pipeline_export_error(p)` is non-empty AND non-zero rc, emit the truthful artifact hint via the pipeline's `pipeline.err` phase knowledge (the snapshot is only ever set by `export_after_publish`, so its presence identifies the phase exactly). Otherwise the existing generic hint is unchanged, preserving the spec's "genuine pipeline failure" scenario.
+
+**D3 — Test at the pipeline+store level, not full MCP.**
+A full read-only-repo `index_repository` run in tests is heavy and platform-dependent (root can write anywhere; EROFS needs a real read-only mount). Instead the regression test drives `cbm_pipeline_run` with `persistence` enabled against a fixture repo whose artifact directory is replaced by a read-only directory (`chmod 0555` on the `.codebase-memory` parent — works for non-root; skipped politely when running as root, where it is not meaningful). Asserts: `rc != 0`, `cbm_pipeline_export_error(p)` non-empty and mentioning the artifact directory, and the pipeline error log contains `pipeline.err`.
+
+## Risks / Trade-offs
+
+- [chmod-based read-only fixture flaky under root/CI] → Test skips when `geteuid() == 0` (root bypasses permission checks); CI runs the suite as non-root in this repo's matrix.
+- [Hint wording churn] → The truthful-hint string is new; existing tests asserting the exact generic hint on non-export failures keep passing because the snapshot is empty there (spec scenario 2 pins this).
+- [Adding a field to `cbm_pipeline_t`] → Internal struct, single definition; accessor keeps the public header minimal.
+
+## Migration Plan
+
+No data/config migration. Behavior: only the error message of an already-failing run becomes accurate. Rollback: revert.
+
+## Open Questions
+
+None.
@@ -0,0 +1,24 @@
+## Why
+
+`index_repository` on a read-only repository with `--persistence true` fails after a successful publish. The graph database was published and is usable; only the team-sharing artifact export (`.codebase-memory/graph.db.zst` written inside the repo) fails with EROFS. The run then reports `status: "error"` with a generic hint — "Pipeline failed. Check repo_path exists and contains source files" — which blames `repo_path` for a cause (read-only repo) that the hint never names, and hides the fact that the index itself succeeded.
+
+## What Changes
+
+- The artifact export failure is captured on the pipeline and surfaced in the `index_repository` error hint, naming `artifact_export`, the artifact path, and the remedy (writable checkout or `--persistence false`), instead of the generic repo_path blame.
+- Fail-hard semantics are **preserved**: a failed persistence export still returns a non-zero run result, exactly per the in-tree decision ("A failed persistence export intentionally fails the run; this used to be ignored", pipeline.c). Only the error message becomes truthful and actionable.
+- The pipeline records its own artifact-export error snapshot so a hint is never built from a stale global error left over from a previous run.
+
+## Capabilities
+
+### New Capabilities
+- `index-artifact-export-error-surfacing`: the `index_repository` error response truthfully attributes a post-publish persistence failure to the artifact export.
+
+### Modified Capabilities
+- None (no existing specs).
+
+## Impact
+
+- `src/pipeline/pipeline.c` — capture the artifact export error in `export_after_publish`; expose a query accessor.
+- `src/pipeline/pipeline.h` — declare the accessor.
+- `src/mcp/mcp.c` — error branch of `handle_index_repository` builds a truthful hint when the failure is the artifact export.
+- `tests/test_pipeline.c` or `tests/test_artifact.c` — regression test: persistence export failure into an unwritable artifact directory returns non-zero, names the artifact export, and does not blame repo_path.
@@ -0,0 +1,22 @@
+## Purpose
+Defines how `index_repository` reports a post-publish persistence artifact export failure: the run result stays failed (the in-tree policy is deliberate), but the error response names the actual cause — the artifact export into the repo — instead of blaming `repo_path`.
+
+## ADDED Requirements
+
+### Requirement: Failure attribution names the artifact export
+When `index_repository` returns a non-zero result and the failure was the post-publish artifact export (`export_after_publish`), the response SHALL carry a hint that states the persistence artifact export failed, names `.codebase-memory/` (and the export error detail when available), and states that the index database itself was published — the remedy being a writable checkout or running without `--persistence`.
+
+#### Scenario: persistence export fails on a read-only repo
+- **WHEN** `index_repository` runs with `--persistence true` on a read-only repository and the graph database publishes successfully while the artifact export fails
+- **THEN** the returned result has `status: "error"` and its hint names the artifact export failure (with the artifact directory and the export error), does not claim the repository path is missing or source-less, and does not recommend a mode change as the remedy
+
+#### Scenario: a genuine pipeline failure keeps the existing hint
+- **WHEN** `index_repository` fails before publish (e.g. the repository path does not exist or is empty)
+- **THEN** the hint is the existing generic message ("Pipeline failed. Check repo_path exists and contains source files"), unchanged, and no artifact-export wording is shown
+
+### Requirement: The surfaced error is from the current run only
+The pipeline SHALL capture the artifact export error of the run it just executed and expose it to the response builder; the response SHALL NOT be built from obsolete global error state belonging to a previous run.
+
+#### Scenario: previous run failed, current run fails earlier
+- **WHEN** a run fails after publish due to artifact export, and a later run fails before export (or never requests one)
+- **THEN** the later response does not mention the earlier run's artifact export error in its hint
@@ -0,0 +1,20 @@
+## 1. Pipeline snapshot of the artifact export error
+
+- [ ] 1.1 Add `char export_error[CBM_SZ_1K]` to `cbm_pipeline_t` (pipeline_internal.h), zero it at the start of `cbm_pipeline_run`, and in `export_after_publish` copy `cbm_artifact_export_last_error()` (fallback "unknown") into it on failure
+- [ ] 1.2 Add `const char *cbm_pipeline_export_error(const cbm_pipeline_t *p);` to pipeline.h and implement it (NULL-safe) in pipeline.c
+- [ ] 1.3 Verify: `make -f Makefile.cbm test-focused TEST_SUITES=pipeline` compiles and passes
+
+## 2. Truthful MCP error hint
+
+- [ ] 2.1 In `handle_index_repository`'s error branch (src/mcp/mcp.c), when `rc != 0` and `cbm_pipeline_export_error(p)` is non-empty, emit a hint naming the artifact export failure (`.codebase-memory/` directory, the export error detail when available), state the index database was published, and name the remedies (writable checkout or `--persistence false`); keep the generic hint verbatim in all other failure cases
+
+## 3. Regression test
+
+- [ ] 3.1 Add a test in tests/test_pipeline.c: build a pipeline with `persistence` enabled over a fixture repo, replace the artifact directory with a chmod 0555 directory (skip when `geteuid() == 0`), run `cbm_pipeline_run`, assert `rc != 0`, `cbm_pipeline_export_error(p)` non-empty, and the exported error mentions the artifact path
+- [ ] 3.2 Verify: `make -f Makefile.cbm test-focused TEST_SUITES=pipeline` passes with the new test
+
+## 4. Final validation
+
+- [ ] 4.1 Run `make -f Makefile.cbm test` (full suite) and `make -f Makefile.cbm lint-ci` — pass
+- [ ] 4.2 `OPENSPEC_NO_UPDATE_CHECK=1 openspec validate oss-codebase-memory-mcp-issue-1665 --json` — clean
+- [ ] 4.3 Archive: `OPENSPEC_NO_UPDATE_CHECK=1 openspec archive oss-codebase-memory-mcp-issue-1665 --yes`
@@ -0,0 +1,24 @@
+# index-artifact-export-error-surfacing Specification
+
+## Purpose
+Defines how `index_repository` reports a post-publish persistence artifact export failure: the run result stays failed (the in-tree policy is deliberate), but the error response names the actual cause — the artifact export into the repo — instead of blaming `repo_path`.
+
+## Requirements
+
+### Requirement: Failure attribution names the artifact export
+When `index_repository` returns a non-zero result and the failure was the post-publish artifact export (`export_after_publish`), the response SHALL carry a hint that states the persistence artifact export failed, names `.codebase-memory/` (and the export error detail when available), and states that the index database itself was published — the remedy being a writable checkout or running without `--persistence`.
+
+#### Scenario: persistence export fails on a read-only repo
+- **WHEN** `index_repository` runs with `--persistence true` on a read-only repository and the graph database publishes successfully while the artifact export fails
+- **THEN** the returned result has `status: "error"` and its hint names the artifact export failure (with the artifact directory and the export error), does not claim the repository path is missing or source-less, and does not recommend a mode change as the remedy
+
+#### Scenario: a genuine pipeline failure keeps the existing hint
+- **WHEN** `index_repository` fails before publish (e.g. the repository path does not exist or is empty)
+- **THEN** the hint is the existing generic message ("Pipeline failed. Check repo_path exists and contains source files"), unchanged, and no artifact-export wording is shown
+
+### Requirement: The surfaced error is from the current run only
+The pipeline SHALL capture the artifact export error of the run it just executed and expose it to the response builder; the response SHALL NOT be built from obsolete global error state belonging to a previous run.
+
+#### Scenario: previous run failed, current run fails earlier
+- **WHEN** a run fails after publish due to artifact export, and a later run fails before export (or never requests one)
+- **THEN** the later response does not mention the earlier run's artifact export error in its hint
@@ -8345,9 +8345,25 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) {
         yyjson_mut_obj_add_str(doc, root, "status", degraded ? "degraded" : "indexed");
     } else {
         yyjson_mut_obj_add_str(doc, root, "status", "error");
-        yyjson_mut_obj_add_str(doc, root, "hint",
-                               "Pipeline failed. Check repo_path exists and contains source files. "
-                               "Try mode='fast' for a quicker diagnostic run.");
+        /* #1665: a post-publish artifact export failure (read-only repo, etc.)
+         * must not be blamed on the repository path. The pipeline snapshots the
+         * export error of THIS run, so its presence names the phase exactly —
+         * the graph database was already published when the export ran. */
+        const char *export_error = cbm_pipeline_export_error(p);
+        if (export_error && export_error[0]) {
+            char hint[CBM_SZ_1K];
+            (void)snprintf(
+                hint, sizeof(hint),
+                "Index database was published, but the persistence artifact export failed for "
+                "repo_path/.codebase-memory (%s). Use a writable checkout, or re-run with "
+                "--persistence false if the shared artifact is not needed.",
+                export_error);
+            yyjson_mut_obj_add_strcpy(doc, root, "hint", hint);
+        } else {
+            yyjson_mut_obj_add_str(doc, root, "hint",
+                                   "Pipeline failed. Check repo_path exists and contains source "
+                                   "files. Try mode='fast' for a quicker diagnostic run.");
+        }
     }
 
     char *json = yy_doc_to_str(doc);

@@ -157,6 +157,13 @@ struct cbm_pipeline {
     atomic_int *cancelled;
     bool persistence; /* write .codebase-memory/graph.db.zst after indexing */
 
+    /* Snapshot of the artifact export failure of THIS run (set only by
+     * export_after_publish failure, zeroed at run start, cleared on success).
+     * The MCP layer reads it to attribute a failed run to artifact export
+     * without consulting cbm_artifact_export_last_error() directly — that
+     * global can still hold a PREVIOUS run's error. */
+    char export_error[CBM_SZ_1K];
+
     /* Indexing state (set during run) */
     cbm_gbuf_t *gbuf;
     cbm_registry_t *registry;
@@ -304,6 +311,10 @@ void cbm_pipeline_set_persistence(cbm_pipeline_t *p, bool enabled) {
     }
 }
 
+const char *cbm_pipeline_export_error(const cbm_pipeline_t *p) {
+    return p ? p->export_error : "";
+}
+
 bool cbm_pipeline_set_project_name(cbm_pipeline_t *p, const char *name) {
     if (!p || !name || !name[0]) {
         return false;
@@ -2497,6 +2508,12 @@ static int export_after_publish(cbm_pipeline_t *p, const char *final_path) {
         if (rc != 0) {
             const char *err = cbm_artifact_export_last_error();
             cbm_log_error("pipeline.err", "phase", "artifact_export", "err", err ? err : "unknown");
+            /* #1665: snapshot the error of THIS run so the MCP layer can
+             * attribute the failure truthfully, instead of re-reading the
+             * process-global export error (which may describe a previous run)
+             * and instead of the generic "Pipeline failed" hint that blames
+             * repo_path for a write-permission failure. */
+            (void)snprintf(p->export_error, sizeof(p->export_error), "%s", err ? err : "unknown");
         }
         return rc;
     }
@@ -2510,6 +2527,7 @@ int cbm_pipeline_run(cbm_pipeline_t *p) {
     if (!p) {
         return CBM_NOT_FOUND;
     }
+    p->export_error[0] = '\0';
     char *final_path = resolve_db_path(p);
     if (!final_path || !ensure_db_parent(final_path)) {
         free(final_path);

@@ -53,6 +53,13 @@ cbm_pipeline_t *cbm_pipeline_new(const char *repo_path, const char *db_path, cbm
  * When enabled, the pipeline writes a compressed artifact after indexing. */
 void cbm_pipeline_set_persistence(cbm_pipeline_t *p, bool enabled);
 
+/* Snapshot of the artifact export failure of the last cbm_pipeline_run, or ""
+ * when the run succeeded / did not reach post-publish export. Used to
+ * truthfully attribute a failed run to the persistence export (#1665) instead
+ * of the generic pipeline-error hint. Valid until the next cbm_pipeline_run or
+ * cbm_pipeline_free(). Returns "" for NULL p. */
+const char *cbm_pipeline_export_error(const cbm_pipeline_t *p);
+
 /* Free a pipeline and all its internal state. NULL-safe. */
 void cbm_pipeline_free(cbm_pipeline_t *p);