fix(cli): report missing notes with a failing exit status - #1508
Conversation
Signed-off-by: phernandez <paul@basicmachines.co>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ade8334968
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6979472298
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
|
Broader verification on 2286800: uv run pytest tests/mcp -q --no-cov passed all 1,371 tests. The full MCP suite found no additional regressions. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2286800a12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 534b8acd28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Why
A read after deleting a note returned null fields and exit status 0, so scripts and agents could not distinguish a missing note from success.
What Changed
Unresolved JSON reads now include
error: "NOTE_NOT_FOUND"and an identifier-specific message. The CLI exits 1 in JSON, piped, plain, and Rich modes while retaining related-note suggestions. Missing UUIDs receive the same structured result. Successful reads and ordinary MCP text guidance retain their behavior.Implementation Details
Only confirmed resolution misses enter title/text fallback. Authorization, timeout, retrieval, and fallback-search failures propagate instead of being classified as absence. A resource 404 behind an existing entity remains a read error. Because a sliced entity GET also returns 404 for an existing non-Markdown entity, that error path confirms absence with an unsliced GET; successful UUID line reads still use one request.
The CLI renders the not-found result in the requested mode before signaling failure, including when
--frontmatteris requested. The manual documents the contract.Testing
uv run pytest tests/mcp/test_read_note_request_counts.py tests/mcp/test_tool_read_note.py tests/mcp/test_line_scanning.py test-int/cli/test_cli_tool_json_failure_integration.py test-int/mcp/test_line_scanning_integration.py test-int/mcp/test_read_note_missing_identifier_integration.py -q --no-cov: 98 passed.BASIC_MEMORY_TEST_POSTGRES=1 uv run pytest test-int/cli/test_cli_tool_json_failure_integration.py test-int/mcp/test_read_note_missing_identifier_integration.py -q --no-cov: 14 passed.uv run pytest tests/test_man_pages.py -q --no-cov: 39 passed.just fast-checkandjust doctor: passed.Risks / Follow-ups
Scripts that treated missing notes as successful reads now receive exit status 1. JSON retains the existing null fields and adds error/message fields. A failed UUID line read may make one additional request to distinguish a missing entity from missing Markdown content.
Closes #1495.