Skip to content

feat(config): ignore_worktrees rework for DeusData#1430 - #1

Merged
vitalNohj merged 11 commits into
mainfrom
cloud/cbm-worktree-ignore-rework-6ddc
Sep 30, 2026
Merged

vitalNohj merged 11 commits into
mainfrom
cloud/cbm-worktree-ignore-rework-6ddc

Conversation

@vitalNohj

@vitalNohj vitalNohj commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Lands ignore_worktrees on this fork's main.

Rebased onto current upstream/main so this pull request is only the feature. When the key is enabled, a linked git worktree is skipped by auto-index and refused by an explicit index_repository (pass index_worktree=true, or turn the key off). The default stays off, so existing checkouts are unchanged.

Worktree detection uses cbm_path_info_utf8 rather than stat(), matching the Unicode repository-path fix already on main.

The upstream contribution is still DeusData#1430 and is not changed by this pull request.

Signed-off-by: Nohj [email protected]


Summary

Adds an optional ignore_worktrees setting so short-lived linked Git worktrees do not each get their own stored project index. It is off by default, so existing indexing behavior stays the same.

When enabled, automatic indexing skips linked worktrees, including jobs waiting to retry. A manual index_repository request is refused unless index_worktree=true is passed. Session guidance explains that an unindexed worktree was skipped on purpose instead of suggesting an indexing request that would be refused. Main checkouts, ordinary clones, and submodules are unaffected.

Files touched and purpose

  • README.md and docs/CONFIGURATION.md: Explain the setting and the one-call override.
  • src/cli, src/daemon, src/git, and src/mcp: Add the setting, identify linked worktrees, and apply the behavior to indexing and session guidance.
  • tests: Cover the default behavior, skipped indexing, the manual override, and guidance for nested or linked paths.

@cursor
cursor Bot force-pushed the cloud/cbm-worktree-ignore-rework-6ddc branch from 6bb8ad6 to 0d2f840 Compare September 23, 2026 06:20
@vitalNohj
vitalNohj marked this pull request as ready for review September 23, 2026 06:46
@nohjbot

nohjbot Bot commented Sep 23, 2026

Copy link
Copy Markdown

nohjbot

In response to this PR's description.

<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
CI fix for upstream [DeusData#1430](https://github.com/DeusData/codebase-memory-mcp/pull/1430\).

**Do not merge this fork PR.** Delivery is `fm/cbm-worktree-ignore-1` @ `0d2f840c`.

## This revision

  • Removed `Co-authored-by` so `%(trailers:key=Signed-off-by,valueonly)` returns `Nohj <[email protected]>` matching the author. Local `scripts/check-dco.sh upstream/main..HEAD` is OK.
  • clang-format-20 on `src/cli/cli.c`, `src/cli/cli.h`, `src
    …

Review failed. I could not obtain all the changes needed for a complete review.

What would satisfy approval

Not approved: a valid review has not completed. No new code defect was established by this incomplete attempt.

Have the workflow operator restore complete access to the PR diff and commit history. If it exceeds supported API limits, split the PR into reviewable changes.

Approval needs a complete review of the current head with no concrete code bugs remaining, followed by a successful check and confirmed GitHub approval. Tests and additional proof are not an approval requirement. This request is not queued for an automatic retry. Once the cause is addressed, post a new @nohjbot review with any relevant context.

Comment thread internal/cbm/extract_dbt.c Resolved
Comment thread src/cli/client_adapter.c Resolved
Comment thread src/cypher/cypher.c Resolved
Comment thread src/pipeline/pipeline.c Resolved

@kody-ai kody-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found critical issues please review the requested changes

Comment thread internal/cbm/cbm.c Resolved
Comment thread internal/cbm/cbm.c Resolved
Comment thread internal/cbm/extract_dbt.c Resolved

@kody-ai kody-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitalNohj

Copy link
Copy Markdown
Owner Author

@nohjbot review

@nohjbot

nohjbot Bot commented Sep 25, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review failed. I could not obtain all the changes needed for a complete review.

What would satisfy approval

Not approved: a valid review has not completed. No new code defect was established by this incomplete attempt.

Have the workflow operator restore complete access to the PR diff and commit history. If it exceeds supported API limits, split the PR into reviewable changes.

Approval needs a complete review of the current head with no concrete code bugs remaining, followed by a successful check and confirmed GitHub approval. Tests and additional proof are not an approval requirement. This request is not queued for an automatic retry. Once the cause is addressed, post a new @nohjbot review with any relevant context.

Agents that use `git worktree add` to run parallel branches end up with one
indexed project per worktree. Each is a near-duplicate of the main checkout,
so the cache fills with redundant graphs and project pickers get noisy.

Add an opt-in `ignore_worktrees` config key. When enabled, a linked worktree
is left alone:

  - auto-index on connect skips it (logged as autoindex.skip)
  - the daemon's background auto-index skips it
  - explicit index_repository refuses with an actionable message naming both
    ways forward (index_worktree=true, or turn the key off)
  - the hook-augment "not indexed" guidance stops telling the agent to run
    index_repository in a directory where it would only be refused

Detection is git plumbing, no subprocess: <path>/.git must be a regular file
holding a "gitdir:" pointer AND that gitdir must contain a `commondir` file.
The commondir check is what separates a linked worktree from a submodule,
whose .git is also a gitlink file but whose gitdir has no commondir. The main
checkout of a repo that has worktrees is never affected.

Defaults to false, so behaviour is unchanged unless the key is set.

Signed-off-by: Nohj <[email protected]>
Upstream's Unicode path fix dropped stat() from this file because it follows
symlinks and rejects non-ASCII paths on Windows. Keep ignore_worktrees
detection on cbm_path_info_utf8 so the rebase does not restore that probe.

Signed-off-by: Nohj <[email protected]>
@vitalNohj
vitalNohj force-pushed the cloud/cbm-worktree-ignore-rework-6ddc branch from 0d2f840 to d63d0e1 Compare September 29, 2026 03:24
@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

@vitalNohj

Copy link
Copy Markdown
Owner Author

@nohjbot review

@nohjbot

nohjbot Bot commented Sep 29, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review

Code review passed at d63d0e16. R1 and R2 reviewed independently; R3 challenged their findings and made the final decision. GitHub approval is a separate verified publishing step.

Reasoned review decisions

R1 - correctness and execution flow (claude-opus-5-5)

I traced the new cbm_git_is_linked_worktree predicate and each call site, and found no concrete code bug. Detection reads &lt;path&gt;/.git as a regular file, parses gitdir: (resolving a relative pointer against path), and requires &lt;gitdir&gt;/commondir. That correctly separates linked worktrees from main checkouts, plain directories and submodules, and bounded snprintf checks guard every buffer. In handle_index_repository the refusal runs after the path is resolved, canonicalized and workspace-authorized, but before cross-repo mode, the daemon index_executor hand-off and supervised or in-process execution. So an explicit call cannot bypass it on any executor path, and index_worktree=true or an unset key falls through unchanged. The standalone maybe_auto_index skip and the daemon application_background_initialize_impl skip both set the auto-index decision to false before the file-count guard and job subscription. In the hook, worktree_ignored is computed before the owned server is freed and only changes the guidance text. Non-blocking observations: detection is root-only, so a session root or repo_path that is a subdirectory of a worktree is not treated as one. The daemon and hook paths read the key from the session MCP server's config (session-&gt;mcp, or a fresh cbm_mcp_server_new(NULL) in the CLI hook), while the daemon reads auto_index from application-&gt;config. I could not see whether those servers get a config attached; if they do not, the key reads as its default (off) there. That is fail-open, not a crash.

R2 - independent regression and risk (gpt-6-sol)

The default-off setting, explicit-index refusal and override, and both auto-index gates follow the intended paths. I found one inconsistency in lifecycle guidance: the hook checks the supplied working directory for a .git file, so it misses linked worktrees when the agent runs from a subdirectory. No tests were run.

R3 - challenge and final decision (claude-opus-5-5)

I found no concrete code defect in this PR. cbm_git_is_linked_worktree correctly separates linked worktrees from main checkouts, plain directories and submodules: it requires a regular .git gitlink file whose gitdir contains commondir, and every buffer write is bounded. The explicit index_repository refusal runs after path resolution, canonicalization and the workspace boundary check. It also runs before the cross-repo, daemon-executor, supervised and in-process paths, so no execution route bypasses it; index_worktree=true and the default-off key both fall through unchanged. The standalone maybe_auto_index gate and the daemon background-initialize gate both clear the auto-index decision before file counting and job subscription. The hook change only alters guidance text and computes its flag before the owned server is freed. The single candidate describes a limitation of the root-only detection, not a regression or a failing path. It does not block. The Kody comments concern files outside this diff and are not findings under this review.

Candidate adjudication

  • R2-C1: Recognize linked worktrees when lifecycle cwd is a subdirectory - Non-blocking. The detection is root-only by design, and the docs describe it as applying to a session whose root is a linked worktree. When the hook's cwd is a subdirectory of a worktree, the guidance falls back to the pre-existing text; no behavior regresses. The claimed failure is also not established. An agent following that text from a subdirectory would pass the subdirectory as repo_path, and the predicate checks &lt;path&gt;/.git there too, so the explicit call would not be refused. The only effect is that an informational hint is less specific than it could be. That is a possible enhancement (resolving the containing worktree root), not a code bug.

Context limits: diff, bounded past discussion, changed-file source, and heuristic related context. 3 stage-requested source window(s), 0 unavailable/omitted, 2 truncated. R3 received 17 source entries; 23 omitted entries are recorded in the execution. At most one round of six read-only source requests per stage at pinned head/base. This is not exhaustive repository/caller coverage. No code or tests were run. Tests, logs and proof packages are not approval requirements.

Approval status

No code changes are requested by this review. GitHub approval is submitted separately after the review check succeeds and the PR snapshot is reverified. If approval is missing, the workflow operator should check the check/approval publishing steps and restore any failed API access, then request @nohjbot review on the current head. A clean comment alone is not a recorded approval.


Automatic review of new commits is off. Request another review with @nohjbot review, @nohjbot thoughts, or @nohjbot thoughts?. Add context on the same line or below, or start a review request with @nohjbot followed by your question.

@nohjbot nohjbot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No open findings in commit d63d0e16d18cf46836f0d019a03726bb6a577d16.

Full nohjbot review. This approval covers that reviewed commit only.

Comment thread src/daemon/application.c Resolved
Comment thread src/daemon/application.c Resolved
A session queued while capacity was full could still be indexed after the
setting was turned on. Retry admission now applies the same linked-worktree
gate as the initial path.

Signed-off-by: Nohj <[email protected]>
@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

@vitalNohj

Copy link
Copy Markdown
Owner Author

@nohjbot review

@nohjbot

nohjbot Bot commented Sep 29, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review

Code review passed at 3bb85c8c. R1 and R2 reviewed independently; R3 challenged their findings and made the final decision. GitHub approval is a separate verified publishing step.

Reasoned review decisions

R1 - correctness and execution flow (claude-opus-5-5)

I traced the one changed hunk in application_auto_index_retry_pending_locked and found no concrete defect. The new gate uses the same predicate pair as initial admission in application_background_initialize_impl: cbm_mcp_ignore_worktrees_enabled(session-&gt;mcp) and cbm_git_is_linked_worktree(root_path). It runs after the workspace-authorization and existing-DB checks and before args are built and application_job_subscribe_locked is called. So when the setting is enabled, a session queued for a capacity slot can no longer be subscribed to an index job once application_job_publish frees a slot.

On skip, the new path clears auto_index_retry_pending and calls application_refresh_watch_locked, exactly like the neighbouring skip branches. It then continues, so the loop moves on to other pending sessions rather than stopping. Because auto_index_evaluated stays true and auto_index_retry_pending is now false, a later application_background_initialize_impl call will not re-attempt auto-index for the session. That matches initial admission, which also marks the session evaluated and never subscribes a linked worktree.

The filesystem probe runs while application-&gt;mutex is held. This follows existing precedent: application_regular_db_exists and application_session_workspace_allowed are already called under the same lock in this loop, so it adds no new locking pattern. The log call matches the key/value shape used at initial admission.

One non-blocking observation: the retry runs on the job thread and reads session-&gt;mcp. The existing retry code already calls cbm_mcp_server_session_project and cbm_mcp_server_session_root on that server from this thread, so the new read does not change the threading pattern. I cannot see the thread-safety of the config read inside cbm_mcp_ignore_worktrees_enabled, and I found no failing path.

The earlier kody findings in unchanged files (extract_dbt.c, client_adapter.c, cypher.c, pipeline.c, cbm.c) are outside this incremental diff and are not prior findings of this review.

R2 - independent regression and risk (gpt-6-sol)

The retry path now rechecks the same linked-worktree setting used at initial admission. When it is enabled for a queued linked worktree, the code clears the pending flag and continues before constructing index arguments or subscribing a job. I found no concrete bug introduced by this change. No tests were run.

R3 - challenge and final decision (claude-opus-5-5)

There are no reviewer candidates and no prior open findings, and I found no concrete defect in the one changed hunk. The new gate in application_auto_index_retry_pending_locked is the same predicate pair used at initial admission in application_background_initialize_impl: cbm_mcp_ignore_worktrees_enabled(session-&gt;mcp) &amp;&amp; cbm_git_is_linked_worktree(root_path). It sits after the workspace-authorization and existing-DB checks and before application_auto_index_args and application_job_subscribe_locked. A queued linked worktree therefore can no longer be subscribed when application_job_publish frees a slot while the setting is enabled.

On skip, the branch clears auto_index_retry_pending, refreshes the watch and continues, exactly like the neighbouring skip branches. It does not break, so other pending sessions are still admitted. With auto_index_evaluated already true and the pending flag cleared, a later background-initialize will not re-attempt the session. That matches initial admission, which also marks the session evaluated and never subscribes it.

The filesystem probe runs under application-&gt;mutex, but this loop already does the same with application_regular_db_exists and application_session_workspace_allowed, so the change adds no new locking pattern. Reading session-&gt;mcp from the job thread matches the existing cbm_mcp_server_session_project/root calls in the same loop. The earlier kody findings in extract_dbt.c, client_adapter.c, cypher.c, pipeline.c and cbm.c are outside this incremental diff and are not prior findings of this review. No tests were run. The change is correct as written.

Candidate adjudication

Neither independent review proposed a new concrete bug. R3 completed the final code-based judgment and prior-finding reconciliation.

Context limits: diff, bounded past discussion, changed-file source, and heuristic related context. 0 stage-requested source window(s), 0 unavailable/omitted, 0 truncated. R3 received 4 source entries; 10 omitted entries are recorded in the execution. At most one round of six read-only source requests per stage at pinned head/base. This is not exhaustive repository/caller coverage. No code or tests were run. Tests, logs and proof packages are not approval requirements.

Approval status

No code changes are requested by this review. GitHub approval is submitted separately after the review check succeeds and the PR snapshot is reverified. If approval is missing, the workflow operator should check the check/approval publishing steps and restore any failed API access, then request @nohjbot review on the current head. A clean comment alone is not a recorded approval.


Automatic review of new commits is off. Request another review with @nohjbot review, @nohjbot thoughts, or @nohjbot thoughts?. Add context on the same line or below, or start a review request with @nohjbot followed by your question.

@nohjbot nohjbot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No open findings in commit 3bb85c8c7c3c005fc59bed0a8ee5c467d903a761.

Full nohjbot review. This approval covers that reviewed commit only.

Comment thread src/cli/hook_augment.c Resolved
Comment thread src/git/git_context.c Resolved
@nohjbot

nohjbot Bot commented Sep 29, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review

Changes needed at 51a18c7b. R1 and R2 reviewed independently; R3 challenged their findings and made the final decision. GitHub approval is a separate verified publishing step.

Reasoned review decisions

R1 - correctness and execution flow (claude-opus-5-5)

I traced the one behavioral change. ha_lifecycle_json_from_root now keeps a resolved project only when cbm_git_same_linked_worktree(cwd, project_root) is true. That helper walks each path to its linked-worktree anchor with walk_linked_worktree_root, then compares the gitdir pointers read from each anchor's .git file.

Traced cases:

  • **Unindexed child worktree nested in an indexed worktree A.** The anchors are B and A. Their gitdirs (.git/worktrees/&lt;B&gt; and .git/worktrees/&lt;A&gt;) differ, so A's graph is dropped and worktree_ignored becomes true.
  • **Exact index of the cwd's own worktree, from a subdirectory.** Both walks reach the same anchor and the gitdirs are equal, so the project is kept.
  • **Main-checkout ancestor.** walk_linked_worktree_root returns false at a .git directory, so the match is dropped, as before.
  • **Key off, or cwd not in a linked worktree.** ignore_linked is false and resolution is unchanged.

project_root is always filled when project is non-NULL: both the derived-name walk and the registry scan canonicalize it or return NULL. All buffers are GIT_OUTPUT_MAX and checked. The new test matrices in test_cli.c and test_git_context.c exercise the child, parent and main cases.

Non-blocking observation: the gitdir strings are compared textually. With git's worktree.useRelativePaths, read_gitlink_target joins the relative pointer onto each walked root. The cwd is not canonicalized but project_root is. If the cwd passes through a symlink, the same worktree could compare unequal, and an exact index would be dropped. That needs a non-default git setting plus a symlinked cwd, and I did not establish it as a concrete failing path.

R2 - independent regression and risk (gpt-6-sol)

The new guard compares linked-worktree identities before retaining a lifecycle hook’s indexed-project match, so a nested child worktree no longer inherits its indexed parent’s graph. I found one remaining compatibility case: the identity comparison uses unnormalized gitdir strings, which can reject an index belonging to the same worktree when the cwd uses a symlink and its gitlink contains a relative pointer. No tests were run.

R3 - challenge and final decision (claude-opus-5-5)

F1 is resolved. The lifecycle guard now keeps a resolved project only when cbm_git_same_linked_worktree(cwd, project_root) finds that both paths belong to the same linked-worktree anchor, judged by that anchor's gitdir pointer. An unindexed child worktree nested inside an indexed worktree therefore no longer inherits the parent graph. An exact index of the cwd's own worktree, and a main-checkout ancestor, still behave correctly.

R2-C1 is confirmed as a narrow bug introduced by this commit. The new identity check compares raw gitdir strings. For a relative gitdir: pointer, read_gitlink_target builds each string by joining the pointer onto the walked anchor path. The hook's cwd is taken from the payload and is only slash-normalized, while project_root is canonicalized. The failure needs a relative gitdir pointer, written by git 2.48+ under worktree.useRelativePaths or --relative-paths, plus a cwd reached through a symlink (for example macOS /tmp vs /private/tmp). Then the same worktree yields two different strings. The hook drops the worktree's own index and tells the agent it is deliberately unindexed and not to run index_repository. The previous guard kept this case. The fix is small: normalize the two gitdir targets before comparing them.

Candidate adjudication

  • R2-C1: Relative gitdir pointers make a symlinked cwd lose its own worktree index - Confirmed as F2. read_gitlink_target resolves a relative pointer by textually prefixing the anchor path, with no canonicalization. cbm_git_same_linked_worktree walks from the raw cwd and from the canonical project_root, then compares the joined gitdir strings with strcmp. With a relative gitlink, git 2.48+ worktree.useRelativePaths, and a cwd that passes through a symlink, the two anchors differ only by alias. The strings then differ, so the function returns false for the same worktree. The hook frees the valid project and emits the ignored-worktree guidance. This path is new in this commit; the prior boolean guard kept the project.

Confirmed code bugs

F2 - P2 - Relative gitdir pointers make a symlinked cwd lose its own worktree index

src/git/git_context.c:276

Failing path: A linked worktree has a relative gitdir: pointer and is indexed at its real path. A session starts from a symlink to that worktree. Project resolution can find the index and supply its canonical root, but the new comparison reads the same relative pointer once through the symlink path and once through the real path. The resulting strings differ, so the hook discards the worktree’s own index and says it was deliberately left unindexed.

Previous findings

  • F1 - Resolved: The guard no longer accepts any linked-worktree root. It calls cbm_git_same_linked_worktree, which walks each path to its own linked-worktree anchor and compares their gitdir pointers. A child worktree nested in an indexed worktree reaches a different anchor, and its gitdir (.git/worktrees/<child>) differs from the parent's. The parent's project is therefore freed and worktree_ignored is set. This is the anchor comparison F1 required.

Context limits: diff, bounded past discussion, changed-file source, and heuristic related context. 0 stage-requested source window(s), 0 unavailable/omitted, 0 truncated. R3 received 7 source entries; 22 omitted entries are recorded in the execution. At most one round of six read-only source requests per stage at pinned head/base. This is not exhaustive repository/caller coverage. No code or tests were run. Tests, logs and proof packages are not approval requirements.

What would satisfy approval

Not approved: 1 unresolved finding remains. Each describes a code-supported bug. Code inspection of the current head can resolve it; no separate proof package is required.

  • F2: Relative gitdir pointers make a symlinked cwd lose its own worktree index (src/git/git_context.c:276)

    • Code change needed: In cbm_git_same_linked_worktree, canonicalize git_a and git_b before strcmp (realpath on POSIX, _fullpath on Windows, falling back to the raw string if resolution fails). Alternatively, canonicalize both input paths before walking.

Push the code fix, then comment @nohjbot review; explanations and relevant caller paths are welcome. The reviewer will inspect the code and execution flow again. Tests, logs, and additional proof are not an approval requirement. Suggested tests and uncertain observations are non-blocking. Approval follows when no concrete code bug remains and the review/check publication succeeds for that head. Other repository checks still apply.


Automatic review of new commits is off. Request another review with @nohjbot review, @nohjbot thoughts, or @nohjbot thoughts?. Add context on the same line or below, or start a review request with @nohjbot followed by your question.

A 64-level walk missed a deep session path, and a relative gitdir joined
onto a symlink spelling no longer matched the real project root. Walk to
the filesystem root and compare canonical gitdirs. Drop the unused project
resolver that failed the macOS -Werror build.

Signed-off-by: Nohj <[email protected]>
@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

@vitalNohj

Copy link
Copy Markdown
Owner Author

@nohjbot review

@nohjbot

nohjbot Bot commented Sep 29, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review

Code review passed at 72f1a52d. R1 and R2 reviewed independently; R3 challenged their findings and made the final decision. GitHub approval is a separate verified publishing step.

Reasoned review decisions

R1 - correctness and execution flow (claude-opus-5-5)

This commit makes three changes. It removes the unused ha_resolve_indexed_project wrapper, lets walk_linked_worktree_root run to the filesystem root instead of stopping at 64 levels, and canonicalizes both gitdirs before cbm_git_same_linked_worktree compares them.

  • **Removed wrapper:** nothing else refers to it. ha_resolve_and_query and ha_resolve_coverage both call ha_resolve_indexed_project_with_root directly, and the wrapper was static, so no other file could use it.
  • **Unbounded walk:** the loop always ends. Each parent_directory call either shortens current by at least one component or returns false. It returns false at the POSIX root, at a Windows drive root, and when a relative path has no separator. The strcmp(previous, current) check is a second guard. Copying current into previous cannot truncate, because current is limited to GIT_OUTPUT_MAX-1 bytes. The anchor rules are unchanged: the walk still stops at a main-checkout .git directory, a non-regular .git, or a submodule gitlink. A deep cwd or repo_path inside a linked worktree now reaches its anchor, so ignore_worktrees applies to it.
  • **Canonical compare:** both joined gitdir strings go through cbm_canonical_path, with the raw string as a fallback, before strcmp. A symlinked cwd whose gitlink holds a relative pointer now resolves to the same gitdir as the canonical project_root.

I found no new defect. Two non-blocking notes:

  • A path with no git anchor above it now costs one path probe per ancestor up to the root, not at most 64.
  • If only one side canonicalizes, the raw fallback can compare unequal. That needs a missing gitdir, and a live linked worktree always has one.

R2 - independent regression and risk (gpt-6-sol)

The ancestor walk now stops at a Git anchor or the filesystem root rather than at 64 components, so the depth limit no longer bypasses the worktree check. The identity comparison now canonicalizes both gitdirs before comparing them, addressing the symlinked-cwd case in F2. Removing the unused resolver does not change the visible call path. I found no new code-supported bug in this increment; no tests were run.

R3 - challenge and final decision (claude-opus-5-5)

Neither independent review raised a candidate, and I found no new defect caused by this increment. The commit makes three changes:

  • **Removed wrapper:** it deletes the unused static ha_resolve_indexed_project wrapper. Both remaining callers use ha_resolve_indexed_project_with_root directly.
  • **Uncapped walk:** it removes the 64-level cap in walk_linked_worktree_root. The loop still terminates. parent_directory either removes a component or returns false at the POSIX root, at a Windows drive root, or when no separator remains. The strcmp(previous, current) check is a second guard. The anchor rules are unchanged, so a deep cwd or repo_path inside a linked worktree now reaches its gitlink and ignore_worktrees applies to it.
  • **Canonical compare:** it canonicalizes both joined gitdir paths with cbm_canonical_path before the strcmp in cbm_git_same_linked_worktree. Its header contract says it resolves links on an existing path, with a 4096-byte output that matches GIT_OUTPUT_MAX.

The raw-string fallback is effectively unreachable for a detected linked worktree. linked_worktree_anchor returns 1 only after finding &lt;gitdir&gt;/commondir as a regular file, so the gitdir exists and canonicalization succeeds.

F2 is resolved: a relative gitdir joined onto a symlinked cwd root and onto the canonical project root now resolves to the same real path. The one remaining cost, extra stat probes on ancestors up to the filesystem root when no git anchor exists, is not a defect.

Candidate adjudication

Neither independent review proposed a new concrete bug. R3 completed the final code-based judgment and prior-finding reconciliation.

Previous findings

  • F2 - Resolved: The comparison no longer uses the raw joined strings. git_a and git_b are canonicalized with cbm_canonical_path before strcmp. Per the header, that function resolves links on an existing path, and after resolving the symlinked root, realpath also resolves the .. in a relative gitdir. A relative gitdir: pointer joined onto a symlink spelling of the worktree root therefore canonicalizes to the same real gitdir as the pointer joined onto the canonical project_root, so the worktree's own index is kept. The raw fallback applies only when canonicalization fails. A linked-worktree anchor is accepted only after &lt;gitdir&gt;/commondir is confirmed as a regular file, so the gitdir exists and canonicalization does not fail. This matches F2's approval criteria.

Context limits: diff, bounded past discussion, changed-file source, and heuristic related context. 2 stage-requested source window(s), 0 unavailable/omitted, 1 truncated. R3 received 6 source entries; 16 omitted entries are recorded in the execution. At most one round of six read-only source requests per stage at pinned head/base. This is not exhaustive repository/caller coverage. No code or tests were run. Tests, logs and proof packages are not approval requirements.

Approval status

No code changes are requested by this review. GitHub approval is submitted separately after the review check succeeds and the PR snapshot is reverified. If approval is missing, the workflow operator should check the check/approval publishing steps and restore any failed API access, then request @nohjbot review on the current head. A clean comment alone is not a recorded approval.


Automatic review of new commits is off. Request another review with @nohjbot review, @nohjbot thoughts, or @nohjbot thoughts?. Add context on the same line or below, or start a review request with @nohjbot followed by your question.

@nohjbot nohjbot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No open findings in commit 72f1a52dc6bb898166a2998b335c0682110a5c31.

Full nohjbot review. This approval covers that reviewed commit only.

Comment thread src/cli/hook_augment.c Resolved
A symlink into a linked worktree has no .git on its lexical path, so the
hook told the agent to run index_repository. That call canonicalizes first
and then refuses. Resolve the cwd before project lookup and the
ignore_worktrees checks.

Signed-off-by: Nohj <[email protected]>
@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

@vitalNohj

Copy link
Copy Markdown
Owner Author

@nohjbot review

@nohjbot

nohjbot Bot commented Sep 29, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review

Code review passed at 394faaac. R1 and R2 reviewed independently; R3 challenged their findings and made the final decision. GitHub approval is a separate verified publishing step.

Reasoned review decisions

R1 - correctness and execution flow (claude-opus-5-5)

This increment canonicalizes the lifecycle hook's cwd with ha_canonical_path before project resolution and before both worktree checks. I found no new defect. I traced the added symlink case: the alias resolves to &lt;worktree&gt;/nested, no project matches, cbm_git_is_linked_worktree reaches the linked worktree's .git gitlink, and the note tells the agent not to run index_repository. Before this change, the lexical walk from the alias found no .git anywhere and recommended indexing. If canonicalization fails (a relative path, a non-existent path, or a path that does not fit the buffer), cwd stays unchanged, so the previous behavior is kept. canonical_cwd stays in scope for every later use. cbm_git_same_linked_worktree now receives two canonical paths, since project_root was already canonicalized. On macOS, /tmp canonicalizes to /private/tmp, so the derived-name probe can miss a project indexed under the /tmp spelling. The registry fallback canonicalizes both the cwd and each stored root before its containment check, so such indexes are still found; the existing indexed-worktree and main-checkout assertions depend on that. The tool-search and coverage paths are unchanged. Non-blocking: a project indexed at a lexical ancestor of a symlink that points outside that ancestor no longer matches. That looks more accurate than inheriting a graph that probably does not include the symlinked tree, and I did not establish a failing path. No tests were run.

R2 - independent regression and risk (gpt-6-sol)

The lifecycle hook now canonicalizes the cwd before both project resolution and the ignore_worktrees checks. On POSIX, this makes a symlink into an unindexed linked worktree reach the worktree’s Git anchor and receive the ignored-worktree guidance; an indexed worktree still goes through the same-project comparison. I found no new code-supported bug in this increment. No tests were run.

R3 - challenge and final decision (claude-opus-5-5)

This increment has no blocking defect; neither independent review raised a candidate. ha_lifecycle_json_from_root now canonicalizes the hook cwd with ha_canonical_path before it resolves the project and before either worktree check. A symlink into an unindexed linked worktree is therefore classified from its real path. The ancestor walk reaches the worktree's gitlink, the gitdir has commondir, and the note tells the agent not to run index_repository. That matches index_repository, which also canonicalizes before its gate. If canonicalization fails (relative path, missing path, or oversized result), cwd keeps its previous value, so earlier behavior is unchanged. canonical_cwd stays in scope for every later use. cbm_git_same_linked_worktree now compares two canonical inputs, since project_root was already canonical. One side effect is that the derived-name probe now uses the canonical spelling; on macOS, /tmp becomes /private/tmp. A project stored under a non-canonical root is still found, because ha_registry_project_for_path canonicalizes each stored root before its containment check. The tool-search and coverage paths are unchanged. The new test adds the symlink-alias case alongside the existing matrix. Prior findings F1 and F2 were already resolved and stay resolved; there are no open or uncertain findings to reassess. No tests were run by this workflow.

Candidate adjudication

Neither independent review proposed a new concrete bug. R3 completed the final code-based judgment and prior-finding reconciliation.

Context limits: diff, bounded past discussion, changed-file source, and heuristic related context. 0 stage-requested source window(s), 0 unavailable/omitted, 0 truncated. R3 received 5 source entries; 16 omitted entries are recorded in the execution. At most one round of six read-only source requests per stage at pinned head/base. This is not exhaustive repository/caller coverage. No code or tests were run. Tests, logs and proof packages are not approval requirements.

Approval status

No code changes are requested by this review. GitHub approval is submitted separately after the review check succeeds and the PR snapshot is reverified. If approval is missing, the workflow operator should check the check/approval publishing steps and restore any failed API access, then request @nohjbot review on the current head. A clean comment alone is not a recorded approval.


Automatic review of new commits is off. Request another review with @nohjbot review, @nohjbot thoughts, or @nohjbot thoughts?. Add context on the same line or below, or start a review request with @nohjbot followed by your question.

@nohjbot nohjbot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No open findings in commit 394faaacfbec823e3d138407bb21ad46869f9022.

Full nohjbot review. This approval covers that reviewed commit only.

The ignore_linked initializer exceeded the formatter's assignment break.
Lint treats that as an error, so the line now matches .clang-format.

Signed-off-by: Nohj <[email protected]>
@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

@vitalNohj

Copy link
Copy Markdown
Owner Author

@nohjbot review

@nohjbot

nohjbot Bot commented Sep 29, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review

Code review passed at 3bab0613. R1 and R2 reviewed independently; R3 challenged their findings and made the final decision. GitHub approval is a separate verified publishing step.

Reasoned review decisions

R1 - correctness and execution flow (claude-opus-5-5)

This increment only reflows the ignore_linked initializer in ha_lifecycle_json_from_root to satisfy clang-format. It introduces no defect. The condition is unchanged: srv &amp;&amp; cwd &amp;&amp; cbm_mcp_ignore_worktrees_enabled(srv) &amp;&amp; cbm_git_is_linked_worktree(cwd), with the same operands in the same order and the same short-circuiting. The drop guard on the next line, cbm_git_same_linked_worktree(cwd, project_root), and the later worktree_ignored calculation are also unchanged. Execution around it still follows this order:

  • the cwd is canonicalized;
  • the project is resolved with its root;
  • a parent graph is dropped when the cwd is in a different linked worktree;
  • the owned config is detached and closed;
  • the owned server is freed.

That is the same as at 394faaa. The prior findings F1 and F2 are already resolved and are not reopened. No tests were run.

R2 - independent regression and risk (gpt-6-sol)

The only change reflows the ignore_linked assignment. It preserves the same short-circuit evaluation and leaves the subsequent same-worktree check, project cleanup, and guidance selection unchanged. I found no new code-supported defect in this increment; there are no prior open or uncertain findings to reassess.

R3 - challenge and final decision (claude-opus-5-5)

Code review passed at 3bab061. This commit only reformats the ignore_linked initializer in ha_lifecycle_json_from_root to satisfy clang-format. The condition is the same expression, srv &amp;&amp; cwd &amp;&amp; cbm_mcp_ignore_worktrees_enabled(srv) &amp;&amp; cbm_git_is_linked_worktree(cwd), with the same operands, order and short-circuiting, so behavior does not change. The surrounding logic is untouched and runs in the same order as at 394faaa: canonicalize the cwd, resolve the project with its root, apply the same-worktree drop through cbm_git_same_linked_worktree, compute worktree_ignored, then detach and close the owned config and free the owned server. Neither independent review raised a candidate, and there is nothing to adjudicate. Prior findings F1 and F2 were already resolved and are not reopened. No tests were run.

Candidate adjudication

Neither independent review proposed a new concrete bug. R3 completed the final code-based judgment and prior-finding reconciliation.

Context limits: diff, bounded past discussion, changed-file source, and heuristic related context. 0 stage-requested source window(s), 0 unavailable/omitted, 0 truncated. R3 received 4 source entries; 11 omitted entries are recorded in the execution. At most one round of six read-only source requests per stage at pinned head/base. This is not exhaustive repository/caller coverage. No code or tests were run. Tests, logs and proof packages are not approval requirements.

Approval status

No code changes are requested by this review. GitHub approval is submitted separately after the review check succeeds and the PR snapshot is reverified. If approval is missing, the workflow operator should check the check/approval publishing steps and restore any failed API access, then request @nohjbot review on the current head. A clean comment alone is not a recorded approval.


Automatic review of new commits is off. Request another review with @nohjbot review, @nohjbot thoughts, or @nohjbot thoughts?. Add context on the same line or below, or start a review request with @nohjbot followed by your question.

@nohjbot nohjbot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No open findings in commit 3bab06134bdf1401e568884e1e624c464522635e.

Full nohjbot review. This approval covers that reviewed commit only.

@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

The memory-core ratchet rejects another raw free() in the hook. Keep the
parent name until the existing free(project) and skip it when the match
is a different worktree.

Signed-off-by: Nohj <[email protected]>
@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

@vitalNohj

Copy link
Copy Markdown
Owner Author

@nohjbot review

@nohjbot

nohjbot Bot commented Sep 29, 2026

Copy link
Copy Markdown

nohjbot

In response to your request:

@nohjbot review

Review

Code review passed at b21c8ccb. R1 and R2 reviewed independently; R3 challenged their findings and made the final decision. GitHub approval is a separate verified publishing step.

Reasoned review decisions

R1 - correctness and execution flow (claude-opus-5-5)

This increment removes a second free() from ha_lifecycle_json_from_root and gets the same behavior through a drop_ancestor flag. I traced it against the previous head:

  • **Old code:** it freed project and set it to NULL when ignore_linked &amp;&amp; project &amp;&amp; !cbm_git_same_linked_worktree(cwd, project_root). It then set worktree_ignored = ignore_linked &amp;&amp; !project. The later if (project) picked the indexed or unindexed message.
  • **New code:** it keeps project allocated. drop_ancestor is exactly the old drop condition. worktree_ignored = ignore_linked &amp;&amp; (!project || drop_ancestor) matches the old value, because drop_ancestor already implies ignore_linked. The message branch is project &amp;&amp; !drop_ancestor, which matches the old non-NULL test.
  • **Memory:** the single free(project) after the message still releases the string on every path. There is no leak and no double free.
  • **Use between the flag and the message:** nothing reads project there. The config detach/close and cbm_mcp_server_free(owned_server) do not touch it, because it is a separately allocated name.
  • **Short-circuiting:** cbm_git_same_linked_worktree is still called only when ignore_linked is true and project is non-NULL.

The unindexed message and the worktree_ignored guidance are therefore the same as at 3bab061. I found no new defect. Prior findings F1 and F2 are resolved, and this change does not affect them. No tests were run.

R2 - independent regression and risk (gpt-6-sol)

The change retains a mismatched parent project name until the existing cleanup, while drop_ancestor prevents the indexed-project message from using it. The ignored-worktree guidance still applies when that match is dropped, and the name is freed once on either path. I found no code-supported defect in this increment; no tests were run.

R3 - challenge and final decision (claude-opus-5-5)

Code review passed at b21c8cc. This commit replaces the early free(project); project = NULL; in ha_lifecycle_json_from_root with a drop_ancestor flag. Behavior is unchanged. drop_ancestor is exactly the old drop condition, with the same short-circuit order, so cbm_git_same_linked_worktree still runs only when ignore_linked is true and a project was resolved. worktree_ignored = ignore_linked &amp;&amp; (!project || drop_ancestor) gives the same value the old ignore_linked &amp;&amp; !project gave after the drop, because drop_ancestor already implies ignore_linked. The indexed-message branch now tests project &amp;&amp; !drop_ancestor, which matches the old non-NULL test. On the dropped path the retained name reaches no output, and the existing single free(project) still releases it on every path, so there is no leak and no double free. Nothing between the flag and the message reads project: the config detach/close and cbm_mcp_server_free(owned_server) do not touch the separately allocated name. Neither independent review raised a candidate, and I found no new defect caused by this increment. F1 and F2 were already resolved and are not reopened. No tests were run.

Candidate adjudication

Neither independent review proposed a new concrete bug. R3 completed the final code-based judgment and prior-finding reconciliation.

Context limits: diff, bounded past discussion, changed-file source, and heuristic related context. 0 stage-requested source window(s), 0 unavailable/omitted, 0 truncated. R3 received 4 source entries; 11 omitted entries are recorded in the execution. At most one round of six read-only source requests per stage at pinned head/base. This is not exhaustive repository/caller coverage. No code or tests were run. Tests, logs and proof packages are not approval requirements.

Approval status

No code changes are requested by this review. GitHub approval is submitted separately after the review check succeeds and the PR snapshot is reverified. If approval is missing, the workflow operator should check the check/approval publishing steps and restore any failed API access, then request @nohjbot review on the current head. A clean comment alone is not a recorded approval.


Automatic review of new commits is off. Request another review with @nohjbot review, @nohjbot thoughts, or @nohjbot thoughts?. Add context on the same line or below, or start a review request with @nohjbot followed by your question.

@nohjbot nohjbot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No open findings in commit b21c8ccb6ec37b53d026984278bd795db5f3abc1.

Full nohjbot review. This approval covers that reviewed commit only.

@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

1 similar comment
@vitalNohj

Copy link
Copy Markdown
Owner Author

@kody review --force

@vitalNohj
vitalNohj merged commit cb7dfae into main Sep 30, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant