Repository navigation
chore/more fixes - #171
chore/more fixes#171
Conversation
Decouple core Git-dependent application logic (like cache and journal services) from the underlying library (`libgit2`) using the Port and Adapter (Hexagonal) pattern. The core change is the introduction of the abstract `gm_git_repository_port_t` interface (`core/include/gitmind/ports/`) which defines abstract methods for repository-scoped operations, including tree building, object reading, commit creation, and reference updates. A concrete adapter, `gm_libgit2_repository_port`, is implemented to fulfill this contract using `libgit2`. The application context (`gm_context_t`) is updated to hold and manage this new abstract port, replacing direct dependencies on `libgit2` for these high-level operations across the application (e.g., in `apps/cli/main.c` and `benchmarks/`). Additionally, `gm_cache_meta_t` is updated to include `cache_tip_oid` to align cache management with the new port-based commit and reference update capabilities. This isolation significantly improves modularity and testability.
The commit refactors the calculation of a Git commit's total tree size,
moving the concrete `libgit2` implementation into the abstract
`gm_git_repository_port_t` interface.
This change:
* Adds `commit_tree_size` to `gm_git_repository_port_vtbl_t` and provides
a concrete recursive implementation (`tree_size_recursive`) in the
`libgit2` adapter.
* Updates `gm_cache_stats` to use this new port method via
`gm_git_repository_port_commit_tree_size` to determine the actual
cache size.
* Deletes the now redundant `core/src/cache/tree_size.c` and
`core/src/cache/cache_internal.h` files, removing the direct
`libgit2` dependency from the cache core.
This improves architectural modularity and isolation between the core
application logic and the Git library adapter.
This change further isolates the core application logic from the concrete
Git implementation (`libgit2`) by abstracting key repository
operations behind the `gm_git_repository_port_t` interface.
The abstraction includes:
* `head_branch`: Retrieving the name of the current HEAD branch.
* `commit_read_message`: Reading the raw commit message for an OID.
* `walk_commits`: Providing a mechanism to traverse the commit history
for a given reference (`ref_name`) using a callback.
The `journal/reader` is updated to utilize these new port methods,
removing direct dependencies on `libgit2` headers and APIs for commit
traversal and branch resolution.
The journal writer is refactored to rely on the abstract
`gm_git_repository_port_t` interface for all Git write operations, further
decoupling the core from the `libgit2` implementation.
This change:
* Removes all direct `libgit2` headers and types (`git_repository`,
`git_oid`, `git_commit`, etc.) from `core/src/journal/writer.c`.
* Switches `journal_ctx_t` to use the abstract `gm_git_repository_port_t`.
* Updates `get_current_branch` and `resolve_branch_name` to use the
existing `gm_git_repository_port_head_branch` method.
Subsequent commits will introduce new port methods for tree, commit, and
reference creation to fully port the writing logic.
This change finalizes the refactoring of the journal writing path by:
1. **Generalizing Commit Specification:** The `gm_git_commit_spec_t` is
extended to support an arbitrary list of parent OIDs and a parent
count, replacing the previous implicit single-parent assumption.
2. **Updating Adapter:** The `libgit2` adapter is updated to handle the
new parent list, performing necessary lookups to convert OIDs to
`git_commit` objects required by the underlying library.
3. **Abstracting Journal Writer:** `core/src/journal/writer.c` is fully
ported to the abstract API, replacing all remaining direct `libgit2`
calls with:
* `gm_git_repository_port_walk_commits` to find the parent commit.
* `gm_git_repository_port_commit_create` for commit generation.
* `gm_git_repository_port_reference_update` for updating the branch.
This completes the abstraction of all Git read and write operations from
the core journal logic.
… port for testing
This change set streamlines core port interfaces and significantly upgrades the mock object used for testing Git-related components.
Key improvements:
1. **Port Clarity Refactoring:**
* **Filesystem Paths:** Renamed generic path segment arguments (`s1`, `s2`, etc.) in the `gm_fs_temp_port_path_join` function across its declaration and implementations (`fs_temp_port.h`, `posix_temp_adapter.c`, `fake_fs_temp_port.c`) to descriptive names like **`component1`**, **`component2`**, etc.
* **Git Walk Callback:** Renamed the `walk_commits` callback parameter from `cb` to **`visit_callback`** in the `git_repository_port` interface and the `libgit2` adapter.
2. **Enhanced Fake Git Repository Port:**
* Introduced new internal structures (`gm_fake_git_ref_entry_t`, `gm_fake_git_commit_entry_t`) to allow the fake port to store and serve mocked Git history, including commit **OIDs** and **messages**, indexed by reference.
* Added public functions (`gm_fake_git_repository_port_set_head_branch`, `gm_fake_git_repository_port_add_ref_commit`) for tests to **programmatically define history**.
* Implemented full functionality for `fake_head_branch`, `fake_walk_commits`, and `fake_commit_read_message` using this new mocked state, ensuring that components (like the cache and journal) can now be thoroughly unit tested without skipping Git logic seams.
3. **Header Tidy:** Replaced a direct `#include <git2/repository.h>` with a **forward declaration** (`typedef struct git_repository git_repository;`) in `libgit2_repository_port.h`, reducing build time dependencies.
…b identity resolution from a legacy shim (ctx->git_ops) to the new, preferred gm_git_repository_port interface. This change advances the decoupling of core logic from the underlying libgit2 implementation. This diff implements a significant refactoring effort to move Git blob identity resolution from a legacy shim (ctx->git_ops) to the new, preferred gm_git_repository_port interface. This change advances the decoupling of core logic from the underlying libgit2 implementation. Core Changes: Porting Blob Resolution to the Git Port Port Interface Addition (core/include/gitmind/ports/git_repository_port.h): A new function, gm_git_repository_port_resolve_blob_at_head, is added to the gm_git_repository_port_vtbl_t structure. This function is responsible for looking up the Object ID (gm_oid_t) of a file given its path at the repository's current HEAD. libgit2 Implementation (core/src/adapters/git/libgit2_repository_port.c): The resolve_blob_at_head_impl is implemented using libgit2 functions (git_repository_head, git_commit_lookup, git_tree_entry_by_path) to resolve the blob OID. It introduces a helper function, open_blob_entry, to consolidate the common logic for looking up a tree entry from a commit. Edge Module Refactor (core/src/edge/attributed.c and core/src/edge/edge.c): The core gm_edge_create and gm_edge_attributed_create functions are updated: The deprecated Git shim is removed. A new helper, resolve_blob_identity, is introduced, which calls the new port function (gm_git_repository_port_resolve_blob_at_head). This helper not only retrieves the canonical gm_oid_t but also automatically populates the legacy src_sha/tgt_sha arrays by copying the raw OID bytes. This simplifies the creation functions and maintains compatibility with older fields during the transition. Testing Framework Update The unit test environment is substantially modified to support the new port-based logic: Fake Port Enhancement: The fake port (core/tests/fakes/git/fake_git_repository_port.h/.c) gains new functions (gm_fake_git_repository_port_add_blob_mapping, fake_resolve_blob_at_head) that allow tests to pre-configure a map of file paths to expected blob OIDs, providing a realistic mock of Git resolution. Test Refactoring (core/tests/unit/test_edge.c): The previous custom mock function (mock_resolve_blob) for SHA resolution is removed. The test_edge_create_success unit test now uses the gm_fake_git_repository_port_t to set up the expected OIDs for "src/a.c" and "src/b.c" before calling the gm_edge_create API. Build System: meson.build is updated to include the source file for the fake Git repository port (core/tests/fakes/git/fake_git_repository_port.c) when building the test_edge executable. Miscellaneous The function gm_snprintf in core/include/gitmind/security/string.h is changed from static to static inline to potentially improve performance by allowing the compiler to substitute the function body at the call site. The AGENTS.md log is updated to reflect the completion of this feature, confirming that edge creation now relies on the gm_git_repository_port.This diff implements a significant refactoring effort to move Git blob identity resolution from a legacy shim (ctx->git_ops) to the new, preferred gm_git_repository_port interface. This change advances the decoupling of core logic from the underlying libgit2 implementation. Core Changes: Porting Blob Resolution to the Git Port Port Interface Addition (core/include/gitmind/ports/git_repository_port.h): A new function, gm_git_repository_port_resolve_blob_at_head, is added to the gm_git_repository_port_vtbl_t structure. This function is responsible for looking up the Object ID (gm_oid_t) of a file given its path at the repository's current HEAD. libgit2 Implementation (core/src/adapters/git/libgit2_repository_port.c): The resolve_blob_at_head_impl is implemented using libgit2 functions (git_repository_head, git_commit_lookup, git_tree_entry_by_path) to resolve the blob OID. It introduces a helper function, open_blob_entry, to consolidate the common logic for looking up a tree entry from a commit. Edge Module Refactor (core/src/edge/attributed.c and core/src/edge/edge.c): The core gm_edge_create and gm_edge_attributed_create functions are updated: The deprecated Git shim is removed. A new helper, resolve_blob_identity, is introduced, which calls the new port function (gm_git_repository_port_resolve_blob_at_head). This helper not only retrieves the canonical gm_oid_t but also automatically populates the legacy src_sha/tgt_sha arrays by copying the raw OID bytes. This simplifies the creation functions and maintains compatibility with older fields during the transition. Testing Framework Update The unit test environment is substantially modified to support the new port-based logic: Fake Port Enhancement: The fake port (core/tests/fakes/git/fake_git_repository_port.h/.c) gains new functions (gm_fake_git_repository_port_add_blob_mapping, fake_resolve_blob_at_head) that allow tests to pre-configure a map of file paths to expected blob OIDs, providing a realistic mock of Git resolution. Test Refactoring (core/tests/unit/test_edge.c): The previous custom mock function (mock_resolve_blob) for SHA resolution is removed. The test_edge_create_success unit test now uses the gm_fake_git_repository_port_t to set up the expected OIDs for "src/a.c" and "src/b.c" before calling the gm_edge_create API. Build System: meson.build is updated to include the source file for the fake Git repository port (core/tests/fakes/git/fake_git_repository_port.c) when building the test_edge executable. Miscellaneous The function gm_snprintf in core/include/gitmind/security/string.h is changed from static to static inline to potentially improve performance by allowing the compiler to substitute the function body at the call site. The AGENTS.md log is updated to reflect the completion of this feature, confirming that edge creation now relies on the gm_git_repository_port.This diff implements a significant refactoring effort to move Git blob identity resolution from a legacy shim (ctx->git_ops) to the new, preferred gm_git_repository_port interface. This change advances the decoupling of core logic from the underlying libgit2 implementation. Core Changes: Porting Blob Resolution to the Git Port Port Interface Addition (core/include/gitmind/ports/git_repository_port.h): A new function, gm_git_repository_port_resolve_blob_at_head, is added to the gm_git_repository_port_vtbl_t structure. This function is responsible for looking up the Object ID (gm_oid_t) of a file given its path at the repository's current HEAD. libgit2 Implementation (core/src/adapters/git/libgit2_repository_port.c): The resolve_blob_at_head_impl is implemented using libgit2 functions (git_repository_head, git_commit_lookup, git_tree_entry_by_path) to resolve the blob OID. It introduces a helper function, open_blob_entry, to consolidate the common logic for looking up a tree entry from a commit. Edge Module Refactor (core/src/edge/attributed.c and core/src/edge/edge.c): The core gm_edge_create and gm_edge_attributed_create functions are updated: The deprecated Git shim is removed. A new helper, resolve_blob_identity, is introduced, which calls the new port function (gm_git_repository_port_resolve_blob_at_head). This helper not only retrieves the canonical gm_oid_t but also automatically populates the legacy src_sha/tgt_sha arrays by copying the raw OID bytes. This simplifies the creation functions and maintains compatibility with older fields during the transition. Testing Framework Update The unit test environment is substantially modified to support the new port-based logic: Fake Port Enhancement: The fake port (core/tests/fakes/git/fake_git_repository_port.h/.c) gains new functions (gm_fake_git_repository_port_add_blob_mapping, fake_resolve_blob_at_head) that allow tests to pre-configure a map of file paths to expected blob OIDs, providing a realistic mock of Git resolution. Test Refactoring (core/tests/unit/test_edge.c): The previous custom mock function (mock_resolve_blob) for SHA resolution is removed. The test_edge_create_success unit test now uses the gm_fake_git_repository_port_t to set up the expected OIDs for "src/a.c" and "src/b.c" before calling the gm_edge_create API. Build System: meson.build is updated to include the source file for the fake Git repository port (core/tests/fakes/git/fake_git_repository_port.c) when building the test_edge executable. Miscellaneous The function gm_snprintf in core/include/gitmind/security/string.h is changed from static to static inline to potentially improve performance by allowing the compiler to substitute the function body at the call site. The AGENTS.md log is updated to reflect the completion of this feature, confirming that edge creation now relies on the gm_git_repository_port.This diff implements a significant refactoring effort to move Git blob identity resolution from a legacy shim (ctx->git_ops) to the new, preferred gm_git_repository_port interface. This change advances the decoupling of core logic from the underlying libgit2 implementation. Core Changes: Porting Blob Resolution to the Git Port Port Interface Addition (core/include/gitmind/ports/git_repository_port.h): A new function, gm_git_repository_port_resolve_blob_at_head, is added to the gm_git_repository_port_vtbl_t structure. This function is responsible for looking up the Object ID (gm_oid_t) of a file given its path at the repository's current HEAD. libgit2 Implementation (core/src/adapters/git/libgit2_repository_port.c): The resolve_blob_at_head_impl is implemented using libgit2 functions (git_repository_head, git_commit_lookup, git_tree_entry_by_path) to resolve the blob OID. It introduces a helper function, open_blob_entry, to consolidate the common logic for looking up a tree entry from a commit. Edge Module Refactor (core/src/edge/attributed.c and core/src/edge/edge.c): The core gm_edge_create and gm_edge_attributed_create functions are updated: The deprecated Git shim is removed. A new helper, resolve_blob_identity, is introduced, which calls the new port function (gm_git_repository_port_resolve_blob_at_head). This helper not only retrieves the canonical gm_oid_t but also automatically populates the legacy src_sha/tgt_sha arrays by copying the raw OID bytes. This simplifies the creation functions and maintains compatibility with older fields during the transition. Testing Framework Update The unit test environment is substantially modified to support the new port-based logic: Fake Port Enhancement: The fake port (core/tests/fakes/git/fake_git_repository_port.h/.c) gains new functions (gm_fake_git_repository_port_add_blob_mapping, fake_resolve_blob_at_head) that allow tests to pre-configure a map of file paths to expected blob OIDs, providing a realistic mock of Git resolution. Test Refactoring (core/tests/unit/test_edge.c): The previous custom mock function (mock_resolve_blob) for SHA resolution is removed. The test_edge_create_success unit test now uses the gm_fake_git_repository_port_t to set up the expected OIDs for "src/a.c" and "src/b.c" before calling the gm_edge_create API. Build System: meson.build is updated to include the source file for the fake Git repository port (core/tests/fakes/git/fake_git_repository_port.c) when building the test_edge executable. Miscellaneous The function gm_snprintf in core/include/gitmind/security/string.h is changed from static to static inline to potentially improve performance by allowing the compiler to substitute the function body at the call site. The AGENTS.md log is updated to reflect the completion of this feature, confirming that edge creation now relies on the gm_git_repository_port.This diff implements a significant refactoring effort to move Git blob identity resolution from a legacy shim (ctx->git_ops) to the new, preferred gm_git_repository_port interface. This change advances the decoupling of core logic from the underlying libgit2 implementation. Core Changes: Porting Blob Resolution to the Git Port Port Interface Addition (core/include/gitmind/ports/git_repository_port.h): A new function, gm_git_repository_port_resolve_blob_at_head, is added to the gm_git_repository_port_vtbl_t structure. This function is responsible for looking up the Object ID (gm_oid_t) of a file given its path at the repository's current HEAD. libgit2 Implementation (core/src/adapters/git/libgit2_repository_port.c): The resolve_blob_at_head_impl is implemented using libgit2 functions (git_repository_head, git_commit_lookup, git_tree_entry_by_path) to resolve the blob OID. It introduces a helper function, open_blob_entry, to consolidate the common logic for looking up a tree entry from a commit. Edge Module Refactor (core/src/edge/attributed.c and core/src/edge/edge.c): The core gm_edge_create and gm_edge_attributed_create functions are updated: The deprecated Git shim is removed. A new helper, resolve_blob_identity, is introduced, which calls the new port function (gm_git_repository_port_resolve_blob_at_head). This helper not only retrieves the canonical gm_oid_t but also automatically populates the legacy src_sha/tgt_sha arrays by copying the raw OID bytes. This simplifies the creation functions and maintains compatibility with older fields during the transition. Testing Framework Update The unit test environment is substantially modified to support the new port-based logic: Fake Port Enhancement: The fake port (core/tests/fakes/git/fake_git_repository_port.h/.c) gains new functions (gm_fake_git_repository_port_add_blob_mapping, fake_resolve_blob_at_head) that allow tests to pre-configure a map of file paths to expected blob OIDs, providing a realistic mock of Git resolution. Test Refactoring (core/tests/unit/test_edge.c): The previous custom mock function (mock_resolve_blob) for SHA resolution is removed. The test_edge_create_success unit test now uses the gm_fake_git_repository_port_t to set up the expected OIDs for "src/a.c" and "src/b.c" before calling the gm_edge_create API. Build System: meson.build is updated to include the source file for the fake Git repository port (core/tests/fakes/git/fake_git_repository_port.c) when building the test_edge executable. Miscellaneous The function gm_snprintf in core/include/gitmind/security/string.h is changed from static to static inline to potentially improve performance by allowing the compiler to substitute the function body at the call site. The AGENTS.md log is updated to reflect the completion of this feature, confirming that edge creation now relies on the gm_git_repository_port. Core Changes: Porting Blob Resolution to the Git Port Port Interface Addition (core/include/gitmind/ports/git_repository_port.h): A new function, gm_git_repository_port_resolve_blob_at_head, is added to the gm_git_repository_port_vtbl_t structure. This function is responsible for looking up the Object ID (gm_oid_t) of a file given its path at the repository's current HEAD. libgit2 Implementation (core/src/adapters/git/libgit2_repository_port.c): The resolve_blob_at_head_impl is implemented using libgit2 functions (git_repository_head, git_commit_lookup, git_tree_entry_by_path) to resolve the blob OID. It introduces a helper function, open_blob_entry, to consolidate the common logic for looking up a tree entry from a commit. Edge Module Refactor (core/src/edge/attributed.c and core/src/edge/edge.c): The core gm_edge_create and gm_edge_attributed_create functions are updated: The deprecated Git shim is removed. A new helper, resolve_blob_identity, is introduced, which calls the new port function (gm_git_repository_port_resolve_blob_at_head). This helper not only retrieves the canonical gm_oid_t but also automatically populates the legacy src_sha/tgt_sha arrays by copying the raw OID bytes. This simplifies the creation functions and maintains compatibility with older fields during the transition. Testing Framework Update The unit test environment is substantially modified to support the new port-based logic: Fake Port Enhancement: The fake port (core/tests/fakes/git/fake_git_repository_port.h/.c) gains new functions (gm_fake_git_repository_port_add_blob_mapping, fake_resolve_blob_at_head) that allow tests to pre-configure a map of file paths to expected blob OIDs, providing a realistic mock of Git resolution. Test Refactoring (core/tests/unit/test_edge.c): The previous custom mock function (mock_resolve_blob) for SHA resolution is removed. The test_edge_create_success unit test now uses the gm_fake_git_repository_port_t to set up the expected OIDs for "src/a.c" and "src/b.c" before calling the gm_edge_create API. Build System: meson.build is updated to include the source file for the fake Git repository port (core/tests/fakes/git/fake_git_repository_port.c) when building the test_edge executable. Miscellaneous The function gm_snprintf in core/include/gitmind/security/string.h is changed from static to static inline to potentially improve performance by allowing the compiler to substitute the function body at the call site. The AGENTS.md log is updated to reflect the completion of this feature, confirming that edge creation now relies on the gm_git_repository_port.
…_repository_port Completes the refactoring to decouple core Git operations from the legacy `ctx->git_ops` shims and direct `git_repository *` access. This commit: * **Post-Commit Hook**: Initializes a live `libgit2`-backed `gm_git_repository_port` instance and uses it for all Git interactions, eliminating the old `resolve_blob`/`create_commit` shims. * **Journal Writer**: Replaces direct Git calls with port functions (`gm_git_repository_port_walk_commits` for parent lookup and `gm_git_repository_port_commit_create` for commit creation). * **Cleanup**: Standardizes header inclusions and removes obsolete type declarations and logic across several files (`post-commit.c`, `writer.c`, `reader.c`). * **Safety**: Updates `gm_fs_path_normalize_logical` to use `gm_memcpy_span` for improved buffer safety.
…aware blob resolution This set of changes represents a significant step in isolating the core logic from the underlying Git implementation by migrating augment hooks to the gm_git_repository_port abstraction layer. This feature introduces commit-aware blob resolution to the port, allowing the hooks to look up file content (blobs) at previous commits (HEAD~N) without using raw libgit2 calls. This greatly enhances the system's testability using the new fake port functionality. Core Abstraction and API Changes The core architectural change is the introduction of a new method to the gm_git_repository_port: A new virtual function, resolve_blob_at_commit, is added to core/include/gitmind/ports/git_repository_port.h. This function is implemented in the production adapter (libgit2_repository_port.c) and allows resolving a blob OID for a specific file path at a given commit OID. The function signatures for gm_hook_get_blob_sha and gm_hook_process_changed_file in core/include/gitmind/hooks/augment.h are updated to accept the new const gm_git_repository_port_t *repo_port instead of the old, un-abstracted git_repository *repo. Hook Logic Refactoring The augment hook implementation (core/src/hooks/augment.c) is completely refactored to use the new port functionality: gm_hook_get_blob_sha no longer uses git_revparse_single, git_commit_lookup, or git_tree_entry_bypath. For HEAD lookups (offset 0), it uses the existing gm_git_repository_port_resolve_blob_at_head. For HEAD~N lookups (offset >0), it first uses gm_git_repository_port_walk_commits to find the target commit OID, and then calls the new gm_git_repository_port_resolve_blob_at_commit. gm_hook_process_changed_file is updated to pass the repo_port to subsequent blob-fetching calls, ensuring the entire augments pipeline adheres to the abstraction. The post-commit.c hook is also updated to use the new repo_port when processing all changed files. Testing and Documentation Fake Git Port: The fake_git_repository_port is updated with a new helper, gm_fake_git_repository_port_add_commit_blob_mapping, which allows tests to set up explicit blob mappings for specific commit OIDs, ensuring fine-grained test coverage for the commit-aware lookup logic. Architecture Documentation: The docs/architecture/augments-system.md file is updated with a new sequence diagram and detailed text to illustrate the new, clean flow: Post-Commit Hook ➜ gm_git_repository_port ➜ Git (libgit2). This documents the successful separation of concerns. Agent Log: The AGENTS.md log is updated to reflect the completion of the augment helper port migration.
…mit_parent_count
The diff represents a **feature implementation and migration** focused on fully isolating the Git functionality within the augmentation hooks from direct use of the `libgit2` library, moving it behind the **`gm_git_repository_port_t`** abstraction.
Here is a breakdown of the key changes in the provided diff:
* **`core/include/gitmind/ports/git_repository_port.h`**:
* Adds a new virtual table function (`vtbl` entry) to the `gm_git_repository_port_vtbl` struct: `commit_parent_count`.
* This function takes a `commit_oid` and a pointer to a `size_t` for the output.
* A corresponding inline wrapper function, **`gm_git_repository_port_commit_parent_count`**, is added for safe, abstracted access to this new functionality. This is the crucial step that enables merge detection without direct `libgit2` calls.
* **`core/src/adapters/git/libgit2_repository_port.c`**:
* Implements the new function, **`commit_parent_count_impl`**, which uses `libgit2`'s `git_commit_lookup` and **`git_commit_parentcount`** to get the number of parents for a given commit OID.
* The corresponding bridge function is added to the **`GM_LIBGIT2_REPOSITORY_PORT_VTBL`** to complete the concrete implementation for the `libgit2` adapter.
* **`core/include/gitmind/hooks/augment.h`**:
* Removes the direct inclusion of the `git2/types.h` header, eliminating a dependency on `libgit2` types in the public header.
* Updates the signature of **`gm_hook_is_merge_commit`** to accept the abstracted **`const gm_git_repository_port_t *repo_port`** instead of the concrete `git_repository *repo`.
* Updates the deprecated and preferred inline helper functions (`is_merge_commit`, `gm_augment_is_merge_commit`) to use the new port-based signature.
* **`core/src/hooks/augment.c`**:
* The implementation of **`gm_hook_is_merge_commit`** is completely rewritten to use the new **`gm_git_repository_port_t`**. It now resolves the HEAD branch, gets the tip OID using `gm_git_repository_port_reference_tip`, and finally calls the new **`gm_git_repository_port_commit_parent_count`** to determine if `parent_total > 1U`.
* **`core/src/hooks/post-commit.c`**:
* **Refactors the hook execution flow:** The `gm_libgit2_repository_port_create` call is moved up to initialize the `ctx.git_repo_port` immediately after the repository is initialized. This ensures the abstracted port is available early.
* The `should_process_commit` function is updated to take `const gm_git_repository_port_t *repo_port` and now calls the ported `is_merge_commit` (which uses the port).
* The logic to dispose of the repository port is centralized and performed whenever the hook exits (e.g., if it skips processing a merge or exits normally).
* **`core/tests/fakes/git/fake_git_repository_port.h`** and **`core/tests/fakes/git/fake_git_repository_port.c`**:
* The **fake Git adapter** is enhanced to support the new functionality.
* A **`gm_fake_git_commit_entry_t`** struct now includes **`parent_count`** and an array of `parents` OIDs.
* A new public function, **`gm_fake_git_repository_port_set_commit_parents`**, is added to allow tests to program the fake commit history.
* The **`fake_commit_parent_count`** implementation is added to return the programmed `parent_count`.
* A bug fix is included in **`fake_resolve_blob_at_head`** to check commit-scoped blob mappings when resolving HEAD, addressing an issue introduced in a previous commit's migration.
* **`core/tests/unit/test_hook_augment.c`**: (New File)
* Adds a new unit test suite specifically for the augment helpers using the new fake port.
* Tests cover:
* **`test_get_blob_sha_head_and_parent`**: Verifies both HEAD and HEAD~1 (parent) blob lookups via the port, confirming the blob resolution from the fake adapter works as expected.
* **`test_is_merge_commit_detection`**: Verifies that a commit explicitly set with two parents is correctly identified as a merge.
* **`test_is_merge_commit_linear_history`**: Verifies that a commit set with one parent is correctly identified as not a merge.
* **`meson.build`**:
* Adds the new test executable, **`test_hook_augment`**, to the build configuration and includes it in the list of tests to run.
In summary, this diff **completes the migration** of Git merge detection into the abstraction layer, strengthens the architecture, and adds robust unit test coverage to lock in the new port-based behavior.
Retire the legacy 'git_ops' and 'git_repo' fields from 'gm_context_t'.
This change completes the migration to a fully port-based core context,
ensuring 'gm_context_t' is now pure and depends only on abstracted ports.
The raw 'git_repository *' handle is now owned by the 'gm_cli_ctx_t'
runtime, which handles its initialization and final cleanup.
Key updates include:
- The core 'gm_context_t' struct is cleaned of raw git dependencies.
- CLI runtime ('main.c', 'cli_runtime.h') now holds and frees the raw repo
handle.
- The 'cache_rebuild' command is ported to use 'ctx->git_repo_port' to
retrieve the current branch name.
- Cache, journal, and benchmark tests are updated to remove assignments
to the now-defunct 'ctx->git_repo'.
This update ports the two remaining direct usages of the cache query fanout function in the benchmark suite. The function signature for 'gm_cache_query_fanout' was previously updated to accept 'gm_context_t *' instead of the raw 'git_repository *'. This change ensures the benchmark code is compatible with the new port-based core context architecture.
WalkthroughIntroduces a Git repository port abstraction and a LibGit2 adapter, refactors CLI, cache, hooks, journal, and tests to use the port, updates context structures and headers, removes direct libgit2 usages in core logic, replaces tree builder/size with port calls, adds fakes for testing, updates docs and build/test wiring. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI
participant Ctx as Context (gm_context_t)
participant Port as RepoPort (gm_git_repository_port)
participant L2 as Adapter: LibGit2
participant Git as Git (libgit2)
CLI->>L2: gm_libgit2_repository_port_create(repo*)
L2-->>CLI: gm_git_repository_port (+dispose)
CLI->>Ctx: set git_repo_port, disposer
Note over Ctx,Port: Runtime operations via port vtbl
Ctx->>Port: head_branch(out_name)
Port->>L2: vtbl.head_branch(self,…)
L2->>Git: resolve HEAD branch
Git-->>L2: branch name
L2-->>Port: ok
Port-->>Ctx: branch name
Ctx->>Port: reference_tip(ref, out_tip)
Port->>L2: vtbl.reference_tip(self,…)
L2->>Git: lookup ref tip
Git-->>L2: tip OID + when
L2-->>Ctx: tip
Ctx->>Port: commit_read_blob(commit_oid, path)
Port->>L2: vtbl.commit_read_blob(self,…)
L2->>Git: load blob
Git-->>L2: data ptr/size
L2-->>Ctx: blob data
Ctx-->>Port: blob_dispose(data,size)
CLI-->>L2: dispose(port) at shutdown
sequenceDiagram
autonumber
participant App as Cache Rebuild
participant Port as RepoPort
participant FS as FS Temp Port
App->>Port: reference_tip("refs/heads/<branch>", out_tip)
alt no tip
App->>App: handle not found
else tip found
App->>FS: build temp dir contents
App->>Port: build_tree_from_directory(tmpdir, out_tree)
App->>Port: commit_create(spec(tree, parents, author))
App->>Port: reference_update(update_spec)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120–180 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
92a5928 to
258c9f0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/include/gitmind/security/string.h (1)
73-80: Restore printf-format attribute ongm_snprintf.Switching to a
static inlinedefinition dropped the GCC/Clangformat(printf, 3, 4)attribute, so we now lose compile-time checking of format strings vs. arguments across every callsite. That’s a regression in the header’s safety contract. Please keep the attribute when inlining.-/* Safe snprintf wrapper that suppresses security warnings */ -static inline int gm_snprintf(char *str, size_t size, const char *format, ...) { +/* Safe snprintf wrapper that suppresses security warnings */ +#if defined(__GNUC__) || defined(__clang__) +static inline __attribute__((format(printf, 3, 4))) +#else +static inline +#endif +int gm_snprintf(char *str, size_t size, const char *format, ...) {docs/architecture/augments-system.md (1)
13-19: Promote “Table of Contents” to the mandated H2 headingThe docs style guide calls for a literal
## Table of Contentssection header. Right now it’s plain text, so the build-time doc checks will fail. Please update the heading accordingly. As per coding guidelines-Table of Contents +## Table of Contents
🧹 Nitpick comments (4)
core/include/gitmind/cache.h (1)
51-53: Clarify the tail-layout commentThe note about “Added at struct tail to preserve ABI layout” still sits on
journal_tip_oid_bin, butcache_tip_oidis now the final field. Please move or update the comment so future readers don’t get misled about which member is guaranteed to live at the tail.Apply this diff to relocate the comment:
- gm_oid_t journal_tip_oid_bin; /**< OID of last processed journal commit (preferred). Added at struct tail to preserve ABI layout. */ - gm_oid_t cache_tip_oid; /**< OID of cache commit for this branch. */ + gm_oid_t journal_tip_oid_bin; /**< OID of last processed journal commit (preferred). */ + gm_oid_t cache_tip_oid; /**< OID of cache commit for this branch. Added at struct tail to preserve ABI layout. */apps/cli/main.md (1)
99-110: Align cleanup snippet with new repo ownershipThis section now hands ownership of the raw handle to
cli->repo, but the cleanup example later still frees the localrepovariable. Please update the cleanup snippet to freecli->reposo the doc stays internally consistent with the new lifecycle.- git_repository_free(repo); + git_repository_free(cli->repo);core/src/edge/edge.c (1)
16-18: Depend on the port interface, not the libgit2 adapterEdge code should stay adapter-agnostic. Pulling in the libgit2 adapter header couples core logic to a specific transport. Include the generic port interface instead so we keep the layering intact.
As per coding guidelines, apply this diff:
-#include "gitmind/context.h" -#include "gitmind/result.h" -#include "gitmind/adapters/git/libgit2_repository_port.h" +#include "gitmind/context.h" +#include "gitmind/result.h" +#include "gitmind/ports/git_repository_port.h"core/include/gitmind/hooks/augment.h (1)
80-85: Fix outdated parameter docstringThe comment still refers to a
git_repositoryeven though the signature now expects the repository port. Updating the docstring will keep the public header accurate.- * @param repo Git repository + * @param repo_port Repository port adapter
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
.github/labeler.yml(0 hunks).github/workflows/label-docs-only.yml(0 hunks)AGENTS.md(2 hunks)apps/cli/cache_rebuild.c(2 hunks)apps/cli/cli_runtime.h(1 hunks)apps/cli/main.c(4 hunks)apps/cli/main.md(1 hunks)benchmarks/wildebeest_stampede.c(5 hunks)core/include/gitmind/adapters/fs/posix_temp_adapter.h(1 hunks)core/include/gitmind/adapters/git/libgit2_repository_port.h(1 hunks)core/include/gitmind/cache.h(2 hunks)core/include/gitmind/context.h(2 hunks)core/include/gitmind/hooks/augment.h(6 hunks)core/include/gitmind/ports/env_port.h(0 hunks)core/include/gitmind/ports/fs_temp_port.h(2 hunks)core/include/gitmind/ports/git_repository_port.h(1 hunks)core/include/gitmind/security/string.h(1 hunks)core/src/adapters/config/env_adapter.c(1 hunks)core/src/adapters/git/libgit2_repository_port.c(1 hunks)core/src/app/cache/cache_rebuild_service.c(11 hunks)core/src/cache/builder.c(1 hunks)core/src/cache/cache_internal.h(0 hunks)core/src/cache/query.c(7 hunks)core/src/cache/tree_builder.c(0 hunks)core/src/cache/tree_size.c(0 hunks)core/src/edge/attributed.c(2 hunks)core/src/edge/edge.c(2 hunks)core/src/fs/path_utils.c(5 hunks)core/src/hooks/augment.c(4 hunks)core/src/hooks/post-commit.c(5 hunks)core/src/journal/reader.c(7 hunks)core/src/journal/writer.c(7 hunks)core/src/ports/env_port.c(1 hunks)core/tests/fakes/fs/fake_fs_temp_port.c(4 hunks)core/tests/fakes/git/fake_git_repository_port.c(1 hunks)core/tests/fakes/git/fake_git_repository_port.h(1 hunks)core/tests/unit/test_cache_branch_limits.c(3 hunks)core/tests/unit/test_cache_edge_map.c(1 hunks)core/tests/unit/test_cache_meta.c(2 hunks)core/tests/unit/test_cache_query.c(3 hunks)core/tests/unit/test_cache_tree_size.c(3 hunks)core/tests/unit/test_edge.c(3 hunks)core/tests/unit/test_hook_augment.c(1 hunks)core/tests/unit/test_journal_mixed_cbor.c(3 hunks)docs/architecture/augments-system.md(5 hunks)docs/architecture/cache/builder.md(1 hunks)docs/code-reviews/PR170/1568a39eaebc49ac1c59409f664cecfa16a9e1e7.md(0 hunks)docs/code-reviews/PR170/857b69c2942bc6221d758387cbcc214a74153973.md(0 hunks)docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md(1 hunks)docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.md(1 hunks)meson.build(3 hunks)
💤 Files with no reviewable changes (8)
- .github/workflows/label-docs-only.yml
- docs/code-reviews/PR170/1568a39eaebc49ac1c59409f664cecfa16a9e1e7.md
- core/src/cache/cache_internal.h
- .github/labeler.yml
- docs/code-reviews/PR170/857b69c2942bc6221d758387cbcc214a74153973.md
- core/src/cache/tree_builder.c
- core/src/cache/tree_size.c
- core/include/gitmind/ports/env_port.h
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{c,cc,cpp,cxx,h,hpp,hxx,m,mm}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use
NOLINTto suppress clang-tidy warnings; ALWAYS fix the underlying issue
Files:
core/tests/unit/test_cache_edge_map.ccore/tests/fakes/git/fake_git_repository_port.ccore/include/gitmind/adapters/git/libgit2_repository_port.hcore/include/gitmind/cache.hcore/tests/unit/test_hook_augment.ccore/tests/unit/test_cache_branch_limits.ccore/src/edge/edge.ccore/src/adapters/git/libgit2_repository_port.ccore/src/edge/attributed.ccore/include/gitmind/security/string.happs/cli/main.ccore/include/gitmind/ports/fs_temp_port.hcore/src/fs/path_utils.ccore/src/journal/writer.ccore/src/cache/builder.ccore/src/adapters/config/env_adapter.ccore/tests/unit/test_cache_meta.cbenchmarks/wildebeest_stampede.ccore/src/hooks/post-commit.ccore/include/gitmind/hooks/augment.hcore/include/gitmind/adapters/fs/posix_temp_adapter.happs/cli/cli_runtime.hcore/src/app/cache/cache_rebuild_service.capps/cli/cache_rebuild.ccore/src/ports/env_port.ccore/tests/unit/test_edge.ccore/include/gitmind/ports/git_repository_port.hcore/src/hooks/augment.ccore/include/gitmind/context.hcore/tests/fakes/git/fake_git_repository_port.hcore/tests/unit/test_journal_mixed_cbor.ccore/tests/fakes/fs/fake_fs_temp_port.ccore/src/cache/query.ccore/tests/unit/test_cache_query.ccore/tests/unit/test_cache_tree_size.ccore/src/journal/reader.c
{core,include}/**/*.{c,h}
📄 CodeRabbit inference engine (AGENTS.md)
{core,include}/**/*.{c,h}: Use C23 with warnings-as-errors; no VLAs or shadowing; declare explicit prototypes
Formatting must follow .clang-format (LLVM-based, 4 spaces, 80 cols, right-aligned pointers)
Naming: functions/vars lower_snake_case with gm_ prefix; macros UPPER_SNAKE; types end with _t
Prefer specific includes; order/regroup includes per .clang-format
Use OID-first equality and APIs; when both OIDs are present they must match; compare using git_oid_cmp
Construct refs via gm_build_ref, reject inputs starting with 'refs/' and enforce ref-name safety
Use safe string/memory functions (gm_snprintf, gm_strcpy_safe, gm_memcpy_span); check return codes; treat truncation as error; zero output buffers on error paths
Prefer GM_PATH_MAX; do not use GM_PATH_MAX*2; allocate if larger is required
Keep touched code SRP-compliant and injection-friendly: isolate side effects behind outbound ports and pass dependencies explicitly
Keep clang-tidy at zero warnings for touched files; do not merge new diagnostics
Zero out output buffers on failure before formatting or copying to prevent partial writes
Files:
core/tests/unit/test_cache_edge_map.ccore/tests/fakes/git/fake_git_repository_port.ccore/include/gitmind/adapters/git/libgit2_repository_port.hcore/include/gitmind/cache.hcore/tests/unit/test_hook_augment.ccore/tests/unit/test_cache_branch_limits.ccore/src/edge/edge.ccore/src/adapters/git/libgit2_repository_port.ccore/src/edge/attributed.ccore/include/gitmind/security/string.hcore/include/gitmind/ports/fs_temp_port.hcore/src/fs/path_utils.ccore/src/journal/writer.ccore/src/cache/builder.ccore/src/adapters/config/env_adapter.ccore/tests/unit/test_cache_meta.ccore/src/hooks/post-commit.ccore/include/gitmind/hooks/augment.hcore/include/gitmind/adapters/fs/posix_temp_adapter.hcore/src/app/cache/cache_rebuild_service.ccore/src/ports/env_port.ccore/tests/unit/test_edge.ccore/include/gitmind/ports/git_repository_port.hcore/src/hooks/augment.ccore/include/gitmind/context.hcore/tests/fakes/git/fake_git_repository_port.hcore/tests/unit/test_journal_mixed_cbor.ccore/tests/fakes/fs/fake_fs_temp_port.ccore/src/cache/query.ccore/tests/unit/test_cache_query.ccore/tests/unit/test_cache_tree_size.ccore/src/journal/reader.c
core/tests/unit/test_*.c
📄 CodeRabbit inference engine (AGENTS.md)
Unit tests must live under core/tests/unit as test_.c, be deterministic and isolated
Files:
core/tests/unit/test_cache_edge_map.ccore/tests/unit/test_hook_augment.ccore/tests/unit/test_cache_branch_limits.ccore/tests/unit/test_cache_meta.ccore/tests/unit/test_edge.ccore/tests/unit/test_journal_mixed_cbor.ccore/tests/unit/test_cache_query.ccore/tests/unit/test_cache_tree_size.c
core/tests/fakes/**
📄 CodeRabbit inference engine (AGENTS.md)
Provide deterministic fakes/mocks/stubs for each port under core/tests/fakes
Files:
core/tests/fakes/git/fake_git_repository_port.ccore/tests/fakes/git/fake_git_repository_port.hcore/tests/fakes/fs/fake_fs_temp_port.c
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Run
make docs-verifywhen changing documentation
Files:
docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.mddocs/architecture/augments-system.mddocs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.mddocs/architecture/cache/builder.md
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Docs must include front matter first, a single H1 whose title matches the filename title, a '## Table of Contents' section, and API docs must include api_version; add License/SPDX comments after front matter
Files:
docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.mddocs/architecture/augments-system.mddocs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.mddocs/architecture/cache/builder.md
{include,core/include}/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
{include,core/include}/**/*.h: Public headers must be umbrella-safe: compile standalone, include directly, have stable header guards, and provide C++ linkage guards
Header guards must follow GITMIND_* naming and remain stable
Ensure public headers compile standalone via header-compile checks
Files:
core/include/gitmind/adapters/git/libgit2_repository_port.hcore/include/gitmind/cache.hcore/include/gitmind/security/string.hcore/include/gitmind/ports/fs_temp_port.hcore/include/gitmind/hooks/augment.hcore/include/gitmind/adapters/fs/posix_temp_adapter.hcore/include/gitmind/ports/git_repository_port.hcore/include/gitmind/context.h
core/src/adapters/**
📄 CodeRabbit inference engine (AGENTS.md)
Each adapter must publish a factory returning the port vtable plus state, and provide teardown hooks
Files:
core/src/adapters/git/libgit2_repository_port.ccore/src/adapters/config/env_adapter.c
core/include/gitmind/ports/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
Ports headers must declare interfaces only (no non-trivial inline behavior) and use gm_result_t signatures
Files:
core/include/gitmind/ports/fs_temp_port.hcore/include/gitmind/ports/git_repository_port.h
core/src/adapters/**/*_adapter.{c,h}
📄 CodeRabbit inference engine (AGENTS.md)
Runtime adapters must be placed under core/src/adapters// with filenames ending in _adapter and paired private headers
Files:
core/src/adapters/config/env_adapter.c
core/include/gitmind/hooks/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
Hooks APIs must surface gm_result_t results and avoid raw integer returns
Files:
core/include/gitmind/hooks/augment.h
🧠 Learnings (20)
📓 Common learnings
Learnt from: flyingrobots
PR: neuroglyph/git-mind#169
File: docs/templates/code-review-feedback.md:26-33
Timestamp: 2025-09-16T17:10:27.477Z
Learning: The user (flyingrobots) uses Obsidian to view/edit Markdown files and prefers to keep Obsidian-specific syntax like collapsible callout markers (e.g., `[!NOTE]-` for collapsible callouts) even if they're not GitHub-compatible.
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to .coderabbit.yml : Maintain .coderabbit.yml to prefer summary-first reviews and limit per-line comments on docs
Applied to files:
docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.mddocs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.md
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to core/include/gitmind/ports/**/*.h : Ports headers must declare interfaces only (no non-trivial inline behavior) and use gm_result_t signatures
Applied to files:
core/include/gitmind/adapters/git/libgit2_repository_port.hcore/src/journal/writer.ccore/src/adapters/config/env_adapter.ccore/include/gitmind/hooks/augment.hcore/include/gitmind/adapters/fs/posix_temp_adapter.hcore/src/app/cache/cache_rebuild_service.ccore/src/ports/env_port.ccore/include/gitmind/ports/git_repository_port.hcore/include/gitmind/context.hcore/tests/fakes/git/fake_git_repository_port.hcore/src/journal/reader.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/cache/{builder,query}.c : Keep naming consistent with OID terminology (e.g., OID_HASH_MULTIPLIER, oid_hash) after migration from SHA
Applied to files:
core/include/gitmind/cache.h
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/cache/{builder,query}.c : Version and migrate cache formats when changing the hash function (e.g., raw OID vs SHA); auto-rebuild or reject mismatched versions
Applied to files:
core/include/gitmind/cache.hcore/src/cache/builder.ccore/src/app/cache/cache_rebuild_service.cdocs/architecture/cache/builder.mdcore/src/cache/query.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/edge.h : Avoid struct bloat: do not store both legacy SHA arrays and OIDs long-term in gm_edge_t; deprecate and phase out legacy SHA fields
Applied to files:
core/include/gitmind/cache.hcore/src/edge/edge.ccore/src/edge/attributed.ccore/tests/unit/test_edge.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/hooks/augment.h : Prefix public hook APIs with gm_hook_ and provide deprecation warnings for legacy macro/function names with a clear removal timeline
Applied to files:
core/tests/unit/test_hook_augment.ccore/include/gitmind/hooks/augment.hcore/src/hooks/augment.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/hooks/augment.h : Maintain stable header guards or provide compatibility for guard renames (GM_AUGMENT_H → GITMIND_AUGMENT_H) to avoid breaking includes
Applied to files:
core/tests/unit/test_hook_augment.ccore/include/gitmind/hooks/augment.hcore/src/hooks/augment.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/edge/edge.c : gm_edge_equal() must compare OIDs first and only fall back to SHA when OIDs are unset/zero; never treat mismatched OIDs as equal due to matching SHAs
Applied to files:
core/src/edge/edge.ccore/src/edge/attributed.ccore/tests/unit/test_edge.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Use safe string/memory functions (gm_snprintf, gm_strcpy_safe, gm_memcpy_span); check return codes; treat truncation as error; zero output buffers on error paths
Applied to files:
core/include/gitmind/security/string.hcore/src/fs/path_utils.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/**/*.{c,h} : Use bounded string functions (e.g., gm_snprintf) and check return values for overflow; ensure explicit NUL termination
Applied to files:
core/include/gitmind/security/string.hcore/src/fs/path_utils.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to include/gitmind.h : Maintain umbrella API at include/gitmind.h
Applied to files:
apps/cli/main.ccore/src/fs/path_utils.ccore/src/journal/writer.ccore/src/adapters/config/env_adapter.cbenchmarks/wildebeest_stampede.ccore/include/gitmind/hooks/augment.hcore/include/gitmind/adapters/fs/posix_temp_adapter.happs/cli/cli_runtime.hcore/src/app/cache/cache_rebuild_service.capps/cli/cache_rebuild.ccore/src/hooks/augment.ccore/include/gitmind/context.hcore/tests/unit/test_journal_mixed_cbor.ccore/src/cache/query.ccore/src/journal/reader.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to include/gitmind/**/*.h : Public, namespaced headers live under include/gitmind/
Applied to files:
apps/cli/main.ccore/include/gitmind/adapters/fs/posix_temp_adapter.h
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Prefer GM_PATH_MAX; do not use GM_PATH_MAX*2; allocate if larger is required
Applied to files:
core/src/fs/path_utils.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to core/include/gitmind/hooks/**/*.h : Hooks APIs must surface gm_result_t results and avoid raw integer returns
Applied to files:
core/src/fs/path_utils.ccore/include/gitmind/hooks/augment.hcore/src/hooks/augment.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/journal/{reader,writer}.c : Eliminate CBOR magic numbers; use centralized keys from gitmind/cbor/keys.h
Applied to files:
core/src/journal/writer.ccore/src/journal/reader.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Keep touched code SRP-compliant and injection-friendly: isolate side effects behind outbound ports and pass dependencies explicitly
Applied to files:
core/src/ports/env_port.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/cbor/keys.h : Define CBOR field keys as named constants/enums in a single header and include it where parsing/encoding occurs
Applied to files:
core/src/hooks/augment.ccore/src/journal/reader.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to {include,core/include}/**/*.h : Header guards use GITMIND_* naming
Applied to files:
core/src/cache/query.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Use OID-first equality and APIs; when both OIDs are present they must match; compare using git_oid_cmp
Applied to files:
core/src/journal/reader.c
🧬 Code graph analysis (35)
core/tests/fakes/git/fake_git_repository_port.c (2)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/include/gitmind/adapters/git/libgit2_repository_port.h (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/include/gitmind/ports/env_port.h (1)
struct gm_env_port {(18-21)
core/include/gitmind/cache.h (1)
core/include/gitmind/cache/cache.h (2)
GM_CACHE_OID_STRING_SIZE(32-40)struct {(43-47)
core/tests/unit/test_hook_augment.c (2)
core/tests/fakes/git/fake_git_repository_port.c (6)
gm_fake_git_repository_port_init(194-228)gm_fake_git_repository_port_set_head_branch(246-260)gm_fake_git_repository_port_add_ref_commit(271-305)gm_fake_git_repository_port_set_tip(238-244)gm_fake_git_repository_port_add_commit_blob_mapping(357-397)gm_fake_git_repository_port_dispose(230-236)core/src/hooks/augment.c (2)
gm_hook_get_blob_sha(96-157)gm_hook_is_merge_commit(295-337)
core/tests/unit/test_cache_branch_limits.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/adapters/fs/posix_temp_adapter.c (1)
dispose_port(615-624)
core/src/edge/edge.c (6)
core/src/edge/attributed.c (1)
resolve_blob_identity(42-70)core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)core/include/gitmind/edge.h (1)
GM_SHA1_SIZE(32-45)core/include/gitmind/edge_attributed.h (1)
GM_SHA1_SIZE(30-48)core/tests/unit/test_edge_equal_semantics.c (1)
test_fallback_to_sha(29-42)core/tests/unit/test_edge_attributed_cbor.c (1)
test_legacy_backfill(56-111)
core/src/adapters/git/libgit2_repository_port.c (3)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/include/gitmind/security/string.h (1)
gm_snprintf(74-80)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/src/edge/attributed.c (4)
core/src/edge/edge.c (1)
resolve_blob_identity(55-83)core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)core/include/gitmind/edge.h (1)
GM_SHA1_SIZE(32-45)core/include/gitmind/edge_attributed.h (1)
GM_SHA1_SIZE(30-48)
core/include/gitmind/security/string.h (1)
core/include/gitmind/security/memory.h (4)
gm_memcpy(62-64)gm_memset(66-68)gm_memcpy_safe(23-35)gm_memset_safe(47-59)
apps/cli/main.c (3)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/error/error.c (1)
gm_error_free(161-178)apps/cli/output.c (1)
gm_output_create(11-22)
core/include/gitmind/ports/fs_temp_port.h (1)
core/src/adapters/fs/posix_temp_adapter.c (3)
join_under_base_bridge(590-598)join_under_base(488-519)GM_PATH_MAX(28-34)
core/src/fs/path_utils.c (1)
core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)
core/src/journal/writer.c (2)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/src/adapters/config/env_adapter.c (1)
core/include/gitmind/error.h (4)
struct gm_error(36-36)GM_ERROR_SMALL_SIZE(25-39)GM_OK(46-84)GM_ERROR_SMALL_SIZE(27-30)
core/tests/unit/test_cache_meta.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/cache/query.c (1)
gm_cache_load_meta(58-138)
benchmarks/wildebeest_stampede.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/cache/query.c (1)
gm_cache_query_fanout(373-377)
core/src/hooks/post-commit.c (4)
core/include/gitmind/hooks/augment.h (2)
is_merge_commit(111-114)process_changed_file(105-109)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/error/error.c (1)
gm_error_free(161-178)
core/include/gitmind/hooks/augment.h (1)
core/src/hooks/augment.c (3)
gm_hook_get_blob_sha(96-157)gm_hook_process_changed_file(252-292)gm_hook_is_merge_commit(295-337)
core/include/gitmind/adapters/fs/posix_temp_adapter.h (2)
core/src/adapters/fs/posix_temp_adapter.c (5)
GM_PATH_MAX(28-34)struct stat(74-74)make_temp_dir_bridge(577-583)struct stat(420-420)ensure_dir_exists(68-112)core/include/gitmind/result.h (1)
struct gm_error(12-12)
core/src/app/cache/cache_rebuild_service.c (2)
core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)
apps/cli/cache_rebuild.c (2)
core/src/error/error.c (1)
gm_error_free(161-178)apps/cli/output.c (1)
gm_output_error(65-70)
core/src/ports/env_port.c (2)
core/include/gitmind/ports/env_port.h (4)
struct gm_env_port {(18-21)struct gm_env_port_vtbl(16-16)struct gm_env_port_vtbl {(26-28)struct gm_env_port_vtbl(20-20)core/include/gitmind/error.h (2)
GM_ERROR_SMALL_SIZE(25-39)struct gm_error(36-36)
docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.md (1)
tools/review/seed_feedback_from_github.py (1)
main(50-134)
apps/cli/main.md (1)
core/include/gitmind/ports/env_port.h (1)
struct gm_env_port {(18-21)
core/tests/unit/test_edge.c (1)
core/tests/fakes/git/fake_git_repository_port.c (5)
gm_fake_git_repository_port_init(194-228)gm_fake_git_repository_port_set_head_branch(246-260)gm_fake_git_repository_port_clear_blob_mappings(307-314)gm_fake_git_repository_port_add_blob_mapping(316-355)gm_fake_git_repository_port_dispose(230-236)
core/include/gitmind/ports/git_repository_port.h (1)
core/include/gitmind/ports/fs_temp_port.h (6)
gm_result_void_t(76-84)gm_result_void_t(86-97)gm_result_void_t(99-108)gm_result_void_t(110-124)gm_result_void_t(126-138)gm_result_void_t(140-147)
core/src/hooks/augment.c (3)
core/src/error/error.c (1)
gm_error_free(161-178)core/src/util/ref.c (1)
gm_build_ref(47-84)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/include/gitmind/context.h (2)
core/include/gitmind/ports/env_port.h (1)
struct gm_env_port {(18-21)core/include/gitmind/io/io.h (1)
struct gm_io_ops {(73-78)
core/tests/fakes/git/fake_git_repository_port.h (2)
core/include/gitmind/ports/git_repository_port.h (14)
gm_result_void_t(99-109)gm_result_void_t(111-119)gm_result_void_t(121-132)gm_result_void_t(134-143)gm_result_void_t(145-155)gm_result_void_t(157-169)gm_result_void_t(171-181)gm_result_void_t(193-205)gm_result_void_t(207-217)gm_result_void_t(219-228)gm_result_void_t(230-239)gm_result_void_t(241-251)gm_result_void_t(253-264)gm_result_void_t(266-277)core/tests/fakes/git/fake_git_repository_port.c (18)
gm_fake_git_repository_port_init(194-228)gm_fake_git_repository_port_dispose(230-236)gm_fake_git_repository_port_set_tip(238-244)gm_fake_git_repository_port_set_head_branch(246-260)gm_fake_git_repository_port_clear_ref_commits(262-269)gm_fake_git_repository_port_add_ref_commit(271-305)gm_fake_git_repository_port_clear_blob_mappings(307-314)gm_fake_git_repository_port_add_blob_mapping(316-355)gm_fake_git_repository_port_add_commit_blob_mapping(357-397)gm_fake_git_repository_port_set_commit_parents(399-431)gm_fake_git_repository_port_set_next_tree(433-446)gm_fake_git_repository_port_set_next_commit(448-461)gm_fake_git_repository_port_set_update_result(463-469)gm_fake_git_repository_port_last_commit_message(471-474)gm_fake_git_repository_port_last_commit_tree(476-479)gm_fake_git_repository_port_last_update_ref(481-484)gm_fake_git_repository_port_last_update_log(486-489)gm_fake_git_repository_port_last_update_target(491-494)
core/tests/unit/test_journal_mixed_cbor.c (3)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/tests/unit/test_fs_temp_port.c (1)
main(24-167)core/src/adapters/fs/posix_temp_adapter.c (1)
dispose_port(615-624)
core/tests/fakes/fs/fake_fs_temp_port.c (1)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)
core/src/cache/query.c (6)
core/src/error/error.c (1)
gm_error_free(161-178)core/src/util/ref.c (1)
gm_build_ref(47-84)core/include/gitmind/security/string.h (1)
gm_snprintf(74-80)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/src/cache/bitmap.c (1)
gm_bitmap_deserialize(84-109)
core/tests/unit/test_cache_query.c (1)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)
core/tests/unit/test_cache_tree_size.c (1)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)
core/src/journal/reader.c (2)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/ports/git_repository_port.h (1)
gm_git_repository_port_commit_message_dispose(183-191)
🔇 Additional comments (5)
AGENTS.md (2)
83-85: Updated lessons read clearly.The new guidance on wiring and faking ports is concise and keeps the Obsidian-friendly callout flow intact. Nicely done.
418-433: Great addition to the activity log.Capturing the repository-port migration details here will save future agents a lot of archeology.
core/src/ports/env_port.c (1)
5-7: Thanks for pulling in the missing headers.Adding the error/result includes (and
<stddef.h>forsize_t) keeps this port helper self-contained and warning-free.core/tests/unit/test_hook_augment.c (1)
21-75: Great HEAD/HEAD~1 coverageThanks for exercising both
HEADandHEAD~1; this locks in the commit-walk behaviour on the new port façade nicely.core/include/gitmind/ports/git_repository_port.h (1)
59-97: Comprehensive vtable surface looks solidNice job capturing the full repository interaction surface in the vtable—with consistent
gm_result_void_tsignatures and clear separation of concerns, this matches the porting guidelines well.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/cli/main.md (1)
25-103: Fix the repository port creation snippet.The snippet now shows
gm_libgit2_repository_port_create(&ctx.git_repo_port, NULL, &ctx.git_repo_port_dispose, repo);, but the actual API isgm_libgit2_repository_port_create(gm_git_repository_port_t *port, void **state_out, void (**dispose_out)(void *), git_repository *repo). Omitting the state argument shifts the rest of the parameters, so anyone copying the snippet will pass the disposer pointer where the state output belongs and leave the disposer uninitialized. Please update the documentation example to match the live signature.core/src/edge/attributed.c (1)
242-321: Missing include for repository port API.The new
resolve_blob_identityhelper callsgm_git_repository_port_resolve_blob_at_head, but this file does not include the header that declares that function (e.g.,gitmind/ports/git_repository_port.h). With warnings-as-errors, the implicit declaration becomes a compile failure. Please add the proper include.core/src/app/cache/cache_rebuild_service.c (2)
138-144: Zero out buffers on error and add null checks in oid_to_hex.Pre-clear the output and validate inputs; also helps satisfy “zero output buffers on failure”.
As per coding guidelines
static int oid_to_hex(const gm_oid_t *oid, char *out, size_t out_size) { - if (out_size < SHA_HEX_SIZE) { - return GM_ERR_INVALID_ARGUMENT; - } - (void)git_oid_tostr(out, out_size, oid); - return GM_OK; + if (out == NULL || out_size == 0) { + return GM_ERR_INVALID_ARGUMENT; + } + out[0] = '\0'; + if (out_size < SHA_HEX_SIZE) { + return GM_ERR_INVALID_ARGUMENT; + } + (void)git_oid_tostr(out, out_size, oid); + return GM_OK; }Also consider replacing
git_oid_tostrwith a gm-level helper to avoid libgit2 coupling.
125-136: Extract and reuse a core-level OID→hex helper
- Move hex-format logic into a central utility, e.g.
gm_oid_to_hex(const gm_oid_t *oid, char *out, size_t out_size), implemented manually (bitwise) incore/src/utils/oid.c+ header.- Replace all
git_oid_fmt/git_oid_tostrcalls incore/src/app/cache/cache_rebuild_service.c(lines 125–136) andcore/src/cache/query.c(lines 38–42) withgm_oid_to_hex.- Drop the local
oid_to_hexstub incache_rebuild_service.cthat still invokes libgit2.core/src/fs/path_utils.c (1)
172-176: Remove cognitive-complexity suppression; add guard for inout_len vs buffer_size.
- Drop the NOLINT.
- Validate that *inout_len ≤ buffer_size before any read/write to base_io.
Apply this diff to remove the suppression:
-// NOLINTNEXTLINE(readability-function-cognitive-complexity) GM_NODISCARD gm_result_void_t gm_fs_path_basename_append(char *base_io, size_t buffer_size, size_t *inout_len, const char *source_path) {Add this guard just after the parameter check (outside this hunk):
size_t base_len = *inout_len; if (base_len > buffer_size) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "basename append length exceeds buffer")); }As per coding guidelines
🧹 Nitpick comments (12)
core/include/gitmind/hooks/augment.h (1)
80-83: Update parameter docstring to match the new signature.The doc block still references a raw repository pointer, but the function now takes a repository port adapter. Refresh the tag to avoid confusing downstream readers.
core/tests/fakes/git/fake_git_repository_port.c (5)
114-120: Use gm_memset_safe instead of memset for consistency.As per coding guidelines for the core directory, use the secure memory functions consistently.
Apply this diff:
- memset(free_slot, 0, sizeof(*free_slot)); + gm_memset_safe(free_slot, sizeof(*free_slot), 0, sizeof(*free_slot));Also on Line 117:
- memset(free_slot, 0, sizeof(*free_slot)); + gm_memset_safe(free_slot, sizeof(*free_slot), 0, sizeof(*free_slot));
202-202: Use gm_memset_safe instead of memset for consistency.Replace the standard memset with the secure variant.
Apply this diff:
- memset(fake, 0, sizeof(*fake)); + gm_memset_safe(fake, sizeof(*fake), 0, sizeof(*fake));
561-564: Use gm_memset_safe instead of memset for consistency.Replace standard memset usage with the secure variant throughout the fake implementation.
Apply this diff:
- memset(out_tree_oid, 0, sizeof(*out_tree_oid)); + gm_memset_safe(out_tree_oid, sizeof(*out_tree_oid), 0, sizeof(*out_tree_oid));
719-719: Use gm_memset_safe instead of memset for consistency.Replace the memset call with the secure variant.
Apply this diff:
- memset(out_commit_oid, 0, sizeof(*out_commit_oid)); + gm_memset_safe(out_commit_oid, sizeof(*out_commit_oid), 0, sizeof(*out_commit_oid));
636-644: Consider using gm_strcpy_safe for memory allocation size calculation.The manual strlen + malloc + memcpy pattern could be simplified and made more consistent with the codebase's string handling practices.
Consider this approach:
- const size_t message_len = strlen(commit->message); - char *copy = (char *)malloc(message_len + 1U); - if (copy == NULL) { - *out_message = NULL; - return gm_err_void( - GM_ERROR(GM_ERR_OUT_OF_MEMORY, "allocating fake commit message")); - } - - memcpy(copy, commit->message, message_len + 1U); + const size_t message_len = strlen(commit->message); + char *copy = (char *)malloc(message_len + 1U); + if (copy == NULL) { + *out_message = NULL; + return gm_err_void( + GM_ERROR(GM_ERR_OUT_OF_MEMORY, "allocating fake commit message")); + } + + gm_strcpy_safe(copy, message_len + 1U, commit->message);core/src/edge/edge.c (1)
73-80: Avoid duplicating blob-identity helpers across edge modules
resolve_blob_identityhere is byte-for-byte identical to the helper already living incore/src/edge/attributed.c. Let's consolidate this into a shared static inline (or dedicated utility) so we have a single implementation to maintain.core/src/hooks/post-commit.c (1)
211-214: Extract helper to dispose the repo portThe three identical dispose blocks make it easy to miss an update later. Consider a small helper so the hook has a single, consistent shutdown path.
@@ static void free_changed_files(char **changed_files, size_t file_count) { if (!changed_files) return; @@ free(changed_files); } + +static void dispose_repo_port(gm_context_t *ctx) { + if (ctx->git_repo_port_dispose != NULL) { + ctx->git_repo_port_dispose(&ctx->git_repo_port); + ctx->git_repo_port_dispose = NULL; + } +} @@ - if (!should_process_commit(&ctx.git_repo_port, verbose)) { - if (ctx.git_repo_port_dispose != NULL) { - ctx.git_repo_port_dispose(&ctx.git_repo_port); - ctx.git_repo_port_dispose = NULL; - } + if (!should_process_commit(&ctx.git_repo_port, verbose)) { + dispose_repo_port(&ctx); @@ if (error != GM_OK) { if (verbose) { fprintf(stderr, "Failed to get changed files\n"); } - if (ctx.git_repo_port_dispose != NULL) { - ctx.git_repo_port_dispose(&ctx.git_repo_port); - ctx.git_repo_port_dispose = NULL; - } + dispose_repo_port(&ctx); @@ - if (ctx.git_repo_port_dispose != NULL) { - ctx.git_repo_port_dispose(&ctx.git_repo_port); - ctx.git_repo_port_dispose = NULL; - } + dispose_repo_port(&ctx);Also applies to: 227-229, 245-247
core/src/app/cache/cache_rebuild_service.c (2)
331-347: Minor: check gm_strcpy_safe return (even after pre-check).You pre-check length, so truncation won’t occur, but still good to assert/check the copy result to catch unexpected failures.
As per coding guidelines
407-423: Optional: consider setting commit parents for traceability.If you want cache commits to form a linear history, populate
parentswith the prior cache ref tip; otherwise, current root-commit behavior is fine.core/include/gitmind/ports/git_repository_port.h (2)
183-191: Remove free() fallback in commit_message_dispose
All current adapters supplycommit_message_dispose, so thefree(message)path is never taken—replace it with an assertion or no-op and document that adapters must implementcommit_message_dispose.
30-36: Deriveoid_hexfromoidin allreference_tippaths. Always callgit_oid_tostr(or equivalent) immediately after settingout_tip->oid—including in fake implementations—to keep the hex and binary OIDs in sync.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
.github/labeler.yml(0 hunks).github/workflows/label-docs-only.yml(0 hunks)AGENTS.md(2 hunks)apps/cli/cache_rebuild.c(2 hunks)apps/cli/cli_runtime.h(1 hunks)apps/cli/main.c(4 hunks)apps/cli/main.md(1 hunks)benchmarks/wildebeest_stampede.c(5 hunks)core/include/gitmind/adapters/fs/posix_temp_adapter.h(1 hunks)core/include/gitmind/adapters/git/libgit2_repository_port.h(1 hunks)core/include/gitmind/cache.h(2 hunks)core/include/gitmind/context.h(2 hunks)core/include/gitmind/hooks/augment.h(6 hunks)core/include/gitmind/ports/env_port.h(0 hunks)core/include/gitmind/ports/fs_temp_port.h(2 hunks)core/include/gitmind/ports/git_repository_port.h(1 hunks)core/include/gitmind/security/string.h(1 hunks)core/src/adapters/config/env_adapter.c(1 hunks)core/src/adapters/git/libgit2_repository_port.c(1 hunks)core/src/app/cache/cache_rebuild_service.c(11 hunks)core/src/cache/builder.c(1 hunks)core/src/cache/cache_internal.h(0 hunks)core/src/cache/query.c(7 hunks)core/src/cache/tree_builder.c(0 hunks)core/src/cache/tree_size.c(0 hunks)core/src/edge/attributed.c(2 hunks)core/src/edge/edge.c(2 hunks)core/src/fs/path_utils.c(5 hunks)core/src/hooks/augment.c(4 hunks)core/src/hooks/post-commit.c(5 hunks)core/src/journal/reader.c(7 hunks)core/src/journal/writer.c(7 hunks)core/src/ports/env_port.c(1 hunks)core/tests/fakes/fs/fake_fs_temp_port.c(4 hunks)core/tests/fakes/git/fake_git_repository_port.c(1 hunks)core/tests/fakes/git/fake_git_repository_port.h(1 hunks)core/tests/unit/test_cache_branch_limits.c(3 hunks)core/tests/unit/test_cache_edge_map.c(1 hunks)core/tests/unit/test_cache_meta.c(2 hunks)core/tests/unit/test_cache_query.c(3 hunks)core/tests/unit/test_cache_tree_size.c(3 hunks)core/tests/unit/test_edge.c(3 hunks)core/tests/unit/test_hook_augment.c(1 hunks)core/tests/unit/test_journal_mixed_cbor.c(3 hunks)docs/architecture/augments-system.md(5 hunks)docs/architecture/cache/builder.md(1 hunks)docs/code-reviews/PR170/1568a39eaebc49ac1c59409f664cecfa16a9e1e7.md(0 hunks)docs/code-reviews/PR170/857b69c2942bc6221d758387cbcc214a74153973.md(0 hunks)docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md(1 hunks)docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.md(1 hunks)meson.build(3 hunks)
💤 Files with no reviewable changes (8)
- docs/code-reviews/PR170/857b69c2942bc6221d758387cbcc214a74153973.md
- .github/labeler.yml
- core/include/gitmind/ports/env_port.h
- .github/workflows/label-docs-only.yml
- core/src/cache/tree_builder.c
- core/src/cache/tree_size.c
- docs/code-reviews/PR170/1568a39eaebc49ac1c59409f664cecfa16a9e1e7.md
- core/src/cache/cache_internal.h
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{c,cc,cpp,cxx,h,hpp,hxx,m,mm}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use
NOLINTto suppress clang-tidy warnings; ALWAYS fix the underlying issue
Files:
core/tests/unit/test_cache_meta.ccore/src/journal/reader.ccore/include/gitmind/adapters/fs/posix_temp_adapter.hcore/include/gitmind/adapters/git/libgit2_repository_port.hcore/src/cache/query.capps/cli/cache_rebuild.ccore/tests/fakes/fs/fake_fs_temp_port.ccore/src/edge/edge.ccore/src/adapters/config/env_adapter.ccore/tests/unit/test_journal_mixed_cbor.ccore/include/gitmind/cache.hcore/tests/fakes/git/fake_git_repository_port.ccore/tests/unit/test_cache_edge_map.ccore/include/gitmind/context.hcore/tests/unit/test_cache_query.ccore/tests/unit/test_cache_tree_size.ccore/src/journal/writer.ccore/src/cache/builder.ccore/include/gitmind/ports/fs_temp_port.happs/cli/cli_runtime.hcore/include/gitmind/hooks/augment.hcore/src/edge/attributed.ccore/src/adapters/git/libgit2_repository_port.ccore/tests/unit/test_cache_branch_limits.ccore/src/ports/env_port.cbenchmarks/wildebeest_stampede.ccore/tests/unit/test_edge.ccore/include/gitmind/ports/git_repository_port.hcore/src/app/cache/cache_rebuild_service.ccore/src/hooks/augment.ccore/src/hooks/post-commit.ccore/tests/unit/test_hook_augment.ccore/tests/fakes/git/fake_git_repository_port.hcore/include/gitmind/security/string.hcore/src/fs/path_utils.capps/cli/main.c
{core,include}/**/*.{c,h}
📄 CodeRabbit inference engine (AGENTS.md)
{core,include}/**/*.{c,h}: Use C23 with warnings-as-errors; no VLAs or shadowing; declare explicit prototypes
Formatting must follow .clang-format (LLVM-based, 4 spaces, 80 cols, right-aligned pointers)
Naming: functions/vars lower_snake_case with gm_ prefix; macros UPPER_SNAKE; types end with _t
Prefer specific includes; order/regroup includes per .clang-format
Use OID-first equality and APIs; when both OIDs are present they must match; compare using git_oid_cmp
Construct refs via gm_build_ref, reject inputs starting with 'refs/' and enforce ref-name safety
Use safe string/memory functions (gm_snprintf, gm_strcpy_safe, gm_memcpy_span); check return codes; treat truncation as error; zero output buffers on error paths
Prefer GM_PATH_MAX; do not use GM_PATH_MAX*2; allocate if larger is required
Keep touched code SRP-compliant and injection-friendly: isolate side effects behind outbound ports and pass dependencies explicitly
Keep clang-tidy at zero warnings for touched files; do not merge new diagnostics
Zero out output buffers on failure before formatting or copying to prevent partial writes
Files:
core/tests/unit/test_cache_meta.ccore/src/journal/reader.ccore/include/gitmind/adapters/fs/posix_temp_adapter.hcore/include/gitmind/adapters/git/libgit2_repository_port.hcore/src/cache/query.ccore/tests/fakes/fs/fake_fs_temp_port.ccore/src/edge/edge.ccore/src/adapters/config/env_adapter.ccore/tests/unit/test_journal_mixed_cbor.ccore/include/gitmind/cache.hcore/tests/fakes/git/fake_git_repository_port.ccore/tests/unit/test_cache_edge_map.ccore/include/gitmind/context.hcore/tests/unit/test_cache_query.ccore/tests/unit/test_cache_tree_size.ccore/src/journal/writer.ccore/src/cache/builder.ccore/include/gitmind/ports/fs_temp_port.hcore/include/gitmind/hooks/augment.hcore/src/edge/attributed.ccore/src/adapters/git/libgit2_repository_port.ccore/tests/unit/test_cache_branch_limits.ccore/src/ports/env_port.ccore/tests/unit/test_edge.ccore/include/gitmind/ports/git_repository_port.hcore/src/app/cache/cache_rebuild_service.ccore/src/hooks/augment.ccore/src/hooks/post-commit.ccore/tests/unit/test_hook_augment.ccore/tests/fakes/git/fake_git_repository_port.hcore/include/gitmind/security/string.hcore/src/fs/path_utils.c
core/tests/unit/test_*.c
📄 CodeRabbit inference engine (AGENTS.md)
Unit tests must live under core/tests/unit as test_.c, be deterministic and isolated
Files:
core/tests/unit/test_cache_meta.ccore/tests/unit/test_journal_mixed_cbor.ccore/tests/unit/test_cache_edge_map.ccore/tests/unit/test_cache_query.ccore/tests/unit/test_cache_tree_size.ccore/tests/unit/test_cache_branch_limits.ccore/tests/unit/test_edge.ccore/tests/unit/test_hook_augment.c
{include,core/include}/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
{include,core/include}/**/*.h: Public headers must be umbrella-safe: compile standalone, include directly, have stable header guards, and provide C++ linkage guards
Header guards must follow GITMIND_* naming and remain stable
Ensure public headers compile standalone via header-compile checks
Files:
core/include/gitmind/adapters/fs/posix_temp_adapter.hcore/include/gitmind/adapters/git/libgit2_repository_port.hcore/include/gitmind/cache.hcore/include/gitmind/context.hcore/include/gitmind/ports/fs_temp_port.hcore/include/gitmind/hooks/augment.hcore/include/gitmind/ports/git_repository_port.hcore/include/gitmind/security/string.h
core/tests/fakes/**
📄 CodeRabbit inference engine (AGENTS.md)
Provide deterministic fakes/mocks/stubs for each port under core/tests/fakes
Files:
core/tests/fakes/fs/fake_fs_temp_port.ccore/tests/fakes/git/fake_git_repository_port.ccore/tests/fakes/git/fake_git_repository_port.h
core/src/adapters/**/*_adapter.{c,h}
📄 CodeRabbit inference engine (AGENTS.md)
Runtime adapters must be placed under core/src/adapters// with filenames ending in _adapter and paired private headers
Files:
core/src/adapters/config/env_adapter.c
core/src/adapters/**
📄 CodeRabbit inference engine (AGENTS.md)
Each adapter must publish a factory returning the port vtable plus state, and provide teardown hooks
Files:
core/src/adapters/config/env_adapter.ccore/src/adapters/git/libgit2_repository_port.c
core/include/gitmind/ports/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
Ports headers must declare interfaces only (no non-trivial inline behavior) and use gm_result_t signatures
Files:
core/include/gitmind/ports/fs_temp_port.hcore/include/gitmind/ports/git_repository_port.h
core/include/gitmind/hooks/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
Hooks APIs must surface gm_result_t results and avoid raw integer returns
Files:
core/include/gitmind/hooks/augment.h
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Run
make docs-verifywhen changing documentation
Files:
docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.mddocs/architecture/augments-system.mddocs/architecture/cache/builder.mddocs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Docs must include front matter first, a single H1 whose title matches the filename title, a '## Table of Contents' section, and API docs must include api_version; add License/SPDX comments after front matter
Files:
docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.mddocs/architecture/augments-system.mddocs/architecture/cache/builder.mddocs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md
🧠 Learnings (22)
📓 Common learnings
Learnt from: flyingrobots
PR: neuroglyph/git-mind#169
File: docs/templates/code-review-feedback.md:26-33
Timestamp: 2025-09-16T17:10:27.477Z
Learning: The user (flyingrobots) uses Obsidian to view/edit Markdown files and prefers to keep Obsidian-specific syntax like collapsible callout markers (e.g., `[!NOTE]-` for collapsible callouts) even if they're not GitHub-compatible.
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/journal/{reader,writer}.c : Eliminate CBOR magic numbers; use centralized keys from gitmind/cbor/keys.h
Applied to files:
core/src/journal/reader.ccore/src/journal/writer.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to include/gitmind.h : Maintain umbrella API at include/gitmind.h
Applied to files:
core/src/journal/reader.ccore/include/gitmind/adapters/fs/posix_temp_adapter.hcore/src/cache/query.capps/cli/cache_rebuild.ccore/src/adapters/config/env_adapter.ccore/tests/unit/test_journal_mixed_cbor.ccore/include/gitmind/context.hcore/src/journal/writer.capps/cli/cli_runtime.hcore/include/gitmind/hooks/augment.hbenchmarks/wildebeest_stampede.ccore/src/app/cache/cache_rebuild_service.ccore/src/hooks/augment.ccore/src/fs/path_utils.capps/cli/main.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/cbor/keys.h : Define CBOR field keys as named constants/enums in a single header and include it where parsing/encoding occurs
Applied to files:
core/src/journal/reader.ccore/src/hooks/augment.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to core/include/gitmind/ports/**/*.h : Ports headers must declare interfaces only (no non-trivial inline behavior) and use gm_result_t signatures
Applied to files:
core/src/journal/reader.ccore/include/gitmind/adapters/fs/posix_temp_adapter.hcore/include/gitmind/adapters/git/libgit2_repository_port.hcore/src/adapters/config/env_adapter.ccore/include/gitmind/context.hcore/src/journal/writer.ccore/include/gitmind/hooks/augment.hcore/src/ports/env_port.ccore/include/gitmind/ports/git_repository_port.hcore/src/app/cache/cache_rebuild_service.ccore/tests/fakes/git/fake_git_repository_port.h
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Use OID-first equality and APIs; when both OIDs are present they must match; compare using git_oid_cmp
Applied to files:
core/src/journal/reader.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to include/gitmind/**/*.h : Public, namespaced headers live under include/gitmind/
Applied to files:
core/include/gitmind/adapters/fs/posix_temp_adapter.happs/cli/main.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to {include,core/include}/**/*.h : Header guards use GITMIND_* naming
Applied to files:
core/src/cache/query.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/edge.h : Avoid struct bloat: do not store both legacy SHA arrays and OIDs long-term in gm_edge_t; deprecate and phase out legacy SHA fields
Applied to files:
core/src/edge/edge.ccore/include/gitmind/cache.hcore/src/edge/attributed.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/edge/edge.c : gm_edge_equal() must compare OIDs first and only fall back to SHA when OIDs are unset/zero; never treat mismatched OIDs as equal due to matching SHAs
Applied to files:
core/src/edge/edge.ccore/src/edge/attributed.ccore/tests/unit/test_edge.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to include/gitmind/**/*.h : Avoid public ABI breaks in structs; append new fields at the tail and mark deprecated fields
Applied to files:
core/include/gitmind/cache.h
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/cache/{builder,query}.c : Version and migrate cache formats when changing the hash function (e.g., raw OID vs SHA); auto-rebuild or reject mismatched versions
Applied to files:
core/include/gitmind/cache.hcore/src/cache/builder.cdocs/architecture/cache/builder.mdcore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/cache/{builder,query}.c : Keep naming consistent with OID terminology (e.g., OID_HASH_MULTIPLIER, oid_hash) after migration from SHA
Applied to files:
core/include/gitmind/cache.h
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/hooks/augment.h : Prefix public hook APIs with gm_hook_ and provide deprecation warnings for legacy macro/function names with a clear removal timeline
Applied to files:
core/include/gitmind/hooks/augment.hcore/src/hooks/augment.ccore/tests/unit/test_hook_augment.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/hooks/augment.h : Maintain stable header guards or provide compatibility for guard renames (GM_AUGMENT_H → GITMIND_AUGMENT_H) to avoid breaking includes
Applied to files:
core/include/gitmind/hooks/augment.hcore/src/hooks/augment.ccore/tests/unit/test_hook_augment.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to core/include/gitmind/hooks/**/*.h : Hooks APIs must surface gm_result_t results and avoid raw integer returns
Applied to files:
core/include/gitmind/hooks/augment.hcore/src/hooks/augment.ccore/src/fs/path_utils.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to .coderabbit.yml : Maintain .coderabbit.yml to prefer summary-first reviews and limit per-line comments on docs
Applied to files:
docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.mddocs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Keep touched code SRP-compliant and injection-friendly: isolate side effects behind outbound ports and pass dependencies explicitly
Applied to files:
core/src/ports/env_port.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Use safe string/memory functions (gm_snprintf, gm_strcpy_safe, gm_memcpy_span); check return codes; treat truncation as error; zero output buffers on error paths
Applied to files:
core/include/gitmind/security/string.hcore/src/fs/path_utils.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/**/*.{c,h} : Use bounded string functions (e.g., gm_snprintf) and check return values for overflow; ensure explicit NUL termination
Applied to files:
core/include/gitmind/security/string.hcore/src/fs/path_utils.c
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Naming: functions/vars lower_snake_case with gm_ prefix; macros UPPER_SNAKE; types end with _t
Applied to files:
core/include/gitmind/security/string.h
📚 Learning: 2025-09-20T17:35:43.632Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.632Z
Learning: Applies to {core,include}/**/*.{c,h} : Prefer GM_PATH_MAX; do not use GM_PATH_MAX*2; allocate if larger is required
Applied to files:
core/src/fs/path_utils.c
🧬 Code graph analysis (36)
core/tests/unit/test_cache_meta.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/cache/query.c (1)
gm_cache_load_meta(58-138)
core/src/journal/reader.c (2)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/ports/git_repository_port.h (1)
gm_git_repository_port_commit_message_dispose(183-191)
core/include/gitmind/adapters/fs/posix_temp_adapter.h (2)
core/src/adapters/fs/posix_temp_adapter.c (5)
GM_PATH_MAX(28-34)struct stat(74-74)make_temp_dir_bridge(577-583)struct stat(420-420)ensure_dir_exists(68-112)core/include/gitmind/result.h (1)
struct gm_error(12-12)
core/include/gitmind/adapters/git/libgit2_repository_port.h (1)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)
core/src/cache/query.c (6)
core/src/error/error.c (1)
gm_error_free(161-178)core/src/util/ref.c (1)
gm_build_ref(47-84)core/include/gitmind/security/string.h (1)
gm_snprintf(74-80)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/src/cache/bitmap.c (1)
gm_bitmap_deserialize(84-109)
apps/cli/cache_rebuild.c (2)
core/src/error/error.c (1)
gm_error_free(161-178)apps/cli/output.c (1)
gm_output_error(65-70)
core/tests/fakes/fs/fake_fs_temp_port.c (3)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/tests/fakes/fs/fake_fs_temp_port.h (1)
GM_PATH_MAX(16-24)core/tests/unit/test_fs_temp_port.c (1)
main(24-167)
core/src/edge/edge.c (5)
core/src/edge/attributed.c (1)
resolve_blob_identity(42-70)core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)core/include/gitmind/edge.h (1)
GM_SHA1_SIZE(32-45)core/include/gitmind/edge_attributed.h (1)
GM_SHA1_SIZE(30-48)core/tests/unit/test_edge_equal_semantics.c (1)
test_fallback_to_sha(29-42)
apps/cli/main.md (1)
core/include/gitmind/ports/env_port.h (1)
struct gm_env_port {(18-21)
core/src/adapters/config/env_adapter.c (1)
core/include/gitmind/error.h (2)
GM_ERROR_SMALL_SIZE(25-39)struct gm_error(36-36)
core/tests/unit/test_journal_mixed_cbor.c (1)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)
core/include/gitmind/cache.h (1)
core/include/gitmind/cache/cache.h (2)
GM_CACHE_OID_STRING_SIZE(32-40)struct {(43-47)
core/tests/fakes/git/fake_git_repository_port.c (2)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/tests/unit/test_cache_edge_map.c (1)
core/tests/unit/test_error.c (1)
test_result_success(64-72)
core/include/gitmind/context.h (2)
core/include/gitmind/ports/env_port.h (1)
struct gm_env_port {(18-21)core/include/gitmind/io/io.h (1)
struct gm_io_ops {(73-78)
core/tests/unit/test_cache_query.c (1)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)
core/tests/unit/test_cache_tree_size.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/cache/tree_size.c (2)
calculate_tree_size_recursive(19-86)gm_cache_calculate_size(89-93)
core/src/journal/writer.c (3)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/include/gitmind/ports/env_port.h (1)
struct gm_env_port {(18-21)
core/include/gitmind/ports/fs_temp_port.h (1)
core/src/adapters/fs/posix_temp_adapter.c (1)
join_under_base_bridge(590-598)
core/include/gitmind/hooks/augment.h (1)
core/src/hooks/augment.c (3)
gm_hook_get_blob_sha(96-157)gm_hook_process_changed_file(252-292)gm_hook_is_merge_commit(295-337)
docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.md (1)
tools/review/seed_feedback_from_github.py (1)
main(50-134)
docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md (1)
tools/review/seed_feedback_from_github.py (1)
main(50-134)
core/src/edge/attributed.c (5)
core/src/edge/edge.c (1)
resolve_blob_identity(55-83)core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)core/include/gitmind/edge.h (1)
GM_SHA1_SIZE(32-45)core/include/gitmind/edge_attributed.h (1)
GM_SHA1_SIZE(30-48)core/tests/unit/test_edge_attributed_cbor.c (1)
test_legacy_backfill(56-111)
core/src/adapters/git/libgit2_repository_port.c (3)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/include/gitmind/security/string.h (1)
gm_snprintf(74-80)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/tests/unit/test_cache_branch_limits.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/adapters/fs/posix_temp_adapter.c (2)
dispose_port(615-624)gm_posix_fs_temp_port_create(626-651)
core/src/ports/env_port.c (2)
core/include/gitmind/ports/env_port.h (4)
struct gm_env_port {(18-21)struct gm_env_port_vtbl(16-16)struct gm_env_port_vtbl {(26-28)struct gm_env_port_vtbl(20-20)core/include/gitmind/error.h (2)
GM_ERROR_SMALL_SIZE(25-39)struct gm_error(36-36)
benchmarks/wildebeest_stampede.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/cache/query.c (1)
gm_cache_query_fanout(373-377)
core/tests/unit/test_edge.c (1)
core/tests/fakes/git/fake_git_repository_port.c (5)
gm_fake_git_repository_port_init(194-228)gm_fake_git_repository_port_set_head_branch(246-260)gm_fake_git_repository_port_clear_blob_mappings(307-314)gm_fake_git_repository_port_add_blob_mapping(316-355)gm_fake_git_repository_port_dispose(230-236)
core/include/gitmind/ports/git_repository_port.h (1)
core/include/gitmind/ports/fs_temp_port.h (6)
gm_result_void_t(76-84)gm_result_void_t(86-97)gm_result_void_t(99-108)gm_result_void_t(110-124)gm_result_void_t(126-138)gm_result_void_t(140-147)
core/src/app/cache/cache_rebuild_service.c (2)
core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)
core/src/hooks/augment.c (3)
core/src/error/error.c (1)
gm_error_free(161-178)core/src/util/ref.c (1)
gm_build_ref(47-84)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/src/hooks/post-commit.c (4)
core/include/gitmind/hooks/augment.h (2)
is_merge_commit(111-114)process_changed_file(105-109)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/error/error.c (1)
gm_error_free(161-178)
core/tests/unit/test_hook_augment.c (2)
core/tests/fakes/git/fake_git_repository_port.c (5)
gm_fake_git_repository_port_init(194-228)gm_fake_git_repository_port_set_head_branch(246-260)gm_fake_git_repository_port_add_ref_commit(271-305)gm_fake_git_repository_port_add_commit_blob_mapping(357-397)gm_fake_git_repository_port_dispose(230-236)core/src/hooks/augment.c (2)
gm_hook_get_blob_sha(96-157)gm_hook_is_merge_commit(295-337)
core/tests/fakes/git/fake_git_repository_port.h (1)
core/include/gitmind/ports/git_repository_port.h (14)
gm_result_void_t(99-109)gm_result_void_t(111-119)gm_result_void_t(121-132)gm_result_void_t(134-143)gm_result_void_t(145-155)gm_result_void_t(157-169)gm_result_void_t(171-181)gm_result_void_t(193-205)gm_result_void_t(207-217)gm_result_void_t(219-228)gm_result_void_t(230-239)gm_result_void_t(241-251)gm_result_void_t(253-264)gm_result_void_t(266-277)
core/src/fs/path_utils.c (1)
core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)
apps/cli/main.c (3)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(977-1005)core/src/error/error.c (1)
gm_error_free(161-178)apps/cli/output.c (1)
gm_output_create(11-22)
🔇 Additional comments (63)
core/src/adapters/config/env_adapter.c (1)
4-6: Direct error header include is correct
gm_err_bool/GM_ERRORare used in this unit, so taking a direct dependency ongitmind/error.hkeeps the adapter self-contained and compliant with our include guidelines.core/include/gitmind/adapters/fs/posix_temp_adapter.h (1)
8-8: LGTM: include result header to satisfy header-standalone rulePulling in
gitmind/result.hmakes the public header self-contained forgm_result_void_t, aligning with the standalone-compile requirement for headers underinclude/gitmind/.core/include/gitmind/security/string.h (1)
74-80: Inlining gm_snprintf is aligned with header usageMaking the wrapper
static inlinekeeps linkage hygiene for this public header and mirrors the pattern used bygm_vsnprintf; no issues spotted.core/tests/unit/test_cache_meta.c (3)
13-14: LGTM: Clean port-based architecture migration.The addition of the new includes for
gm_result_tand the libgit2 repository port adapter is appropriate for the architectural shift from direct Git operations to port-based abstractions.
59-62: LGTM: Proper repository port initialization.The migration from direct Git repository usage to the port-based approach follows the established pattern. The error handling with assertion is appropriate for test code.
68-70: LGTM: Proper resource cleanup.The conditional disposal of the repository port with null-check follows best practices and ensures proper cleanup of resources.
core/src/journal/reader.c (7)
5-6: LGTM: Clean port-based migration.The migration to use the git repository port interface aligns with the architectural shift documented in the PR objectives.
65-65: LGTM: Updated context structure.The change from legacy git_ops to the repository port abstraction is consistent with the port-based architecture migration.
201-201: Function signature updated for port-based architecture.The migration from raw message processing to using the repository port for commit message decoding is appropriate.
220-239: LGTM: Port-based branch resolution with proper error handling.The migration to use
gm_git_repository_port_head_branchwith proper result unwrapping and error cleanup follows established patterns in the codebase.
284-303: LGTM: Port-based commit walking with proper resource management.The migration to the repository port for commit message reading and the proper disposal of messages follows the established port interface patterns. The error handling and resource cleanup are correctly implemented.
305-317: LGTM: Port-based commit walking implementation.The migration to
gm_git_repository_port_walk_commitswith proper error code extraction and cleanup is well implemented.
331-333: LGTM: Repository port validation and usage.The validation of the repository port vtbl and its usage throughout the generic journal read function is consistent with the port-based architecture.
Also applies to: 344-344, 351-351
core/src/cache/query.c (7)
46-56: LGTM: Clean result unwrapping utility.The
unwrap_result_codehelper function provides a clean abstraction for convertinggm_result_void_tto integer error codes with proper error cleanup. This pattern is used consistently throughout the file.
59-64: LGTM: Port-based cache metadata loading.The migration from direct Git operations to the repository port interface is well implemented. The validation checks and error handling follow established patterns, and the use of port-based reference operations is consistent with the architectural migration.
Also applies to: 74-97
122-133: LGTM: Consistent port usage for journal reference lookup.The port-based journal tip resolution with proper error handling and resource management is implemented correctly.
142-188: LGTM: Port-based staleness checking with proper OID comparison.The cache staleness check has been properly migrated to use the repository port. The OID comparison logic using
git_oid_cmpfollows the coding guidelines for OID operations. As per coding guidelines: "Use OID-first equality and APIs; when both OIDs are present they must match; compare using git_oid_cmp"
191-231: LGTM: Port-based bitmap loading from cache.The migration to use
gm_git_repository_port_commit_read_blobfor loading bitmap data from the cache tree is properly implemented with correct error handling and resource management.
276-311: LGTM: Consistent port usage across cache query functions.All cache query functions have been properly migrated to use the repository port interface. The implementation maintains consistency with the port-based architecture while preserving the original functionality.
Also applies to: 354-384
398-428: LGTM: Port-based cache statistics with proper tree size calculation.The cache statistics function correctly uses the repository port for tree size calculation, with appropriate fallback logic when the port operation fails.
docs/architecture/cache/builder.md (1)
33-33: LGTM: Documentation reflects port-based architecture migration.The update correctly documents that the Legacy Cache Builder Adapter now validates
ctx->git_repo_portinstead ofctx->git_repo, which aligns with the broader architectural shift from direct Git repository access to port-based abstractions described in the PR objectives.apps/cli/cli_runtime.h (1)
9-15: No changes needed: repository pointer is properly freed in init_context and cleanup_contextcore/include/gitmind/context.h (1)
14-45: Context wiring aligns with new git repo portGood to see the context struct now owning the repository port and its disposer; this mirrors the existing temp-fs port pattern and keeps injection consistent.
core/tests/unit/test_cache_query.c (1)
75-121: Test now exercises port lifecycle correctlyAppreciate the explicit creation and teardown of the libgit2 repository port inside the test fixture; it keeps the context wiring representative of production and avoids leaking adapter state.
core/src/hooks/post-commit.c (1)
193-208: Robust port initialization and failure handlingZeroing the context before wiring the repo port and freeing any creation error via
gm_error_freekeeps the hook on the happy path while remaining commit-safe on failures.meson.build (4)
133-134: LGTM! Repository port integration aligns with architectural refactoring.The addition of the LibGit2 repository port adapter and augment hooks implementation follows the expected port-based architecture pattern.
253-257: LGTM! Test executable properly wired for port-based testing.The updated test_edge executable correctly includes the fake_git_repository_port.c to enable port-based testing, which is consistent with the architectural shift.
259-264: LGTM! New test executable properly configured.The test_hook_augment executable is correctly configured with both the test file and the fake repository port dependency for isolated testing.
376-376: LGTM! Test registration completes the build integration.The hook_augment test is properly registered in the test suite.
core/tests/unit/test_cache_tree_size.c (5)
11-11: LGTM! Include properly added for repository port integration.The include for the LibGit2 repository port header is correctly added to support the port-based architecture.
88-94: LGTM! Port initialization follows standard pattern.The repository port creation follows the established pattern with proper initialization of state and dispose callback.
96-98: LGTM! Port-based API call correctly updated.The tree size calculation has been properly migrated from the direct
gm_cache_calculate_sizeAPI to the port-basedgm_git_repository_port_commit_tree_sizeAPI.
108-110: LGTM! Proper resource cleanup.The port disposal logic properly handles the case where a dispose callback is provided, following the established cleanup patterns.
73-87: Commit creation is required for the port API’s tree-size calculation
The port only exposesgm_git_repository_port_commit_tree_size, so you must wrap your tree in a commit to get its size—there is no direct tree-OID size method.apps/cli/main.c (6)
11-11: LGTM! Include added for repository port integration.The LibGit2 repository port header is correctly included to support the port-based architecture migration.
185-198: LGTM! Repository port initialization properly integrated.The repository port creation follows the established pattern with proper error handling and resource management. The error path correctly cleans up the repository on failure.
199-199: LGTM! Repository ownership transferred to CLI context.The repository handle is properly assigned to the CLI context for later cleanup, maintaining clear ownership semantics.
210-216: LGTM! Error handling properly cascades cleanup.The error path for filesystem port creation correctly disposes of the repository port before cleaning up the repository, preventing resource leaks.
221-233: LGTM! Comprehensive error cleanup.The error path for output creation properly disposes of both ports and the repository in the correct order, ensuring no resource leaks.
249-256: LGTM! Cleanup follows proper disposal order.The cleanup function correctly disposes of the repository port before freeing the repository, maintaining the proper resource disposal order.
core/src/journal/writer.c (9)
7-7: LGTM! Include updated for port-based architecture.The include has been correctly updated to use the repository port interface instead of direct libgit2 usage.
34-37: LGTM! Context structure properly updated for port-based design.The journal context now correctly references the repository port instead of a raw git_repository pointer, and uses
gm_oid_tinstead ofgit_oidfor consistency with the port API.
48-51: LGTM! Port validation follows established pattern.The initialization properly validates that the repository port is available in the context before proceeding.
69-80: LGTM! Branch resolution migrated to port-based API.The function correctly uses the repository port's head_branch method with proper error handling and cleanup.
103-120: LGTM! Parent lookup callback properly implemented.The callback function correctly implements the parent collection logic using port-based types and follows proper error handling patterns.
123-164: LGTM! Commit creation properly migrated to port-based API.The commit creation function has been correctly updated to use:
- Port-based commit walking for parent resolution
- Port-based commit creation with proper spec structure
- Appropriate error handling with port result types
The logic maintains the same functionality while using the new port interface.
189-189: LGTM! Branch resolution updated for port context.The function correctly uses the repository port from the context instead of a raw repository pointer.
194-216: LGTM! Reference update properly migrated to port API.The journal batch flushing correctly uses the port-based reference update method with proper spec structure and error handling.
347-351: LGTM! Public wrapper updated for port-based context.The wrapper function correctly references the repository port from the context, maintaining the same public API while using the new internal port-based implementation.
benchmarks/wildebeest_stampede.c (4)
24-24: LGTM! Include added for repository port integration.The LibGit2 repository port header is correctly included to support the port-based architecture.
89-97: LGTM! Repository port initialization follows established pattern.The context initialization and repository port creation follow the standard pattern with proper error handling and early exit on failure.
119-119: LGTM! Cache query calls updated for port-based context.The
gm_cache_query_fanoutcalls have been correctly updated to use the context instead of the raw repository pointer, aligning with the port-based architecture.Also applies to: 138-138
154-156: LGTM! Resource cleanup properly extended.The cleanup logic correctly disposes of the repository port after the filesystem port, maintaining proper disposal order.
core/src/app/cache/cache_rebuild_service.c (7)
35-38: Good guard for low-resolution clocks.Ceiling division ensures CLOCKS_PER_MS >= 1; matches the “guard clock rate” objective.
246-253: Port-based tree build looks correct.Validates vtbl and routes via the port; error propagation via unwrap_result is consistent.
372-406: Solid tip resolution and safe clearing when missing target.Good use of gm_build_ref, structured port call, and zeroing binary OID on no target.
435-444: Reference update via port looks good.Spec construction and force=true match the rebuild semantics.
453-461: Good upfront validation of required ports.Prevents invalid-state paths early.
498-509: End-to-end flow migrated to repository port correctly.Meta population and commit/update now fully port-driven.
63-88: No disposal needed for canonical path
The pointer returned by gm_fs_temp_port_canonicalize_ex is a borrowed reference to the port’s internal scratch buffer, not heap-allocated—no free or dispose call required.core/include/gitmind/ports/git_repository_port.h (2)
59-98: Interface surface is coherent and C++-friendly.Header guards, C linkage, and vtbl layout align with existing fs_temp_port patterns.
99-277: Wrapper pattern and error mapping look solid.Consistent
gm_err_void(GM_ERR_INVALID_STATE, ...)checks and NODISCARD usage match ports style.core/src/fs/path_utils.c (1)
9-9: Good include.Including result.h is correct for gm_result_void_t usage.
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
core/include/gitmind/security/string.h (1)
74-80: Restore printf-format diagnostics ongm_snprintf.Switching to a plain
static inlinedropped theformat(printf, …)attribute, so clang/gcc no longer emit compile-time format-string checks for this helper. That’s a regression in our safety net and will let misformatted calls slip past clang-tidy. Please reinstate the attribute in the inline declaration. As per coding guidelines-/* Safe snprintf wrapper that suppresses security warnings */ -static inline int gm_snprintf(char *str, size_t size, const char *format, ...) { +/* Safe snprintf wrapper that suppresses security warnings */ +#if defined(__clang__) || defined(__GNUC__) +static inline __attribute__((always_inline, format(printf, 3, 4))) +#else +static inline +#endif +int gm_snprintf(char *str, size_t size, const char *format, ...) {core/src/hooks/post-commit.c (2)
49-72: Critical: realloc pointer not propagated to caller (use-after-free/heap corruption risk).read_file_entry grows the array via grow_files_array(&files, ...) where files is passed by value. If realloc moves the block, the caller’s pointer remains stale, leading to invalid frees or writes. Also guard newline removal when len == 0.
Apply this diff to make files a triple pointer and fix newline handling:
-static int read_file_entry(char **files, size_t *count, size_t *capacity, - char *line, ssize_t len) { +static int read_file_entry(char ***files, size_t *count, size_t *capacity, + char *line, ssize_t len) { /* Remove newline */ - if (line[len - 1] == '\n') { - line[len - 1] = '\0'; - } + if (len > 0 && line[len - 1] == '\n') { + line[len - 1] = '\0'; + } /* Grow array if needed */ if (*count >= *capacity) { - int error = grow_files_array(&files, capacity); + int error = grow_files_array(files, capacity); if (error != GM_OK) { return error; } } /* Copy file path */ - files[*count] = strdup(line); - if (!files[*count]) { + (*files)[*count] = strdup(line); + if (!(*files)[*count]) { return GM_NO_MEMORY; } (*count)++; return GM_OK; }
98-105: Follow-up: update call site for read_file_entry triple-pointer.Propagate the possibly reallocated pointer back to the caller.
Apply this diff:
- error = read_file_entry(files, &count, &capacity, line, read); + error = read_file_entry(&files, &count, &capacity, line, read);core/include/gitmind/hooks/augment.h (1)
80-86: Fix param doc: rename ‘repo’ to ‘repo_port’.The comment still says “@PARAM repo Git repository” but the signature takes repo_port.
Apply this diff:
- * @param repo Git repository + * @param repo_port Repository port adaptercore/src/edge/edge.c (1)
350-361: Incorrect OID “fallback” check and potential read overrun when GM_OID_RAWSZ > GM_SHA1_SIZE
- Checking for NULL on edge->{src,tgt}_oid.id will never trigger; the array is never NULL. Use git_oid_is_zero to detect missing OIDs.
- If you fallback to SHA bytes while writing GM_OID_RAWSZ bytes, you can overread edge->{src,tgt}_sha when GM_OID_RAWSZ > GM_SHA1_SIZE (e.g., SHA‑256 builds).
Fix by materializing GM_OID_RAWSZ‑sized buffers, copying min(GM_OID_RAWSZ, GM_SHA1_SIZE), and zero‑padding.
Apply this diff:
- const uint8_t *src_raw = (const uint8_t *)edge->src_oid.id; - const uint8_t *tgt_raw = (const uint8_t *)edge->tgt_oid.id; - if (src_raw == NULL) src_raw = edge->src_sha; /* fallback */ - if (tgt_raw == NULL) tgt_raw = edge->tgt_sha; /* fallback */ - result = write_cbor_bytes(GM_CBOR_KEY_SRC_OID, buffer, available, &offset, src_raw, GM_OID_RAWSZ); + uint8_t src_oid_bytes[GM_OID_RAWSZ] = {0}; + uint8_t tgt_oid_bytes[GM_OID_RAWSZ] = {0}; + if (!git_oid_is_zero(&edge->src_oid)) { + (void)gm_memcpy_span(src_oid_bytes, GM_OID_RAWSZ, edge->src_oid.id, GM_OID_RAWSZ); + } else { + size_t n = (GM_OID_RAWSZ < GM_SHA1_SIZE) ? GM_OID_RAWSZ : GM_SHA1_SIZE; + (void)gm_memcpy_span(src_oid_bytes, GM_OID_RAWSZ, edge->src_sha, n); + if (n < GM_OID_RAWSZ) { + memset(src_oid_bytes + n, 0, GM_OID_RAWSZ - n); + } + } + if (!git_oid_is_zero(&edge->tgt_oid)) { + (void)gm_memcpy_span(tgt_oid_bytes, GM_OID_RAWSZ, edge->tgt_oid.id, GM_OID_RAWSZ); + } else { + size_t n = (GM_OID_RAWSZ < GM_SHA1_SIZE) ? GM_OID_RAWSZ : GM_SHA1_SIZE; + (void)gm_memcpy_span(tgt_oid_bytes, GM_OID_RAWSZ, edge->tgt_sha, n); + if (n < GM_OID_RAWSZ) { + memset(tgt_oid_bytes + n, 0, GM_OID_RAWSZ - n); + } + } + result = write_cbor_bytes(GM_CBOR_KEY_SRC_OID, buffer, available, &offset, src_oid_bytes, GM_OID_RAWSZ); if (!result.ok) { return result; } - result = write_cbor_bytes(GM_CBOR_KEY_TGT_OID, buffer, available, &offset, tgt_raw, GM_OID_RAWSZ); + result = write_cbor_bytes(GM_CBOR_KEY_TGT_OID, buffer, available, &offset, tgt_oid_bytes, GM_OID_RAWSZ); if (!result.ok) { return result; }apps/cli/main.c (1)
205-216: Same leak on fs port failure: ensure libgit2 shutdown before returning.Without a shutdown here, git_libgit2_init() remains unbalanced when init_context fails.
Apply this diff:
if (!fs_port_result.ok) { int code = GM_ERR_UNKNOWN; if (fs_port_result.u.err != NULL) { code = fs_port_result.u.err->code; gm_error_free(fs_port_result.u.err); } if (ctx->git_repo_port_dispose != NULL) { ctx->git_repo_port_dispose(&ctx->git_repo_port); ctx->git_repo_port_dispose = NULL; } git_repository_free(repo); + git_libgit2_shutdown(); return code; }benchmarks/wildebeest_stampede.c (1)
64-140: Fixgm_cache_query_fanoutparameter types
gm_cache_query_fanoutnow takes aconst gm_oid_t *, but we still pass a rawuint8_t[GM_SHA1_SIZE]. With-Werror, this is a hard compile failure, and even without that flag we’d be reading random memory as an OID. Please promote the temporary SHA to agm_oid_tand pass its address to both call sites.- uint8_t mufasa_sha[GM_SHA1_SIZE]; - generate_wildebeest_sha(mufasa_sha); + gm_oid_t mufasa_oid = {0}; + generate_wildebeest_sha(mufasa_oid.id); … - memcpy(beast->tgt_sha, mufasa_sha, GM_SHA1_SIZE); + memcpy(beast->tgt_sha, mufasa_oid.id, GM_SHA1_SIZE); … - memcpy(beast->src_sha, mufasa_sha, GM_SHA1_SIZE); + memcpy(beast->src_sha, mufasa_oid.id, GM_SHA1_SIZE); … - gm_cache_query_fanout(&ctx, branch, mufasa_sha, &scan_result); + gm_cache_query_fanout(&ctx, branch, &mufasa_oid, &scan_result); … - gm_cache_query_fanout(&ctx, branch, mufasa_sha, &cache_result); + gm_cache_query_fanout(&ctx, branch, &mufasa_oid, &cache_result);core/src/hooks/augment.c (2)
239-242: Fix incorrect gm_strcpy_safe return check; zero outputs on error.Comparing to -1 will miss valid non-zero error codes. Also zero out buffers on failure per guidelines.
As per coding guidelines
- if (gm_strcpy_safe(edge.src_path, GM_PATH_MAX, file_path) == -1 || - gm_strcpy_safe(edge.tgt_path, GM_PATH_MAX, file_path) == -1) { - return GM_ERR_BUFFER_TOO_SMALL; - } + int rc_src = gm_strcpy_safe(edge.src_path, GM_PATH_MAX, file_path); + int rc_tgt = gm_strcpy_safe(edge.tgt_path, GM_PATH_MAX, file_path); + if (rc_src != GM_OK || rc_tgt != GM_OK) { + gm_memset_safe(edge.src_path, sizeof edge.src_path, 0, sizeof edge.src_path); + gm_memset_safe(edge.tgt_path, sizeof edge.tgt_path, 0, sizeof edge.tgt_path); + return (rc_src != GM_OK) ? rc_src : rc_tgt; + }
245-246: Check gm_ulid_generate result.Handle failures instead of proceeding with an uninitialized ULID.
- gm_ulid_generate(edge.ulid); + int ulid_rc = gm_ulid_generate(edge.ulid); + if (ulid_rc != GM_OK) { + return ulid_rc; + }
🧹 Nitpick comments (43)
core/src/hooks/post-commit.c (2)
129-141: Log non-merge skip reason when is_merge_commit fails.Currently silent on errors unless merge; add a terse verbose message.
Apply this diff:
- if (error != GM_OK || is_merge) { - if (verbose && is_merge) { - printf("Skipping merge commit\n"); - } - return 0; - } + if (error != GM_OK || is_merge) { + if (verbose) { + if (is_merge) { + printf("Skipping merge commit\n"); + } else { + fprintf(stderr, "Skipping commit: is_merge_commit() failed (%d)\n", error); + } + } + return 0; + }
90-96: Prefer port API over shelling out for changed files.Using popen("git diff …") couples to shell/CLI and reduces testability. Consider adding/enhancing a repo_port method to enumerate changed paths for the last commit and use that instead. Improves SRP and injection, and aligns with the new port abstraction.
As per coding guidelines
core/include/gitmind/ports/git_repository_port.h (3)
30-36: Avoid dual OID representations to prevent invariant drift.
oidandoid_hexcan diverge. Prefer a single source of truth (gm_oid_t) and derive hex at use sites, or guarantee/adapt filling by one code path only.
183-191: Keep port headers minimal; consider removing fallback free from inline.The fallback
free(message)adds behavior in a public header. Prefer delegating disposal to the adapter (or keep the fallback but document allocator expectations).Based on learnings
78-81: Clarify walk callback contract.Briefly document return semantics (e.g., 0=continue, non‑zero=stop; negative=error) to keep adapters consistent.
core/src/journal/reader.c (1)
262-268: Consider failing fast on invalid CBOR instead of silently stopping.Breaking and returning OK can hide corrupt journal entries and drop edges. Prefer returning
GM_ERR_INVALID_FORMATto surface data issues.Apply this diff if stricter behavior is desired:
- if (edge_status == GM_ERR_INVALID_FORMAT) { + if (edge_status == GM_ERR_INVALID_FORMAT) { cbor_debug_log_two_values( "[CBOR DEBUG] Invalid CBOR at commit decode offset=%zu remaining=%zu\n", offset, remaining); - break; + return GM_ERR_INVALID_FORMAT; }Please confirm intended tolerance policy.
docs/architecture/augments-system.md (2)
1-9: Add SPDX license comment after front matter.Docs require SPDX after front matter.
Apply this diff:
--- title: AUGMENTS System Architecture description: System for automatically linking content evolution with AUGMENTS edges. audience: [developers] domain: [architecture] tags: [augments, hooks] status: draft last_updated: 2025-09-27 --- +<!-- SPDX-License-Identifier: LicenseRef-MIND-UCAL-1.0 -->As per coding guidelines
13-19: Make the ToC heading an H2: “## Table of Contents”.Docs gate expects an H2 “Table of Contents”.
Apply this diff:
-Table of Contents +## Table of ContentsAs per coding guidelines
apps/cli/cache_rebuild.c (1)
41-46: Consider surfacing underlying error detail (optional)You currently log a generic GM_ERROR_GET_BRANCH. Optionally append the port error message for diagnosability.
core/src/journal/writer.c (4)
48-52: Validate ctx pointer before dereference (minor)Add a null check for ctx before accessing ctx->git_repo_port.vtbl.
Apply this diff:
- if (ctx->git_repo_port.vtbl == NULL) { + if (ctx == NULL || ctx->git_repo_port.vtbl == NULL) { return GM_ERR_INVALID_STATE; }
69-80: Make port parameter const-correct in get_current_branchFunction does not mutate the port; accept a const pointer to improve API clarity.
Apply this diff:
-static int get_current_branch(gm_git_repository_port_t *port, char *branch_name, +static int get_current_branch(const gm_git_repository_port_t *port, char *branch_name, size_t len) {And update the call (Line 189):
- return get_current_branch(&ctx->git_repo_port, branch, branch_len); + return get_current_branch(&ctx->git_repo_port, branch, branch_len);(no behavioral change; argument is already const-compatible)
127-131: Walk error handling is lossy (optional)On walk_commits failure, you free the error and proceed with zero parents. Consider returning an error when the ref is missing/invalid to avoid silently creating orphaned commits.
343-367: Ref safety: build refs with gm_build_ref or document contractgm_journal_create_commit copies ref directly into jctx.ref_name, then uses it to find the parent via a ref walk. Per guidelines, refs should be constructed via gm_build_ref and inputs starting with 'refs/' rejected, or the function should clearly document it expects a fully-qualified ref. Pick one and enforce consistently.
As per coding guidelines
core/include/gitmind/hooks/augment.h (3)
41-44: Expose hooks as gm_result_t, not raw int.Per our hooks API policy, public hooks should surface gm_result_t/gm_result_void_t instead of plain int error codes for consistency with ports and better error context. Consider introducing result-typed variants (or migrating these) and keeping the int wrappers as deprecated shims temporarily.
As per coding guidelines
Also applies to: 74-76, 84-86
88-114: Mark legacy inline helpers as deprecated.Add deprecation attributes to steer callers to gm_augment_* names and set a removal window.
Apply this diff to annotate wrappers:
-static inline int get_blob_sha(const gm_git_repository_port_t *repo_port, +static inline __attribute__((deprecated("Use gm_augment_get_blob_sha"))) +int get_blob_sha(const gm_git_repository_port_t *repo_port, const char *commit_ref, const char *file_path, gm_oid_t *sha_out) { return gm_hook_get_blob_sha(repo_port, commit_ref, file_path, sha_out); } @@ -static inline int process_changed_file(gm_context_t *ctx, +static inline __attribute__((deprecated("Use gm_augment_process_changed_file"))) +int process_changed_file(gm_context_t *ctx, const gm_git_repository_port_t *repo_port, const char *file_path) { return gm_hook_process_changed_file(ctx, repo_port, file_path); } @@ -static inline int is_merge_commit(const gm_git_repository_port_t *repo_port, +static inline __attribute__((deprecated("Use gm_augment_is_merge_commit"))) +int is_merge_commit(const gm_git_repository_port_t *repo_port, bool *is_merge) { return gm_hook_is_merge_commit(repo_port, is_merge); }If cross-compiler support is a concern, introduce a GM_DEPRECATED macro in a common header and use that instead. Based on learnings
9-9: Preserve header-guard compatibility alias.If this guard changed from GM_AUGMENT_H previously, add a compatibility alias to avoid breaking includes that key off the old symbol.
Apply this addition near the guard:
#ifndef GITMIND_AUGMENT_H #define GITMIND_AUGMENT_H +/* Back-compat for legacy guard name */ +#ifndef GM_AUGMENT_H +#define GM_AUGMENT_H 1 +#endifBased on learnings
core/tests/fakes/git/fake_git_repository_port.c (2)
114-121: Prefer gm_memset_safe over memset for consistency and bounds checking.Use the safe variant when zeroing structs.
Apply this diff:
- memset(free_slot, 0, sizeof(*free_slot)); + gm_memset_safe(free_slot, sizeof(*free_slot), 0, sizeof(*free_slot)); @@ - memset(free_slot, 0, sizeof(*free_slot)); + gm_memset_safe(free_slot, sizeof(*free_slot), 0, sizeof(*free_slot));As per coding guidelines
561-567: Use gm_memset_safe when zeroing OIDs.Minor consistency and safety improvement.
Apply this diff:
- memset(out_tree_oid, 0, sizeof(*out_tree_oid)); + gm_memset_safe(out_tree_oid, sizeof(*out_tree_oid), 0, + sizeof(*out_tree_oid)); @@ - memset(out_commit_oid, 0, sizeof(*out_commit_oid)); + gm_memset_safe(out_commit_oid, sizeof(*out_commit_oid), 0, + sizeof(*out_commit_oid));As per coding guidelines
Also applies to: 719-724
core/src/app/cache/cache_rebuild_service.c (1)
331-347: LGTM: meta population calls into port for journal tip.Branch name length guard + tip retrieval look correct. Minor: consider checking gm_strcpy_safe’s return even after the length guard for uniformity.
Please confirm GM_CACHE_BRANCH_NAME_SIZE includes space for the NUL terminator across all configs.
core/tests/unit/test_hook_augment.c (2)
65-71: Prefer an OID-equality helper over raw memcmp in tests.Improves readability and future-proofs representation changes.
Example helper to place near the top:
static inline bool gm_oid_equals(const gm_oid_t *a, const gm_oid_t *b) { return a && b && memcmp(a->id, b->id, GM_OID_RAWSZ) == 0; }Then:
assert(gm_oid_equals(&resolved, &new_blob)); assert(gm_oid_equals(&resolved_parent, &old_blob));As per coding guidelines
9-13: Drop unused include.gitmind/context.h appears unused here; remove to keep includes minimal and avoid warnings.
As per coding guidelines
core/src/edge/edge.c (1)
55-82: Avoid duplication: resolve_blob_identity appears duplicatedThis helper matches the one in core/src/edge/attributed.c (lines 41-69). Extract to a shared internal header or a common C module to keep a single source of truth and prevent drift.
Based on learnings
apps/cli/main.c (1)
260-283: Always cleanup on init failure; zero-initialize ctx in main.To centralize shutdown and avoid repeating shutdown calls in init_context, consider:
- Zero-init ctx so cleanup is safe on failure.
- Call cleanup_context(&ctx, &cli) when init_context fails.
Proposed changes (outside the selected range for diffs; illustrative):
C code:
gm_context_t ctx = (gm_context_t){0};
...
result = init_context(&ctx, output_level, output_format, &cli);
if (result != GM_OK) {
cleanup_context(&ctx, &cli);
return EXIT_FAILURE;
}This approach balances git_libgit2_init()/shutdown and simplifies error paths. As per coding guidelines.
core/src/edge/attributed.c (2)
186-197: Fix fallback logic when encoding OIDs.src_raw/tgt_raw pointers are never NULL; fallback to legacy SHA won’t trigger. Prefer git_oid_is_zero() to decide fallback.
Minimal change:
const uint8_t *src_bytes = git_oid_is_zero(&e->src_oid) ? e->src_sha : (const uint8_t *)e->src_oid.id; r = gm_cbor_write_bytes(buffer + offset, avail - offset, src_bytes, GM_OID_RAWSZ); const uint8_t *tgt_bytes = git_oid_is_zero(&e->tgt_oid) ? e->tgt_sha : (const uint8_t *)e->tgt_oid.id; r = gm_cbor_write_bytes(buffer + offset, avail - offset, tgt_bytes, GM_OID_RAWSZ);
325-325: Avoid clang-tidy suppression; fix parameter-swappability instead.Remove NOLINT and make parameters self-documenting (e.g., struct for options or reorder/annotate). Can be follow-up.
As per coding guidelines
core/tests/unit/test_edge.c (1)
122-161: Add OID-first equality coverage in tests.Per project rule, gm_edge_equal must compare OIDs first, only falling back to SHA when OIDs are zero. Consider adding a case where SHAs match but OIDs differ to assert inequality, and a case where OIDs match regardless of SHA arrays.
Based on learnings
core/src/cache/query.c (5)
59-65: Zero output on early error paths to avoid partial/garbage state.If inputs/state are invalid, meta may remain uninitialized. Zero it before returning.
As per coding guidelines
Apply this diff:int gm_cache_load_meta(gm_context_t *ctx, const char *branch, gm_cache_meta_t *meta) { - if (ctx == NULL || branch == NULL || meta == NULL) { - return GM_ERR_INVALID_ARGUMENT; - } - if (ctx->git_repo_port.vtbl == NULL) { - return GM_ERR_INVALID_STATE; - } + if (meta) { + gm_memset_safe(meta, sizeof(*meta), 0, sizeof(*meta)); + } + if (ctx == NULL || branch == NULL || meta == NULL) { + return GM_ERR_INVALID_ARGUMENT; + } + if (ctx->git_repo_port.vtbl == NULL) { + return GM_ERR_INVALID_STATE; + }
99-106: Handle possible truncation when copying branch name.gm_strcpy_safe return must be checked; treat truncation as error and leave field zeroed.
As per coding guidelines
Apply this diff:- meta->version = GM_CACHE_VERSION; - meta->shard_bits = GM_CACHE_SHARD_BITS; - (void)gm_strcpy_safe(meta->branch, GM_CACHE_BRANCH_NAME_SIZE, branch); - meta->branch[GM_CACHE_BRANCH_NAME_SIZE - 1] = '\0'; + meta->version = GM_CACHE_VERSION; + meta->shard_bits = GM_CACHE_SHARD_BITS; + if (gm_strcpy_safe(meta->branch, GM_CACHE_BRANCH_NAME_SIZE, branch) != GM_OK) { + gm_memset_safe(meta->branch, GM_CACHE_BRANCH_NAME_SIZE, 0, GM_CACHE_BRANCH_NAME_SIZE); + return GM_ERR_BUFFER_TOO_SMALL; + }
122-133: Check and enforce non-truncation when copying journal tip hex.Avoid silent truncation; either return error or clear the string on failure.
As per coding guidelines
Apply this diff:- } else { - meta->journal_tip_oid_bin = journal_tip.oid; - (void)gm_strcpy_safe(meta->journal_tip_oid, sizeof(meta->journal_tip_oid), - journal_tip.oid_hex); + } else { + meta->journal_tip_oid_bin = journal_tip.oid; + if (gm_strcpy_safe(meta->journal_tip_oid, sizeof(meta->journal_tip_oid), + journal_tip.oid_hex) != GM_OK) { + meta->journal_tip_oid[0] = '\0'; + return GM_ERR_BUFFER_TOO_SMALL; + } }
173-185: Avoid direct libgit2 usage in cache; keep it behind ports/util wrappers.git_oid_fromstr ties cache/query.c to libgit2. Prefer a gm_oid_parse helper or a port-provided compare to keep the module decoupled.
As per coding guidelines
203-208: Remove libgit2 dependency for OID hex formatting in cache.Using git_oid_tostr in cache couples the module back to libgit2. Consider a small local hex formatter or a gm_oid_format utility.
As per coding guidelines
Example replacement:static void oid_raw_to_hex(const uint8_t *raw, char *hex_out /* size >= 41 */) { static const char *digits = "0123456789abcdef"; for (size_t i = 0; i < GM_OID_RAWSZ; ++i) { hex_out[i*2] = digits[(raw[i] >> 4) & 0xF]; hex_out[i*2 + 1] = digits[raw[i] & 0xF]; } hex_out[GM_OID_HEX_CHARS] = '\0'; }Then replace the git_oid_fromraw/git_oid_tostr block with oid_raw_to_hex.
core/src/adapters/git/libgit2_repository_port.c (8)
42-74: Zero output buffer on failure to avoid partial writes.Initialize out_buffer to empty and clear it on copy failure.
As per coding guidelines
Apply this diff:static gm_result_void_t repository_path_impl(gm_libgit2_repository_port_state_t *state, gm_git_repository_path_kind_t kind, char *out_buffer, size_t buffer_size) { - if (out_buffer == NULL || buffer_size == 0U) { + if (out_buffer == NULL || buffer_size == 0U) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "repository path output buffer missing")); } + out_buffer[0] = '\0'; @@ - if (gm_strcpy_safe(out_buffer, buffer_size, source) != GM_OK) { + if (gm_strcpy_safe(out_buffer, buffer_size, source) != GM_OK) { + out_buffer[0] = '\0'; return gm_err_void( GM_ERROR(GM_ERR_PATH_TOO_LONG, "repository path exceeds buffer")); }
76-104: Zero head branch output on failure to prevent garbage.Set out_name empty on entry and on copy error.
As per coding guidelines
Apply this diff:static gm_result_void_t head_branch_impl(gm_libgit2_repository_port_state_t *state, char *out_name, size_t out_name_size) { - if (out_name == NULL || out_name_size == 0U) { + if (out_name == NULL || out_name_size == 0U) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "head branch requires buffer")); } + out_name[0] = '\0'; @@ - if (copy_status != GM_OK) { + if (copy_status != GM_OK) { + out_name[0] = '\0'; return gm_err_void( GM_ERROR(GM_ERR_PATH_TOO_LONG, "head branch name exceeds buffer")); }
435-442: Initialize out_data/out_size to safe defaults.Guarantees callers don’t observe uninitialized outputs on error exits.
As per coding guidelines
Apply this diff:static gm_result_void_t commit_read_blob_impl( gm_libgit2_repository_port_state_t *state, const gm_oid_t *commit_oid, const char *path, uint8_t **out_data, size_t *out_size) { if (commit_oid == NULL || path == NULL || out_data == NULL || out_size == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "commit read blob requires inputs")); } + *out_data = NULL; + *out_size = 0;
491-498: Zero out blob OID on failure paths.Initialize output to zero to maintain invariant on error.
As per coding guidelines
Apply this diff:static gm_result_void_t resolve_blob_at_head_impl( gm_libgit2_repository_port_state_t *state, const char *path, gm_oid_t *out_blob_oid) { if (path == NULL || out_blob_oid == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "blob resolve requires path and output")); } + gm_memset_safe(out_blob_oid, sizeof(*out_blob_oid), 0, sizeof(*out_blob_oid));
540-547: Zero out blob OID on failure paths.Same as resolve_blob_at_head_impl.
As per coding guidelines
Apply this diff:static gm_result_void_t resolve_blob_at_commit_impl( gm_libgit2_repository_port_state_t *state, const gm_oid_t *commit_oid, const char *path, gm_oid_t *out_blob_oid) { if (commit_oid == NULL || path == NULL || out_blob_oid == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "blob resolve requires commit, path, output")); } + gm_memset_safe(out_blob_oid, sizeof(*out_blob_oid), 0, sizeof(*out_blob_oid));
575-594: Initialize parent count to zero before work.Ensures deterministic output on errors.
As per coding guidelines
Apply this diff:static gm_result_void_t commit_parent_count_impl( gm_libgit2_repository_port_state_t *state, const gm_oid_t *commit_oid, size_t *out_parent_count) { if (commit_oid == NULL || out_parent_count == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "parent count requires commit and output")); } + *out_parent_count = 0;
727-735: Initialize output commit OID to zero.Keeps output consistent on failure paths.
As per coding guidelines
Apply this diff:static gm_result_void_t commit_create_impl( gm_libgit2_repository_port_state_t *state, const gm_git_commit_spec_t *spec, gm_oid_t *out_commit_oid) { if (spec == NULL || spec->tree_oid == NULL || spec->message == NULL || out_commit_oid == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "commit spec missing inputs")); } + gm_memset_safe(out_commit_oid, sizeof(*out_commit_oid), 0, sizeof(*out_commit_oid));
365-395: Set out_message to NULL on error to avoid dangling pointers.Initialize and only set to heap string on success; free via disposer.
As per coding guidelines
Apply this diff:static gm_result_void_t commit_read_message_impl( gm_libgit2_repository_port_state_t *state, const gm_oid_t *commit_oid, char **out_message) { if (commit_oid == NULL || out_message == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_ARGUMENT, "commit read message requires inputs")); } + *out_message = NULL;core/src/hooks/augment.c (3)
104-106: Zero out sha_out early to avoid partial/undefined outputs on failure.Aligns with “zero output buffers on failure” guidance.
As per coding guidelines
- size_t offset = 0U; + gm_memset_safe(sha_out, sizeof *sha_out, 0, sizeof *sha_out); + size_t offset = 0U;
262-273: Only treat NOT_FOUND as “new/deleted file”; propagate real errors.This avoids masking unexpected failures from the repo port.
- error = gm_hook_get_blob_sha(repo_port, "HEAD~1", file_path, &old_oid); - if (error != GM_OK) { - /* File might be new, skip */ - return GM_OK; - } + error = gm_hook_get_blob_sha(repo_port, "HEAD~1", file_path, &old_oid); + if (error == GM_ERR_NOT_FOUND) { + /* File might be new, skip */ + return GM_OK; + } else if (error != GM_OK) { + return error; + } - error = gm_hook_get_blob_sha(repo_port, "HEAD", file_path, &new_oid); - if (error != GM_OK) { - /* File might be deleted, skip */ - return GM_OK; - } + error = gm_hook_get_blob_sha(repo_port, "HEAD", file_path, &new_oid); + if (error == GM_ERR_NOT_FOUND) { + /* File might be deleted, skip */ + return GM_OK; + } else if (error != GM_OK) { + return error; + }
83-94: Consider early-terminating the commit walk once capacity is reached.If walk_commits honors a non-zero visitor return to stop, you can avoid unnecessary traversal.
Would you like me to propose a guarded early-return change once we confirm the walk callback stop semantics?
Also applies to: 143-146
core/tests/fakes/git/fake_git_repository_port.h (1)
7-8: Make the header self-contained for bool/size_t.Include std headers to avoid relying on transitive includes.
#include "gitmind/ports/git_repository_port.h" +#include <stdbool.h> +#include <stddef.h>
| char full_path[GM_PATH_MAX]; | ||
| if (gm_snprintf(full_path, sizeof(full_path), "%s/%s", dir_path, entry_name) < | ||
| 0) { | ||
| return gm_err_void( | ||
| GM_ERROR(GM_ERR_PATH_TOO_LONG, "entry path exceeds buffer")); | ||
| } | ||
|
|
There was a problem hiding this comment.
Check snprintf truncation, not just negative return.
gm_snprintf can return a value >= buffer size; treat as buffer-too-small.
As per coding guidelines
Apply this diff:
- if (gm_snprintf(full_path, sizeof(full_path), "%s/%s", dir_path, entry_name) <
- 0) {
+ int rn = gm_snprintf(full_path, sizeof(full_path), "%s/%s", dir_path, entry_name);
+ if (rn < 0 || (size_t)rn >= sizeof(full_path)) {
return gm_err_void(
GM_ERROR(GM_ERR_PATH_TOO_LONG, "entry path exceeds buffer"));
}🤖 Prompt for AI Agents
In core/src/adapters/git/libgit2_repository_port.c around lines 191 to 197, the
gm_snprintf call is only checking for negative return but not for truncation
(return >= buffer size); change the logic to capture the return value (int or
ssize_t), check if it is negative (error) or >= (int)sizeof(full_path)
(truncated), and in either case return gm_err_void with GM_ERR_PATH_TOO_LONG for
truncation or preserve existing error handling for negative results; ensure you
use the captured length for the comparison and do not assume the buffer contains
a complete path when gm_snprintf indicates truncation.
…o chore/more-fixes
…o chore/more-fixes
…o chore/more-fixes
…o chore/more-fixes
…o chore/more-fixes
…Abstraction This commit streamlines memory management by centralizing git_repository ownership outside of the libgit2 port, ensuring robust cleanup across the application lifecycle. It also introduces internal, non-libgit2 OID utility functions to decouple core cache logic from external dependencies. Core Changes Repository Ownership Consolidation: The git_repository * object is now explicitly managed in the gm_context_t (ctx->user_data in the CLI) or the calling function (in benchmarks and hooks). The libgit2_repository_port adapter's dispose function no longer calls git_repository_free(), making it a pure port-disposal function. This fixes multiple cleanup bugs in apps/cli/main.c, benchmarks/wildebeest_stampede.c, and core/src/hooks/post-commit.c by ensuring the underlying libgit2 repository is freed exactly once during context teardown. OID Utility Abstraction: Moved the OID utility functions (gm_raw_to_hex, gm_hex_to_raw, gm_hex_to_oid, gm_oid_equal) from internal libgit2 headers into core/src/cache/query.c. This eliminates the reliance on libgit2/oid.h in core cache query logic, improving isolation. The cache staleness check (gm_cache_is_stale) now uses the new internal gm_oid_equal for comparison, further detaching cache metadata handling from the libgit2 API. Path Utility Refinement: Significantly refactored the complex and highly-analyzed gm_fs_path_normalize_logical and gm_fs_path_dirname functions in core/src/fs/path_utils.c. Introduced smaller, dedicated helper functions (next_segment, push_segment, handle_segment, etc.) to reduce cognitive complexity, enforce stricter buffer checks, and improve overall correctness and maintainability. Cleanup and Robustness: Improved error handling in the journal reader (core/src/journal/reader.c) to correctly extract and free errors from the Git repository port. Replaced direct memcpy with gm_memcpy_span in test_journal_mixed_cbor.c for safer buffer manipulation in tests. Added a check for absolute paths in fake_fs_temp_port.c canonicalization to ensure test-case correctness.
…o chore/more-fixes
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/legacy/src/sha.md (1)
1-12: Restore required doc boilerplate.Per our docs guidelines, every
docs/**/*.mdfile must add the SPDX/License comments immediately after the front matter and include a## Table of Contentssection near the top. This page still lacks both, so the build will fail the docs gate until they’re added. Please drop the standard SPDX header under the front matter and insert the TOC section before diving into content. As per coding guidelinesapps/cli/main.md (1)
24-31: Update the gm_context_t snippet to show the new repository port fieldsThe snippet still advertises the legacy
void *git_repo, but this PR retires that pointer in favor of the port/disposer pair. Please sync the documentation with the actual definition incore/include/gitmind/context.h.typedef struct { - void *git_repo; + gm_git_repository_port_t git_repo_port; + void (*git_repo_port_dispose)(gm_git_repository_port_t *); void (*log_fn)(int level, const char *fmt, ...); // ... extensible } gm_context_t;docs/architecture/augments-system.md (1)
16-22: Promote the contents list to the mandated## Table of ContentssectionPer the docs style guide, the TOC must be a second-level heading titled exactly “Table of Contents”. Right now it’s plain text, so docs verification will complain.
-Table of Contents - -- [Overview](#overview) +## Table of Contents + +- [Overview](#overview) - [Core Concepts](#core-concepts) - [How It Works](#how-it-works) - [Detailed Design](#detailed-design)As per coding guidelines
🧹 Nitpick comments (8)
core/include/gitmind/ports/git_repository_port.h (8)
53-53: Drop redundant typedef redeclarationYou forward-declare and then re-typedef the same vtbl type. Keep only the defining typedef to avoid double-declaration warnings.
Apply this diff:
-typedef struct gm_git_repository_port_vtbl gm_git_repository_port_vtbl_t;
51-52: Clarify walk callback contract (continue/stop/error)Document return semantics (e.g., 0 = continue, >0 = stop gracefully, <0 = abort with error) or introduce a small enum to make it explicit.
Minimal doc addition:
-typedef int (*gm_git_commit_visit_cb)(const gm_oid_t *commit_oid, void *userdata); +/* Return 0 to continue; >0 to stop iteration gracefully; <0 to abort (adapter maps to error). */ +typedef int (*gm_git_commit_visit_cb)(const gm_oid_t *commit_oid, void *userdata);Also applies to: 79-82
158-170: Pre-clear out params to a safe stateIf the port/vtbl is invalid, callers could observe stale values. Set outputs to NULL/0 up front.
GM_NODISCARD static inline gm_result_void_t gm_git_repository_port_commit_read_blob(const gm_git_repository_port_t *port, const gm_oid_t *commit_oid, const char *path, uint8_t **out_data, size_t *out_size) { + if (out_data) *out_data = NULL; + if (out_size) *out_size = 0; if (port == NULL || port->vtbl == NULL || port->vtbl->commit_read_blob == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_STATE, "git repository port missing commit_read_blob")); } return port->vtbl->commit_read_blob(port->self, commit_oid, path, out_data, out_size); }
172-182: Pre-clear out params for commit messageSame rationale as above.
GM_NODISCARD static inline gm_result_void_t gm_git_repository_port_commit_read_message( const gm_git_repository_port_t *port, const gm_oid_t *commit_oid, char **out_message) { + if (out_message) *out_message = NULL; if (port == NULL || port->vtbl == NULL || port->vtbl->commit_read_message == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_STATE, "git repository port missing commit_read_message")); } return port->vtbl->commit_read_message(port->self, commit_oid, out_message); }
100-110: Set out_buffer[0] = '\0' on entryGuarantees a safe string on failure paths. Low-cost defensive default.
GM_NODISCARD static inline gm_result_void_t gm_git_repository_port_repository_path(const gm_git_repository_port_t *port, gm_git_repository_path_kind_t kind, char *out_buffer, size_t buffer_size) { + if (out_buffer && buffer_size) out_buffer[0] = '\0'; if (port == NULL || port->vtbl == NULL || port->vtbl->repository_path == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_STATE, "git repository port missing repository_path")); } return port->vtbl->repository_path(port->self, kind, out_buffer, buffer_size); }
112-120: Set out_name[0] = '\0' on entrySame defensive default for branch name.
GM_NODISCARD static inline gm_result_void_t gm_git_repository_port_head_branch(const gm_git_repository_port_t *port, char *out_name, size_t out_name_size) { + if (out_name && out_name_size) out_name[0] = '\0'; if (port == NULL || port->vtbl == NULL || port->vtbl->head_branch == NULL) { return gm_err_void(GM_ERROR(GM_ERR_INVALID_STATE, "git repository port missing head_branch")); } return port->vtbl->head_branch(port->self, out_name, out_name_size); }
30-36: Avoid duplicated state or document invariant
gm_git_reference_tip_tholds bothoidand its hex string. Either removeoid_hexor document that adapters must fill both and they must match (git_oid_fmt(lowercase)), to prevent drift.
44-50: Document ref safety/normalizationClarify
ref_namerequirements (normalized ref, safety rules). This aligns with the guideline to construct refs viagm_build_refand reject unsafe names.Proposed doc:
typedef struct { - const char *ref_name; + /* Normalized ref name (e.g., "refs/heads/…"); unsafe inputs are rejected by adapter. */ + const char *ref_name; const gm_oid_t *target_oid; const char *log_message; bool force; } gm_git_reference_update_spec_t;
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/images/PRPhiedbachUndBunBun.pngis excluded by!**/*.png
📒 Files selected for processing (36)
AGENTS.md(2 hunks)apps/cli/main.c(4 hunks)apps/cli/main.md(1 hunks)benchmarks/wildebeest_stampede.c(7 hunks)core/include/gitmind/hooks/augment.h(5 hunks)core/include/gitmind/ports/git_repository_port.h(1 hunks)core/include/gitmind/security/string.h(1 hunks)core/src/adapters/fs/posix_temp_adapter.c(1 hunks)core/src/adapters/git/libgit2_repository_port.c(1 hunks)core/src/app/cache/cache_rebuild_service.c(11 hunks)core/src/cache/query.c(8 hunks)core/src/edge/attributed.c(3 hunks)core/src/edge/edge.c(3 hunks)core/src/fs/path_utils.c(2 hunks)core/src/hooks/augment.c(5 hunks)core/src/hooks/post-commit.c(6 hunks)core/src/journal/reader.c(7 hunks)core/src/journal/writer.c(7 hunks)core/tests/fakes/fs/fake_fs_temp_port.c(4 hunks)core/tests/fakes/git/fake_git_repository_port.c(1 hunks)core/tests/fakes/git/fake_git_repository_port.h(1 hunks)core/tests/unit/test_cache_edge_map.c(1 hunks)core/tests/unit/test_cache_query.c(3 hunks)core/tests/unit/test_journal_mixed_cbor.c(3 hunks)docs/architecture/FEEDBACK_hexagonal-ports-and-adapters.md(1 hunks)docs/architecture/MIGRATION_PHILOSOPHY.md(1 hunks)docs/architecture/augments-system.md(5 hunks)docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md(1 hunks)docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.md(1 hunks)docs/legacy/src/attribution.md(1 hunks)docs/legacy/src/edge.md(1 hunks)docs/legacy/src/sha.md(1 hunks)docs/talk-shop/Graph-Lvl-Up-Tank.md(1 hunks)docs/tools/Review_Worksheet_Flow.md(1 hunks)tools/docs/check_docs.py(1 hunks)tools/docs/check_titles.py(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- docs/talk-shop/Graph-Lvl-Up-Tank.md
- docs/legacy/src/attribution.md
- docs/legacy/src/edge.md
- docs/code-reviews/PR171/258c9f05de4f483c8c9a039df4b2307bba72ed97.md
- docs/architecture/FEEDBACK_hexagonal-ports-and-adapters.md
- docs/architecture/MIGRATION_PHILOSOPHY.md
🚧 Files skipped from review as they are similar to previous changes (4)
- core/include/gitmind/security/string.h
- core/src/adapters/git/libgit2_repository_port.c
- core/tests/unit/test_journal_mixed_cbor.c
- docs/code-reviews/PR171/7195186ca1374dcef93a846f882a11ebbc7fb861.md
🧰 Additional context used
📓 Path-based instructions (11)
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Run
make docs-verifywhen changing documentation
Files:
docs/legacy/src/sha.mddocs/architecture/augments-system.mddocs/tools/Review_Worksheet_Flow.md
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Docs must include front matter first, a single H1 whose title matches the filename title, a '## Table of Contents' section, and API docs must include api_version; add License/SPDX comments after front matter
Files:
docs/legacy/src/sha.mddocs/architecture/augments-system.mddocs/tools/Review_Worksheet_Flow.md
**/*.{c,cc,cpp,cxx,h,hpp,hxx,m,mm}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use
NOLINTto suppress clang-tidy warnings; ALWAYS fix the underlying issue
Files:
core/src/adapters/fs/posix_temp_adapter.ccore/src/edge/attributed.ccore/tests/unit/test_cache_query.ccore/src/edge/edge.cbenchmarks/wildebeest_stampede.ccore/include/gitmind/hooks/augment.hcore/tests/fakes/fs/fake_fs_temp_port.ccore/tests/fakes/git/fake_git_repository_port.ccore/src/hooks/augment.ccore/src/hooks/post-commit.ccore/src/cache/query.ccore/src/fs/path_utils.capps/cli/main.ccore/include/gitmind/ports/git_repository_port.hcore/tests/unit/test_cache_edge_map.ccore/src/journal/writer.ccore/src/journal/reader.ccore/tests/fakes/git/fake_git_repository_port.hcore/src/app/cache/cache_rebuild_service.c
{core,include}/**/*.{c,h}
📄 CodeRabbit inference engine (AGENTS.md)
{core,include}/**/*.{c,h}: Use C23 with warnings-as-errors; no VLAs or shadowing; declare explicit prototypes
Formatting must follow .clang-format (LLVM-based, 4 spaces, 80 cols, right-aligned pointers)
Naming: functions/vars lower_snake_case with gm_ prefix; macros UPPER_SNAKE; types end with _t
Prefer specific includes; order/regroup includes per .clang-format
Use OID-first equality and APIs; when both OIDs are present they must match; compare using git_oid_cmp
Construct refs via gm_build_ref, reject inputs starting with 'refs/' and enforce ref-name safety
Use safe string/memory functions (gm_snprintf, gm_strcpy_safe, gm_memcpy_span); check return codes; treat truncation as error; zero output buffers on error paths
Prefer GM_PATH_MAX; do not use GM_PATH_MAX*2; allocate if larger is required
Keep touched code SRP-compliant and injection-friendly: isolate side effects behind outbound ports and pass dependencies explicitly
Keep clang-tidy at zero warnings for touched files; do not merge new diagnostics
Zero out output buffers on failure before formatting or copying to prevent partial writes
Files:
core/src/adapters/fs/posix_temp_adapter.ccore/src/edge/attributed.ccore/tests/unit/test_cache_query.ccore/src/edge/edge.ccore/include/gitmind/hooks/augment.hcore/tests/fakes/fs/fake_fs_temp_port.ccore/tests/fakes/git/fake_git_repository_port.ccore/src/hooks/augment.ccore/src/hooks/post-commit.ccore/src/cache/query.ccore/src/fs/path_utils.ccore/include/gitmind/ports/git_repository_port.hcore/tests/unit/test_cache_edge_map.ccore/src/journal/writer.ccore/src/journal/reader.ccore/tests/fakes/git/fake_git_repository_port.hcore/src/app/cache/cache_rebuild_service.c
core/src/adapters/**/*_adapter.{c,h}
📄 CodeRabbit inference engine (AGENTS.md)
Runtime adapters must be placed under core/src/adapters// with filenames ending in _adapter and paired private headers
Files:
core/src/adapters/fs/posix_temp_adapter.c
core/src/adapters/**
📄 CodeRabbit inference engine (AGENTS.md)
Each adapter must publish a factory returning the port vtable plus state, and provide teardown hooks
Files:
core/src/adapters/fs/posix_temp_adapter.c
core/tests/unit/test_*.c
📄 CodeRabbit inference engine (AGENTS.md)
Unit tests must live under core/tests/unit as test_.c, be deterministic and isolated
Files:
core/tests/unit/test_cache_query.ccore/tests/unit/test_cache_edge_map.c
{include,core/include}/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
{include,core/include}/**/*.h: Public headers must be umbrella-safe: compile standalone, include directly, have stable header guards, and provide C++ linkage guards
Header guards must follow GITMIND_* naming and remain stable
Ensure public headers compile standalone via header-compile checks
Files:
core/include/gitmind/hooks/augment.hcore/include/gitmind/ports/git_repository_port.h
core/include/gitmind/hooks/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
Hooks APIs must surface gm_result_t results and avoid raw integer returns
Files:
core/include/gitmind/hooks/augment.h
core/tests/fakes/**
📄 CodeRabbit inference engine (AGENTS.md)
Provide deterministic fakes/mocks/stubs for each port under core/tests/fakes
Files:
core/tests/fakes/fs/fake_fs_temp_port.ccore/tests/fakes/git/fake_git_repository_port.ccore/tests/fakes/git/fake_git_repository_port.h
core/include/gitmind/ports/**/*.h
📄 CodeRabbit inference engine (AGENTS.md)
Ports headers must declare interfaces only (no non-trivial inline behavior) and use gm_result_t signatures
Files:
core/include/gitmind/ports/git_repository_port.h
🧠 Learnings (27)
📓 Common learnings
Learnt from: flyingrobots
PR: neuroglyph/git-mind#169
File: docs/templates/code-review-feedback.md:26-33
Timestamp: 2025-09-16T17:10:27.477Z
Learning: The user (flyingrobots) uses Obsidian to view/edit Markdown files and prefers to keep Obsidian-specific syntax like collapsible callout markers (e.g., `[!NOTE]-` for collapsible callouts) even if they're not GitHub-compatible.
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/docs/architecture/deprecations/** : Document migration paths for breaking changes (e.g., cache hash format, SHA→OID) under architecture deprecations
Applied to files:
docs/legacy/src/sha.md
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to {core,include}/**/*.{c,h} : Language: C23 with warnings-as-errors; no VLAs; no variable shadowing; use explicit prototypes
Applied to files:
core/src/adapters/fs/posix_temp_adapter.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/edge.h : Avoid struct bloat: do not store both legacy SHA arrays and OIDs long-term in gm_edge_t; deprecate and phase out legacy SHA fields
Applied to files:
core/src/edge/attributed.ccore/src/edge/edge.ccore/src/hooks/augment.ccore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/edge/edge.c : gm_edge_equal() must compare OIDs first and only fall back to SHA when OIDs are unset/zero; never treat mismatched OIDs as equal due to matching SHAs
Applied to files:
core/src/edge/attributed.ccore/src/edge/edge.cbenchmarks/wildebeest_stampede.ccore/src/hooks/augment.ccore/src/cache/query.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to {core,include}/**/*.{c,h} : Use safe string/memory functions (gm_snprintf, gm_strcpy_safe, gm_memcpy_span); check return codes; treat truncation as error; zero output buffers on error paths
Applied to files:
core/src/edge/attributed.ccore/tests/fakes/git/fake_git_repository_port.ccore/src/cache/query.ccore/src/fs/path_utils.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to include/gitmind.h : Maintain umbrella API at include/gitmind.h
Applied to files:
core/src/edge/attributed.cbenchmarks/wildebeest_stampede.ccore/include/gitmind/hooks/augment.hcore/src/hooks/augment.ccore/src/cache/query.capps/cli/main.ccore/src/journal/writer.ccore/src/journal/reader.ccore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to core/include/gitmind/hooks/**/*.h : Hooks APIs must surface gm_result_t results and avoid raw integer returns
Applied to files:
core/src/edge/attributed.ccore/include/gitmind/hooks/augment.hcore/src/hooks/augment.ccore/tests/unit/test_cache_edge_map.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to core/include/gitmind/ports/**/*.h : Ports headers must declare interfaces only (no non-trivial inline behavior) and use gm_result_t signatures
Applied to files:
core/src/edge/attributed.ccore/include/gitmind/hooks/augment.hcore/include/gitmind/ports/git_repository_port.hcore/src/journal/writer.ccore/src/journal/reader.ccore/tests/fakes/git/fake_git_repository_port.hcore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to docs/**/*.md : Docs must include front matter first, a single H1 whose title matches the filename title, a '## Table of Contents' section, and API docs must include api_version; add License/SPDX comments after front matter
Applied to files:
docs/architecture/augments-system.mddocs/tools/Review_Worksheet_Flow.md
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/cache/{builder,query}.c : Keep naming consistent with OID terminology (e.g., OID_HASH_MULTIPLIER, oid_hash) after migration from SHA
Applied to files:
benchmarks/wildebeest_stampede.ccore/src/cache/query.ccore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/cache/{builder,query}.c : Version and migrate cache formats when changing the hash function (e.g., raw OID vs SHA); auto-rebuild or reject mismatched versions
Applied to files:
benchmarks/wildebeest_stampede.ccore/src/cache/query.ccore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/hooks/augment.h : Prefix public hook APIs with gm_hook_ and provide deprecation warnings for legacy macro/function names with a clear removal timeline
Applied to files:
core/include/gitmind/hooks/augment.hcore/src/hooks/augment.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/hooks/augment.h : Maintain stable header guards or provide compatibility for guard renames (GM_AUGMENT_H → GITMIND_AUGMENT_H) to avoid breaking includes
Applied to files:
core/include/gitmind/hooks/augment.hcore/src/hooks/augment.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to core/tests/fakes/** : Provide deterministic fakes/mocks/stubs for each port under core/tests/fakes
Applied to files:
core/tests/fakes/git/fake_git_repository_port.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/**/*.{c,h} : Use bounded string functions (e.g., gm_snprintf) and check return values for overflow; ensure explicit NUL termination
Applied to files:
core/tests/fakes/git/fake_git_repository_port.ccore/src/cache/query.ccore/src/fs/path_utils.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/**/*.{c,h} : Use consistent error handling across modules (prefer GM_OK/GM_ERR_* over mixing with raw libgit2 codes)
Applied to files:
core/tests/fakes/git/fake_git_repository_port.ccore/src/hooks/augment.capps/cli/main.ccore/src/journal/writer.ccore/src/journal/reader.ccore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to {core,include}/**/*.{c,h} : Construct refs via gm_build_ref, reject inputs starting with 'refs/' and enforce ref-name safety
Applied to files:
core/tests/fakes/git/fake_git_repository_port.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to {core,include}/**/*.{c,h} : Use OID-first equality and APIs; when both OIDs are present they must match; compare using git_oid_cmp
Applied to files:
core/src/hooks/augment.ccore/src/journal/reader.ccore/src/app/cache/cache_rebuild_service.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to {core,include}/**/*.{c,h} : Prefer GM_PATH_MAX; do not use GM_PATH_MAX*2; allocate if larger is required
Applied to files:
core/src/cache/query.ccore/src/fs/path_utils.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to {include,core/include}/**/*.h : Header guards use GITMIND_* naming
Applied to files:
core/src/cache/query.c
📚 Learning: 2025-09-20T17:35:43.695Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-20T17:35:43.695Z
Learning: Applies to {core,include}/**/*.{c,h} : Zero out output buffers on failure before formatting or copying to prevent partial writes
Applied to files:
core/src/fs/path_utils.c
📚 Learning: 2025-09-13T03:13:54.145Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: AGENTS.md:0-0
Timestamp: 2025-09-13T03:13:54.145Z
Learning: Applies to include/gitmind/**/*.h : Public, namespaced headers live under include/gitmind/
Applied to files:
apps/cli/main.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/journal/{reader,writer}.c : Eliminate CBOR magic numbers; use centralized keys from gitmind/cbor/keys.h
Applied to files:
core/src/journal/writer.ccore/src/journal/reader.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/journal/reader.c : Keep decoder functions small and composable; split gm_edge_attributed_decode_cbor_ex into logical helpers (avoid 150+ line monoliths)
Applied to files:
core/src/journal/reader.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/src/journal/{reader,writer}.c : Base64-encode CBOR payloads for commit messages to avoid binary corruption
Applied to files:
core/src/journal/reader.c
📚 Learning: 2025-09-19T03:59:13.318Z
Learnt from: CR
PR: neuroglyph/git-mind#0
File: docs/code-reviews/CLAUDE.md:0-0
Timestamp: 2025-09-19T03:59:13.318Z
Learning: Applies to docs/code-reviews/core/include/gitmind/cbor/keys.h : Define CBOR field keys as named constants/enums in a single header and include it where parsing/encoding occurs
Applied to files:
core/src/journal/reader.c
🧬 Code graph analysis (20)
apps/cli/main.md (1)
core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)
core/src/edge/attributed.c (4)
core/src/edge/edge.c (1)
resolve_blob_identity(55-83)core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)core/include/gitmind/edge.h (1)
GM_SHA1_SIZE(32-45)core/include/gitmind/edge_attributed.h (1)
GM_SHA1_SIZE(30-48)
docs/architecture/augments-system.md (1)
core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)
core/tests/unit/test_cache_query.c (2)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(1039-1067)core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)
docs/tools/Review_Worksheet_Flow.md (1)
tools/review/check_worksheets.py (1)
main(70-86)
core/src/edge/edge.c (4)
core/src/edge/attributed.c (1)
resolve_blob_identity(43-71)core/include/gitmind/util/memory.h (1)
gm_memcpy_span(32-47)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/include/gitmind/edge.h (1)
GM_SHA1_SIZE(32-45)
benchmarks/wildebeest_stampede.c (3)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(1039-1067)core/src/cache/query.c (1)
gm_cache_query_fanout(427-431)core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)
core/include/gitmind/hooks/augment.h (2)
core/src/hooks/augment.c (3)
gm_hook_get_blob_sha(100-167)gm_hook_process_changed_file(270-316)gm_hook_is_merge_commit(319-362)core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)
core/tests/fakes/fs/fake_fs_temp_port.c (1)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)
core/tests/fakes/git/fake_git_repository_port.c (2)
core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/src/hooks/augment.c (6)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/src/util/ref.c (1)
gm_build_ref(47-84)core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/src/types/ulid.c (1)
gm_ulid_generate(118-126)core/src/journal/writer.c (1)
gm_journal_append(333-337)
core/src/hooks/post-commit.c (5)
core/include/gitmind/hooks/augment.h (2)
is_merge_commit(111-114)process_changed_file(105-109)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(1039-1067)core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)
core/src/cache/query.c (6)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/src/util/ref.c (1)
gm_build_ref(47-84)core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)core/src/cache/bitmap.c (1)
gm_bitmap_deserialize(84-109)core/include/gitmind/cache.h (1)
GM_CACHE_OID_STRING_SIZE(43-52)
core/src/fs/path_utils.c (2)
core/include/gitmind/util/memory.h (2)
gm_strcpy_safe(113-138)gm_memcpy_span(32-47)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
apps/cli/main.c (3)
core/src/adapters/git/libgit2_repository_port.c (1)
gm_libgit2_repository_port_create(1039-1067)core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)
core/include/gitmind/ports/git_repository_port.h (3)
core/include/gitmind/ports/fs_temp_port.h (9)
gm_result_void_t(76-84)gm_result_void_t(86-97)gm_result_void_t(99-108)gm_result_void_t(110-124)gm_result_void_t(126-138)gm_result_void_t(140-147)struct gm_fs_temp_port_vtbl(50-50)self(56-72)struct {(23-26)core/include/gitmind/context.h (2)
repo(31-51)repo(33-39)core/include/gitmind/ports/env_port.h (1)
struct gm_env_port {(18-21)
core/src/journal/writer.c (2)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)
core/src/journal/reader.c (2)
core/src/error/error.c (1)
gm_error_free(161-178)core/include/gitmind/ports/git_repository_port.h (1)
gm_git_repository_port_commit_message_dispose(184-192)
core/tests/fakes/git/fake_git_repository_port.h (1)
core/tests/fakes/git/fake_git_repository_port.c (18)
gm_fake_git_repository_port_init(195-229)gm_fake_git_repository_port_dispose(231-237)gm_fake_git_repository_port_set_tip(239-245)gm_fake_git_repository_port_set_head_branch(247-261)gm_fake_git_repository_port_clear_ref_commits(263-270)gm_fake_git_repository_port_add_ref_commit(272-306)gm_fake_git_repository_port_clear_blob_mappings(308-315)gm_fake_git_repository_port_add_blob_mapping(317-356)gm_fake_git_repository_port_add_commit_blob_mapping(358-398)gm_fake_git_repository_port_set_commit_parents(400-432)gm_fake_git_repository_port_set_next_tree(434-447)gm_fake_git_repository_port_set_next_commit(449-462)gm_fake_git_repository_port_set_update_result(464-470)gm_fake_git_repository_port_last_commit_message(472-475)gm_fake_git_repository_port_last_commit_tree(477-480)gm_fake_git_repository_port_last_update_ref(482-485)gm_fake_git_repository_port_last_update_log(487-490)gm_fake_git_repository_port_last_update_target(492-495)
core/src/app/cache/cache_rebuild_service.c (2)
core/include/gitmind/security/memory.h (1)
gm_memset_safe(47-59)core/include/gitmind/util/memory.h (1)
gm_strcpy_safe(113-138)
🔇 Additional comments (11)
tools/docs/check_docs.py (1)
13-13: Skip list aligns with new doc bucketsExtending
SKIP_DIRSto covercode-reviewsandlegacykeepsfind_md_filesand the downstream link/ToC checks from recursing into those archival areas, which matches the updated docs policy.tools/docs/check_titles.py (1)
41-41: Path filter matches the broader skip policyMirroring the
code-reviewsandlegacyskips here keeps the title check in sync with the linker check, so we won’t flag intentionally archival Markdown.core/src/adapters/fs/posix_temp_adapter.c (1)
4-4: Guard_GNU_SOURCEdefinition to keep -Werror builds greenOur build already defines
_GNU_SOURCE; redefining it here triggers-Wmacro-redefined, which is fatal under-Werror. Please wrap the define so it only fires when the macro wasn’t supplied by the toolchain.+#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endifdocs/tools/Review_Worksheet_Flow.md (1)
1-13: Restore required license block and Table of ContentsDocs under
docs/must carry the SPDX/license comment immediately after the front matter and include a## Table of Contentssection. This file is missing both, somake docs-verifywill fail until they’re added.--- last_updated: 2025-09-27 --- + +<!-- SPDX-License-Identifier: LicenseRef-MIND-UCAL-1.0 --> +<!-- © 2025 J. Kirby Ross / Neuroglyph Collective --> # Review Worksheet Flow + +## Table of Contents + +- [1. Generation](#1-generation) +- [2. Editing the worksheet](#2-editing-the-worksheet) +- [3. Decision markers](#3-decision-markers) +- [4. What goes where](#4-what-goes-where) +- [5. Merging & archiving](#5-merging--archiving) +- [6. Frequently asked questions](#6-frequently-asked-questions) +- [7. Quick checklist before pushing](#7-quick-checklist-before-pushing)As per coding guidelines
core/src/hooks/augment.c (1)
71-78: Still need an overflow guard while parsingHEAD~N.Large offsets (e.g.
HEAD~999999999999) will wrapsize_t valueand make us walk the wrong commit. Add the standard overflow pre-check before multiplying/adding (and include<limits.h>forSIZE_MAX).- while (*cursor != '\0') { + while (*cursor != '\0') { if (!isdigit((unsigned char)*cursor)) { return false; } - value = (value * 10U) + (size_t)(*cursor - '0'); + size_t digit = (size_t)(*cursor - '0'); + if (value > (SIZE_MAX - digit) / 10U) { + return false; + } + value = (value * 10U) + digit; cursor++; }core/src/journal/writer.c (2)
158-166: Preserve the adapter’s commit-creation error code.We still flatten every failure from
gm_git_repository_port_commit_createintoGM_ERR_UNKNOWN, which hides actionable diagnostics from callers and frustrates retries. Bubble the adapter’s code instead (same pattern as the parent-walk fix).gm_result_void_t commit_result = gm_git_repository_port_commit_create(jctx->repo_port, &spec, commit_oid); free(message); if (!commit_result.ok) { - if (commit_result.u.err != NULL) { - gm_error_free(commit_result.u.err); - } - return GM_ERR_UNKNOWN; + int commit_code = GM_ERR_UNKNOWN; + if (commit_result.u.err != NULL) { + commit_code = commit_result.u.err->code; + gm_error_free(commit_result.u.err); + } + return commit_code; }
205-218: Keep the journal ref append-only; don’t force-update or mask the error.Setting
.force = trueturns a non-fast-forward rejection into a history rewrite, breaking the append-only guarantee we just introduced. At the same time we squash the adapter’s precise failure code intoGM_ERR_UNKNOWN, hiding whether the rejection was a genuine FF conflict. Let the update fail fast-forward-only and propagate the real code.gm_git_reference_update_spec_t spec = { .ref_name = jctx->ref_name, .target_oid = &commit_oid, .log_message = "Journal append", - .force = true, + .force = false, }; gm_result_void_t update_result = gm_git_repository_port_reference_update( jctx->repo_port, &spec); if (!update_result.ok) { - if (update_result.u.err != NULL) { - gm_error_free(update_result.u.err); - } - return GM_ERR_UNKNOWN; + int update_code = GM_ERR_UNKNOWN; + if (update_result.u.err != NULL) { + update_code = update_result.u.err->code; + gm_error_free(update_result.u.err); + } + return update_code; }core/src/app/cache/cache_rebuild_service.c (1)
433-438: Force-updating the cache ref still rewrites history.This path must stay append-only; otherwise a transient race will overwrite previous cache commits. Drop
.force = trueso the update obeys fast-forward semantics and callers can surface/handle the conflict.gm_git_reference_update_spec_t update_spec = { .ref_name = ref_name, .target_oid = commit_oid, .log_message = "Cache rebuild", - .force = true, + .force = false, };core/tests/fakes/git/fake_git_repository_port.c (2)
753-770: Propagategm_strcpy_safeerrors infake_reference_update.Lines 754 and 761 ignore the status from
gm_strcpy_safe, so truncated ref/log strings pass as success. We must detect truncation and surface an error to keep the fake deterministic.@@ - if (spec->ref_name != NULL) { - (void)gm_strcpy_safe(fake->last_update_ref, sizeof(fake->last_update_ref), - spec->ref_name); - } else { + if (spec->ref_name != NULL) { + int ref_rc = gm_strcpy_safe(fake->last_update_ref, + sizeof(fake->last_update_ref), + spec->ref_name); + if (ref_rc != GM_OK) { + fake->last_update_ref[0] = '\0'; + return gm_err_void( + GM_ERROR(GM_ERR_BUFFER_TOO_SMALL, + "fake update ref too long")); + } + } else { fake->last_update_ref[0] = '\0'; } - if (spec->log_message != NULL) { - (void)gm_strcpy_safe(fake->last_update_log, - sizeof(fake->last_update_log), spec->log_message); - } else { + if (spec->log_message != NULL) { + int log_rc = gm_strcpy_safe(fake->last_update_log, + sizeof(fake->last_update_log), + spec->log_message); + if (log_rc != GM_OK) { + fake->last_update_log[0] = '\0'; + return gm_err_void( + GM_ERROR(GM_ERR_BUFFER_TOO_SMALL, + "fake update log too long")); + } + } else { fake->last_update_log[0] = '\0'; }As per coding guidelines
716-733: Enforce safe-string contract infake_commit_create.Line 717 calls
gm_strcpy_safebut discards its status, so truncated commit messages slip through silently, violating our safe string policy. Please check the return value and bail out when the buffer is too small.@@ - if (spec->message != NULL) { - (void)gm_strcpy_safe(fake->last_commit_message, - sizeof(fake->last_commit_message), spec->message); - } else { + if (spec->message != NULL) { + int msg_rc = gm_strcpy_safe(fake->last_commit_message, + sizeof(fake->last_commit_message), + spec->message); + if (msg_rc != GM_OK) { + fake->last_commit_message[0] = '\0'; + return gm_err_void( + GM_ERROR(GM_ERR_BUFFER_TOO_SMALL, + "fake commit message too long")); + } + } else { fake->last_commit_message[0] = '\0'; }As per coding guidelines
core/include/gitmind/ports/git_repository_port.h (1)
71-79: Ownership and disposal for heap outputs — resolved
blob_disposeandcommit_message_disposeplus the NULL-safe helpers correctly define ownership across adapters. LGTM.Also applies to: 184-205
| if (ctx.git_repo_port_dispose != NULL) { | ||
| ctx.git_repo_port_dispose(&ctx.git_repo_port); | ||
| } | ||
| git_repository_free(repo); |
There was a problem hiding this comment.
Don't free the shared repo inside run_stampede.
run_stampede is invoked three times from main with the same git_repository *repo. Freeing it here invalidates the pointer after the first run, so the second iteration dereferences freed memory and the third double-frees it. Move the cleanup to the caller and free the repository once after the loop completes.
- if (ctx.git_repo_port_dispose != NULL) {
- ctx.git_repo_port_dispose(&ctx.git_repo_port);
- }
- git_repository_free(repo);
+ if (ctx.git_repo_port_dispose != NULL) {
+ ctx.git_repo_port_dispose(&ctx.git_repo_port);
+ }Then call git_repository_free(repo); once in main after the stampede loop.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (ctx.git_repo_port_dispose != NULL) { | |
| ctx.git_repo_port_dispose(&ctx.git_repo_port); | |
| } | |
| git_repository_free(repo); | |
| if (ctx.git_repo_port_dispose != NULL) { | |
| ctx.git_repo_port_dispose(&ctx.git_repo_port); | |
| } |
🤖 Prompt for AI Agents
In benchmarks/wildebeest_stampede.c around lines 154-157, run_stampede currently
calls git_repository_free(repo) which frees a shared git_repository used across
three invocations; remove the git_repository_free(repo) call from run_stampede
(leave any per-run ctx.git_repo_port_dispose calls if they are per-run) and
instead add a single git_repository_free(repo) in main after the loop that
invokes run_stampede three times so the repository is freed exactly once when
all runs complete.
| if (strcmp(segment, "..") == 0) { | ||
| if (*count > 0) { | ||
| (*count)--; | ||
| return gm_ok_void(); | ||
| } | ||
| if (*cursor == '\0') { | ||
| break; | ||
| if (!is_absolute) { | ||
| return push_segment(segments, count, max_segments, segment); | ||
| } | ||
| return gm_ok_void(); | ||
| } |
There was a problem hiding this comment.
Preserve chained .. segments in relative paths.
The current logic collapses "../.." (and longer chains) down to "." because the second .. pops the first one from the stack. That breaks relative path semantics (e.g., joining into a temp dir now yields the wrong location). Only consume a .. when there’s a real segment to cancel. Otherwise, retain the new .. for relative paths.
- if (strcmp(segment, "..") == 0) {
- if (*count > 0) {
- (*count)--;
- return gm_ok_void();
- }
- if (!is_absolute) {
- return push_segment(segments, count, max_segments, segment);
- }
- return gm_ok_void();
- }
+ if (strcmp(segment, "..") == 0) {
+ if (*count > 0 && strcmp(segments[*count - 1], "..") != 0) {
+ (*count)--;
+ return gm_ok_void();
+ }
+ if (!is_absolute) {
+ return push_segment(segments, count, max_segments, segment);
+ }
+ return gm_ok_void();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (strcmp(segment, "..") == 0) { | |
| if (*count > 0) { | |
| (*count)--; | |
| return gm_ok_void(); | |
| } | |
| if (*cursor == '\0') { | |
| break; | |
| if (!is_absolute) { | |
| return push_segment(segments, count, max_segments, segment); | |
| } | |
| return gm_ok_void(); | |
| } | |
| if (strcmp(segment, "..") == 0) { | |
| if (*count > 0 && strcmp(segments[*count - 1], "..") != 0) { | |
| (*count)--; | |
| return gm_ok_void(); | |
| } | |
| if (!is_absolute) { | |
| return push_segment(segments, count, max_segments, segment); | |
| } | |
| return gm_ok_void(); | |
| } |
This commit primarily refines **reference handling** and **memory management** in the Git repository port and related services, particularly focusing on safe reference updates and resource cleanup.
* **Non-Fast-Forward Reference Update Prevention:**
* In `core/src/adapters/git/libgit2_repository_port.c`, the `reference_update_impl` function is completely refactored to explicitly prevent **non-fast-forward updates** when `spec->force` is `false`.
* It now performs a lookup of the existing reference (`git_reference_lookup`), checks if the new target is a **descendant** of the current target using `git_graph_descendant_of`, and rejects the update with `GM_ERR_ALREADY_EXISTS` if it's not a fast-forward change.
* If no reference exists (`GIT_ENOTFOUND`), it attempts to create it with `force = false`. If it exists and is a fast-forward, it updates the target using `git_reference_set_target`.
* The `force` parameter in the **cache rebuild service** (`cache_update_ref`) and the **journal writer** (`flush_journal_batch`) is changed from `true` to **`false`** to leverage this new safety mechanism.
* **Optimistic Locking in Journal Writer:**
* The `flush_journal_batch` function in `core/src/journal/writer.c` is wrapped in a **two-attempt loop** to handle potential race conditions (optimistic locking).
* If the reference update fails with `GM_ERR_ALREADY_EXISTS` (non-fast-forward), it indicates another process updated the journal's branch. The loop continues to the second attempt, effectively forcing a re-read of the parents for the next commit creation, before failing the write.
* The creation of the journal commit (`create_journal_commit`) now correctly captures and returns the underlying error code from `gm_git_repository_port_commit_create`.
* **Cache Rebuild Parent Commit Handling:**
* The cache rebuild service (`cache_build_commit_and_update` and `cache_create_commit` in `core/src/app/cache/cache_rebuild_service.c`) is updated to **read the current cache ref tip** to use as the **parent commit** for the new cache commit. This correctly builds a commit history for the cache branch.
* **Cleanup and Defensive Programming:**
* The **fallback cleanup** logic is removed from `gm_git_repository_port_commit_message_dispose` in `core/include/gitmind/ports/git_repository_port.h` because the port implementation is responsible for disposing of the message, not the generic inline function.
* Added the `_GNU_SOURCE` definition check in `core/src/adapters/fs/posix_temp_adapter.c`.
* Introduced a helper function `dispose_repo_port` in `core/src/hooks/post-commit.c` to deduplicate and simplify port cleanup calls.
* Added stricter size checks and defensive `gm_memset_safe` calls in the path utility functions (`gm_fs_path_normalize_logical`, `gm_fs_path_dirname`) and the fake Git repository port.
* Used `gm_strcpy_safe` instead of `memcpy` for commit message copying in `fake_git_repository_port.c`.
…o chore/more-fixes
1 participant
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests
Chores