feat(notes):implement notes command#380
Conversation
![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: c738d88144
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| CREATE INDEX IF NOT EXISTS idx_ref_name_timestamp ON `reflog`(`ref_name`, `timestamp`); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS `notes` ( |
There was a problem hiding this comment.
Adding the notes table only to the bootstrap SQL means repositories created before this commit never get the table: normal command connections open the existing database and do not replay sqlite_20260309_init.sql, and this commit does not register a sql/migrations/* entry for notes. In that common upgrade path, any libra notes add/list/show/remove query will fail with SQLite no such table: notes; add an idempotent migration and register it in builtin_migrations() (or an equivalent schema ensure path) so existing repos are upgraded.
Useful? React with 👍 / 👎.
| } | ||
| ``` | ||
|
|
||
| When `<object>` is given and no note exists, `note_hash` is `null`. |
There was a problem hiding this comment.
This documented contract says libra notes list <object> --json returns success with note_hash: null when no note exists, but the implementation returns NotesError::NotFound, which is rendered as an error response instead. Agents or scripts built from this docs page will handle the missing-note case incorrectly, so either change list(Some(...)) to return the documented nullable payload or update the docs/tests to describe the error response.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 755ee1d9fe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e4ed29048
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94a4eceb5a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eacbf62e35
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for msg in messages { | ||
| parts.push(msg.clone()); | ||
| } | ||
| for file_path in files { |
There was a problem hiding this comment.
When users mix content sources, this loop appends every -F file after every -m message because clap has already split them into separate vectors. For example, libra notes add -F header.txt -m trailer stores the trailer before the file, while the command line order says the file paragraph came first; git notes add -h documents both -m and -F as note-content inputs and --separator as the paragraph separator, so scripts that compose notes from generated files plus messages will get a different blob/hash than expected. Preserve the original argument occurrence order, e.g. with clap value indices or explicit ArgMatches parsing.
Useful? React with 👍 / 👎.
| parts.push(data); | ||
| } | ||
|
|
||
| if parts.is_empty() { |
There was a problem hiding this comment.
This only checks that at least one -m/-F option was present, so libra notes add -m '' or an empty -F file silently creates an empty note. git notes add -h exposes --allow-empty specifically to permit empty notes, and without that flag Git treats empty content as missing; since Libra documents the implemented add surface as Git-compatible and does not provide --allow-empty, accidental empty variables/files become successful state changes. Either reject an empty assembled note here or add/document an explicit allow-empty flag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf109717c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// require that the paragraph order matches the command-line order (e.g. | ||
| /// `-F header -m trailer` must produce `header\n\ntrailer`, not the reverse). | ||
| fn ordered_content_parts() -> Vec<ContentPart> { | ||
| let args: Vec<String> = std::env::args().collect(); |
There was a problem hiding this comment.
When notes is invoked through the public in-process API (libra::exec_async/cli::parse_async(Some(...))) or from tests that pass argv explicitly, this rereads the host process std::env::args() instead of the argv that clap just parsed. In that context NotesSubcommand::Add { message, file, .. } contains the requested -m/-F values, but this helper sees the outer process arguments and either reports “provide a message” or uses unrelated flags, so embedded callers cannot reliably add notes. Preserve ordering from the parsed matches/value indices or pass the normalized argv into the handler rather than consulting process-global args.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c27071f59
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// Default notes ref namespace. | ||
| pub const DEFAULT_NOTES_REF: &str = "refs/notes/commits"; | ||
|
|
||
| /// Validates that a notes ref starts with `refs/notes/`. |
There was a problem hiding this comment.
When users pass the Git-compatible form --ref review, this validation rejects it because it requires the already-expanded refs/notes/ prefix. Git accepts --ref=review and stores it under refs/notes/review, so existing scripts using named notes namespaces fail before add/list/show/remove runs. Prefix short names with refs/notes/ before validation, while still rejecting refs outside that namespace.
Useful? React with 👍 / 👎.
| Some(hash) => println!( | ||
| "{} {}", | ||
| short_display_hash(hash), | ||
| short_display_hash(&entry.annotated_object) |
There was a problem hiding this comment.
For libra notes list HEAD, this always prints <note-hash> <annotated-object>, but the Git-compatible object-scoped form emits only the note hash. Scripts commonly use notes list <object> as a hash lookup, and the extra object field makes the stdout value invalid as a single hash; carry whether the list was object-scoped into rendering and print only the note hash for that case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90b7fbc8f8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let result = db | ||
| .execute(Statement::from_sql_and_values( | ||
| sea_orm::DatabaseBackend::Sqlite, | ||
| "INSERT OR IGNORE INTO notes (notes_ref, object, blob) VALUES (?, ?, ?)", |
There was a problem hiding this comment.
When a user runs libra notes add and then shares the repository via normal push/fetch/clone, this writes only the local SQLite notes mapping; the transport code I checked still filters/plans refs to branches/tags (push: src/command/push.rs:1235, fetch: src/command/fetch.rs:1094 and src/command/fetch.rs:1755) and never advertises or persists refs/notes/*. Since the new docs and changelog describe Git-compatible notes refs for review/CI metadata, collaborators will silently lose those notes after cloning or fetching; either store/update real transferable refs or explicitly mark/block the feature as local-only until transport support exists.
Useful? React with 👍 / 👎.
Resolved conflicts in migration registration and test assertions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rt names;Object-scoped list now emits only the note hash;noted notes are local-only
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 939db0b28b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if !raw.contains('/') { | ||
| return Ok(format!("refs/notes/{raw}")); |
There was a problem hiding this comment.
Fresh evidence after the earlier short-ref fix: this branch only expands short names that contain no slash, so libra notes --ref review/team ... is still rejected even though git notes -h documents --ref <notes-ref> and a local check with Git accepts --ref review/team as refs/notes/review/team. Scripts that use hierarchical notes namespaces for review/CI channels will fail before any note operation; expand any valid non-fully-qualified notes ref under refs/notes/ and reject only invalid refnames or refs outside that namespace.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba57e2a63b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if i < argv.len() { | ||
| parts.push(ContentPart::File(argv[i].clone())); | ||
| } | ||
| } else if let Some(val) = arg.strip_prefix("-m") { |
There was a problem hiding this comment.
When users combine the new force flag with -m in a standard short-option cluster, e.g. libra notes add -fm "updated" HEAD, clap accepts this as -f plus -m and Git does too (checked git notes add -h, which lists both -f and -m as short options). This raw argv scanner only recognizes tokens that start with -m, so it misses the message after a leading -f and the handler rejects the command as if no content was provided. Use clap value indices / parsed matches or split clustered short options before rebuilding content order.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 785cbb4625
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| storage | ||
| .put(&blob.id, &blob.data, blob.get_type()) | ||
| .map_err(NotesError::StoreBlobFailed)?; |
There was a problem hiding this comment.
When libra notes add is run without -f for an object that already has a note, this stores the new note blob before the later INSERT OR IGNORE detects the conflict and returns AlreadyExists. That failed command still leaves the rejected note content in the object store (and potentially the background object index/cloud backup), so an expected no-op conflict changes repository state and can retain sensitive note text. Check for the existing mapping before persisting the blob, or clean up/avoid indexing the blob when the insert reports rows_affected() == 0.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bf81f71f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| #[command(about = "Add, show, list, or remove notes attached to commits")] | ||
| Notes(command::notes::NotesArgs), |
There was a problem hiding this comment.
This adds notes as a Git-compatible top-level command, but the integration plan was not updated: I searched docs/development/integration-scenarios/README.md, docs/development/integration-test-plan.md, docs/development/integration-scenarios.yaml, and tools/integration-runner/src and found no notes owner/smoke scenario. The repo’s command-surface checklist requires every new Git-compatible command to get a Command → Scenario Map row plus at least one cli.<cmd>-smoke runner scenario, so check-plan/black-box coverage will remain out of sync for this new public CLI surface.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc8b035346
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| util::get_commit_base(s) | ||
| .await | ||
| .map_err(|e| NotesError::InvalidObject(s.to_string(), e)) |
There was a problem hiding this comment.
For notes attached to blobs, trees, or tag objects, this path rejects the target before the note operation runs because get_commit_base only accepts commits and peels tags to commits. I checked git notes -h, whose add/list/show/remove forms all take [<object>], and this PR's docs/compat table also advertises the Git-compatible <object> parameter; scripts that annotate generated blobs or tag objects will fail with “reference is not a commit” or attach to the peeled commit instead. Use an object resolver that returns the named object hash without requiring ObjectType::Commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58c63e7224
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ); | ||
| } | ||
|
|
||
| let content = parts.join("\n\n"); |
There was a problem hiding this comment.
When users build a note with -m, Git terminates the assembled message with a trailing LF (checked locally with git notes add -m hi, whose git notes show bytes end in 0a). This join returns hi or one\n\ntwo without the final newline, so libra notes show output and the stored note blob hash differ for every -m-created note, breaking scripts that compare note hashes or output with Git. Add the final newline for message-sourced paragraphs while preserving exact -F file content if that is the intended file behavior.
Useful? React with 👍 / 👎.
# Conflicts: # COMPATIBILITY.md # Cargo.lock # tests/db_migration_test.rs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The merge of origin/main into libra-tools#380 added two migrations from independent branches — 2026053101 (notes, from this PR) and 2026060801 (revert_sequence, from main) — so builtin_runner() now registers 11 migrations. The inline unit test builtin_runner_registers_current_builtin_migrations still asserted 10: a semantic conflict git could not detect (both sides happened to assert 10, each having added one migration), so it auto-merged without a textual conflict and only failed at runtime under cargo test --lib. Matches the same fix already applied to tests/db_migration_test.rs during conflict resolution.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
2 participants
概述
实现
libra notes命令，兼容 Git notes 机制，支持四个子命令（add、list、show、remove），采用 SQLite + 对象库 blob 双层存储。附带幂等迁移脚本（2026053101），确保已有仓库在下次启动时自动升级。变更清单
src/internal/notes.rsINSERT OR IGNORE+ON CONFLICT无竞态 add、事务化 remove（两阶段 +AND blob = ?并发保护）、list 返回 nullable、短 ref 名自动展开src/command/notes.rs-m/-F按命令行顺序拼接、对象作用域 list 仅输出 note hash、拒绝空内容、7 种错误映射tests/command/notes_test.rssql/sqlite_20260309_init.sqlnotes表 DDL（bootstrap 路径）sql/migrations/2026053101_notes.sqlIF NOT EXISTS）sql/migrations/2026053101_notes_down.sqlsrc/internal/db/migration.rsbuiltin_migrations()，10 个迁移中最大版本2026060401tests/db_migration_test.rsrunner.len() = 10，max = 2026060401）docs/commands/notes.mdCargo.tomlCOMPATIBILITY.mdpartial（4 个已实现，6 个暂不实现），标注 local-onlyCHANGELOG.mdsrc/cli.rsexecute_safe接收argv: &[String]保留-m/-F顺序总计：13 个文件，+2,491 / −28 行。
设计决策
ClientStorage::put()/get()基础设施force=true用INSERT ... ON CONFLICT DO UPDATE原子 upsert；force=false用INSERT OR IGNORE+rows_affected()检测冲突，消除 check-then-insert 竞态窗口AND blob = ?条件——若并发add -f覆盖了 blob，rows_affected()为 0 则回滚事务，防止误删并发写入的笔记--ref review自动展开为refs/notes/review，兼容 Git 习惯。非refs/notes/开头且含/的完整路径仍拒绝-m/-F按命令行顺序拼接：通过显式传入argv: &[String]保留-m/-F的原始出现顺序，而非 clap 默认的分离 Vec 顺序libra notes list HEAD只打印 hash，匹配 Git 的git notes list <object>行为，方便脚本解析-m ''或空文件返回错误，与 Git 的--allow-empty显式许可语义一致-m/-F指定内容；编辑器调用与 headless/agent 工作流不兼容resolve_ref("HEAD")走Head::current_commit_result()API，返回HeadUnborn（LBR-REPO-003, exit 128），而非泛化的InvalidObject测试覆盖
63 passed, 0 failed, 0 ignored, 0 measured
--ref+--quiet兼容性
add/list/show/removeappend/edit/copy/merge/prune/get-ref已知限制
add必须通过-m或-F指定内容notes表目前缺少author/timestamp等元数据列