Skip to content

fix: close index bootstrap recovery actions - #118

Merged
catoncat merged 2 commits into
catoncat:mainfrom
z2z23n0:fix/close-bootstrap-next-action
Aug 18, 2026
Merged

fix: close index bootstrap recovery actions#118
catoncat merged 2 commits into
catoncat:mainfrom
z2z23n0:fix/close-bootstrap-next-action

Conversation

@z2z23n0

@z2z23n0 z2z23n0 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Problem observed

We hit a concrete first-use failure while asking an agent to recover context from an earlier QA task:

  1. The agent selected the Sherlog Skill and issued the matching read-only query.
  2. The local index did not exist yet, so the CLI returned index_unavailable.
  3. The recovery action amounted to argv: ["shlog", "sync"].
  4. The agent recognized that sync writes local state, but the action did not say which resolved database/source/selector it would write and did not declare the side effect in a machine-readable form.
  5. In a read-only investigation, the agent therefore declined to guess or broaden the write and bypassed Sherlog for another data source.

From the user perspective, Sherlog was installed and the Skill was selected, but the first query still did not initialize Sherlog or produce a result.

Why this is a general contract bug

This is deterministic whenever the queried index is absent: a fresh installation, a new custom --db, or an index that was moved or removed. Whether the user notices it depends on the host. A permissive host may reconstruct and run a broad sync; a conservative host will reject or avoid the ambiguous write. The underlying recovery contract is incomplete in both cases.

The missing logic should not be added as Skill-side if/else branches. By the time the error is built, the CLI has already resolved the effective database, source, root, and selector. Asking the Skill or host to reconstruct those values duplicates CLI resolution rules and can silently sync the default Codex root or default database instead of the scope that failed.

This PR partially addresses #107, specifically P0.3 for index_unavailable recovery actions.

Root cause

The read path preserved the resolved query context internally, but discarded it when converting a missing SQLite index into AppError::IndexUnavailable. The error exposed a human-oriented suggestion, not a replayable host action:

{
  "argv": ["shlog", "sync"]
}

That shape has three problems:

  • it does not preserve the --db, source, and selector resolved for the failed query;
  • it hard-codes the executable name instead of inheriting the running binary;
  • it does not tell a host that executing it writes the index.

What changed

1. The CLI now owns recovery command construction

IndexBootstrapCommand builds the action only after the effective selector and database have been resolved. A single-source/scope failure now returns a command equivalent to:

{
  "selector": { "source": "codex", "kind": "cwd", "root": "...", "cwd": "..." },
  "command": {
    "executable": "inherit",
    "args": [
      "sync",
      "--source", "codex",
      "--selector", "<canonical-selector-key>",
      "--db", "<the-db-used-by-the-failed-query>",
      "--json"
    ],
    "sideEffect": "write_index"
  },
  "argv": ["shlog", "sync", "...the same closed args..."]
}

argv remains as a compatibility mirror; the typed command is the authoritative action for hosts that understand it.

2. Missing-index errors retain the failed command scope

  • single-source find preserves its resolved all, cwd, root, or explicit selector;
  • list preserves its resolved selector;
  • read-range, read-page, and stats preserve the source-specific default all selector;
  • an unscoped multi-source find, where there is no single exact selector, keeps the bounded default Codex alternatives instead of pretending that one exact recovery scope exists.

3. The Skill stays thin

The Skill adds one invariant rather than a recovery decision tree:

Run the read-only query directly; if it fails, execute the typed nextAction.commands[].command unchanged when host policy permits its declared side effect.

The Skill does not preflight with status, rebuild sync arguments, or unconditionally run a broad sync.

Why this closes the loop

The resulting first-use flow is now explicit and policy-safe:

read-only query
  -> index_unavailable
  -> closed command + sideEffect: write_index
  -> host allows it or asks for authorization
  -> exact scoped sync
  -> retry the original read-only query

This preserves both important invariants:

  • Sherlog read commands remain side-effect free and never implicitly sync;
  • a host no longer has to guess what the recovery write will touch.

The regression test exercises that exact loop: it runs a scoped find against a missing custom database, executes the returned command.args unchanged, verifies that sync creates the requested index, then retries the original query and finds the expected session.

Non-goals

This PR does not:

  • auto-sync from a read command;
  • index at install time;
  • add a watcher or daemon;
  • teach the Skill a matrix of first-run conditions;
  • silently expand a failed scoped query into an all-history sync.

Validation

  • cargo fmt --all -- --check
  • cargo test --workspace --all-targets --all-features --locked
  • cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
  • cargo build --release --locked --bin shlog
  • npm run check
  • manual missing-index smoke test confirmed that the failed read does not create the database
  • GitHub CI: all six checks passed

@z2z23n0
z2z23n0 force-pushed the fix/close-bootstrap-next-action branch from e02ab77 to 21778c8 Compare August 17, 2026 11:39
Default find --cwd/--root/--selector now emits one closed sync command
per source instead of falling back to Codex all-history alternatives.
@catoncat
catoncat merged commit f66f5f4 into catoncat:main Aug 18, 2026
6 checks passed
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.

2 participants