fix(search): confine the file index to the project root and key relative paths to the walk

[Vidhaankhare16]

Problem

FileIndexer is the walk behind BOSS Spotlight's file results. Its containment check carries the comment "Security: Validates that all indexed files remain within the project root to prevent path traversal via symlinks", and it does not do that. Two independent holes in one line, FileIndexer.kt:207 on dev:

if (!childCanonicalPath.startsWith(rootCanonicalPath))

1. java.io.File.canonicalPath does not follow a symlink or a directory junction on Windows. NIO's toRealPath does, on every platform, but canonicalPath does not, so on Windows a junction inside the project resolves to <root>\link, which is trivially under the root, and the entire subtree behind it is walked and indexed. mklink /J requires no elevation, unlike mklink /D, so planting one takes only write access inside the project. Measured on Windows 11 with JDK 17: a junction at <project>\link pointing at a sibling directory reports canonicalPath = <project>\link while toRealPath() = <sibling>, and the file behind it is indexed.

2. The comparison is a String prefix with no separator, so a sibling escapes on every platform. "C:\w\proj-secrets\credentials.env".startsWith("C:\w\proj") is true. With File.separator appended it is false. On macOS and Linux, where canonicalPath does resolve links, this is the live half: a symlink to a sibling whose name merely begins with the project's name passes.

There is a second, unrelated defect in the same function. relativePath is produced by slicing the child's absolute path at the length of the root's canonical path plus one:

if (absolutePath.length > rootPathLength) absolutePath.substring(rootPathLength) else name

Those are two different spellings of the same directory, so whenever they differ in length every entry in the index is cut at the wrong offset and the leading characters of the relative path are eaten. This needs no link and no unusual setup: it fires on an ordinary project tree whenever the root's absolute and canonical spellings differ at all. On Windows an 8.3 short component does it, which is how the system hands out %TEMP%: C:\Users\VICTUS~1\... canonicalises to C:\Users\VICTUS 15\..., one character longer, so one character is lost from every entry: src\Main.kt is displayed as rc\Main.kt, and Root.kt as oot.kt. On macOS a project under /tmp, really /private/tmp, makes the offset longer than the whole path, so every file falls through to the else branch and loses its folder entirely. relativePath is what Spotlight shows the user, so the corruption is visible and silent.

Both defects are already solved correctly 100 lines away in the same package. ContentSearchService.resolveFile appends File.separatorChar, re-verifies on toRealPath(), and its KDoc states the Windows canonicalPath behaviour in as many words. It also explains why the non-resolved spelling is the one to hand back. FileIndexer is the copy that drifted, and it has no test file today.

What this changes

Containment now resolves with java.nio.file.Path.toRealPath() and compares with Path.startsWith(Path), which matches whole name components rather than characters, closing both holes with the same comparison. Path.startsWith is already the idiom used correctly for the zip-slip guard in ChromiumAutoDownloader.extractWithJava.

The resolve is paid only for entries that are links. Only a link can take an entry out of the directory it was listed in, and the walk only descends through directories already confined, so a non-link entry is inside the root by construction. That is the trade TabPaths.pathsMatch documents for canonicalisation: the expensive check earns its place only where the cheap one cannot answer. It matters here because resolving unconditionally measured slightly MORE expensive than the canonicalPath call being replaced, while the gated form measures about a tenth of it, so the net effect is a cheaper walk rather than a more expensive one. A Windows junction is not reported by Files.isSymbolicLink but does set isOther, so both are checked, and an attribute read that fails answers true so an unclassifiable entry goes to the resolve rather than being waved through.

relativePath is now produced by relativizing two paths that are both in the walk's own spelling, so there is no offset to get wrong. The absolute spelling stays the one used for both IndexedFile.path and IndexedFile.relativePath, deliberately: a tab is keyed by the path it was opened with, and handing back the resolved path would make the index disagree with the editor about which file is which. That is the same reasoning ContentSearchService.resolveFile records for returning the non-canonical candidate.

The root is resolved once into a small IndexRoot holding both spellings, so the pair cannot be mixed up at a call site, and resolution failure on the root now fails closed with a warn rather than walking an unconfined tree.

scanDirectory's per-entry work moved into isSkipped, isConfined and passesExtensionFilter, with the two pure path helpers isLink and resolvesInsideRoot at file scope beside IndexRoot, where logger already lives. Those suppressed findings then stop firing rather than needing to be re-keyed to the new signature, so CyclomaticComplexMethod, NestedBlockDepth and LoopWithTooManyJumpStatements for FileIndexer come out of detekt-baseline.xml instead of moving. The cheap name tests are ordered before isConfined so an excluded directory never touches the filesystem.

Precedence and scope

No function-level overlap with any open PR. PR #593 (fix/spotlight-project-session-isolation) is the only open PR that touches FileIndexer.kt; it rewrites indexProject, adds a scan injection seam and adds SpotlightFileIndexOwner, and does not touch scanProjectFiles or scanDirectory, which is where both defects are. Whichever lands second rebases; the regions do not overlap. PRs #628 and #625 touch ContentSearchService, not FileIndexer. PRs #582, #581 and #575 touch GlobalSearchService, not FileIndexer. Checked with gh api repos/risa-labs-inc/BossConsole/pulls/<n>/files --paginate across all open PRs, because the non-paginated file list stops at 100 entries and several open PRs are over 200 files.

This is scoped to the index walk. It does not touch ContentSearchService, GlobalSearchService or the FileIndexer locking that #593 is changing.

On converging the confinement checks: after this there are two of them in this package, not three. TabPaths.pathsMatch tests whether two paths name the same file, which is identity rather than containment, so it is a different operation. The other is ContentSearchService.resolveFile, and it is correct today - it appends File.separatorChar before comparing and re-verifies on toRealPath, so it has neither hole this fixes. They still do not share a helper, and that is deliberate rather than overlooked: resolveFile accepts a relative rawPath, resolves it against the project, and must hand back the NON-resolved candidate because the editor keys open buffers by the path a tab was opened with, while the index walk needs the gate-on-link optimisation and returns nothing. A shared helper is worthwhile and I am happy to do it, but it means editing ContentSearchService.kt, which #628 and #625 both touch right now, so it belongs in its own PR once those land rather than widening this one into a collision.

Known limits

Tightening containment can only remove entries from the index, never add them, so nothing downstream can receive a path it would not have received before. FileIndexer has one caller, GlobalSearchService.fileIndexer.

A file that is itself a link to a location outside the project is now excluded, where before it was indexed. That is the intended behaviour and matches the comment that was already on the function, but it is a behaviour change for anyone who was relying on a linked file appearing in Spotlight.

A link that resolves to a location still inside the project is legitimately followed, so a project containing a link to its own ancestor walks the same subtree more than once. That is unchanged from before, and is bounded by the existing maxDepth of 15, so it is noted rather than fixed here.

Confinement and descent are a check-then-act pair: isConfined(child) runs, then child.isDirectory and listFiles(). An entry swapped for a link in that window would be walked on the strength of a check that no longer describes it. This is unchanged from the previous code and cannot be closed with java.io.File, which offers no open-then-verify handle; closing it properly means holding a directory stream over the walk. Naming it rather than leaving it for review to find.

toRealPath() requires the target to exist, so a broken link is skipped. The previous code also skipped it, via the same continue, so this is unchanged and is the fail-closed direction.

Validation

Base: origin/dev @ 9a2e5034, 0 behind and 1 ahead. The full suite below was run on c18e24a5; dev has since been rewritten (the 9.5.15 batch), and the rebase onto 9a2e5034 changed no file this PR touches, so the counts still describe this diff.

Full :composeApp:desktopTest plus ktlintCheck and detekt, on JDK 17 (17.0.20.1), Windows 11:

• 4665 tests, 486 classes, 15 skipped, 2 failures, 0 errors.
• ktlint clean and detekt clean, both executed against the rebased tree, with a detekt baseline that is now 3 entries smaller (1857 to 1854).
• Both failures are this machine's known environmental pair, and both fail with java.nio.file.FileSystemException: A required privilege is not held by the client on Files.createSymbolicLink: DesktopFileScannerTest and DiscardDownloadContainmentTest. Neither touches anything in this change. Creating a symlink on Windows needs SeCreateSymbolicLinkPrivilege, which this account does not hold.
• The 3 new tests in FileIndexerConfinementTest pass with 0 skipped, because they fall back to mklink /J, which needs no privilege. That is deliberate: the two tests above are the reason a symlink-only test cannot run on an ordinary Windows account, and the new ones are written not to have that problem.

All three tests were run against the unmodified dev copy of FileIndexer in this same harness, with only the test file added, and all three fail there:

a directory link out of the project does not put outside files in the index
  -> a file outside the project root must not be indexed: [...\proj\link\credentials.env, ...]
a root reached through a link still reports relative paths under that root
  -> expected: <src\Main.kt> but was: <rc\Main.kt>
an ordinary tree indexes with relative paths rooted at the project
  -> expected: <Root.kt> but was: <oot.kt>

The third is worth calling out: it contains no link at all. It fails on dev purely because this machine's temp directory carries an 8.3 short name, which is the everyday shape the relative-path defect needs. On a host where the root's absolute and canonical spellings already match, that test passes either way and the first two carry the proof.

Measured per-entry cost of the confinement check, mean of 5 passes over 1661 real source entries, against the getCanonicalPath the previous code called: resolving every entry with toRealPath is about 1.1x to 1.2x the old cost, while gating the resolve on the link test is 0.048x. So this is a reduction in the indexing walk's cost, not an increase.

On the gate itself: Files.isSymbolicLink measured false for a real mklink /J junction while isOther measured true, so both flags are needed. They are read once rather than twice, because Files.isSymbolicLink performs the same readAttributes(..., NOFOLLOW_LINKS) internally and returns one flag from it. Calling it first and then reading the attributes again cost 0.092x against 0.048x for the single read, almost exactly double, for identical answers on a junction, an ordinary directory and an ordinary file.

Manual checks remaining

The end-to-end Spotlight UI was not exercised: a fork build stops at the sign-in screen without SUPABASE_URL, SUPABASE_ANON_KEY, SUPABASE_FUNCTION_URL and JXBROWSER_LICENSE_KEY, so it never reaches the workspace. The assertions are against the contents of FileIndexer.indexedFiles, which is what the UI renders, not against a screenshot. Worth a maintainer eye on an installed build to confirm the Spotlight rows read as expected.

Attribution

Authored by Vidhaankhare16 vidhankhare16@gmail.com.

Maintainer review update (2026-09-14)

Original implementation and three regression tests remain credited to @Vidhaankhare16. Review-agent commit c87b61e adds accepted internal-directory-link and rejected external-file-link tests, pins linked-root absolute spelling, narrows symlink setup exceptions, bounds junction creation with diagnostics, and orders excluded-name checking before filesystem stat. It also documents the broader reparse-point performance case. Dev integration preserves the original author commit.

On current head c87b61e, all 5 focused tests passed, 0 failures/errors/skips, and composeApp ktlintCheck/detekt passed on macOS. The original three tests all failed against current dev's unchanged FileIndexer, demonstrating the regressions locally. Hosted CI: https://github.com/risa-labs-inc/BossConsole/actions/runs/34821641913 (still pending at this update).

Claude's review found production logic correct; requested test gaps and setup safeguards are addressed. Evidence and responses: #644 (comment). Helpers stay private because real-filesystem public-output tests cover the integration. Existing TOCTOU and depth-bounded repeated walks remain explicitly outside this patch.

No duplicate consolidation: #593 changes session ownership, #625/#628 replacement behavior. The pre-existing Windows highlight separator issue remains separate follow-up context. End-to-end Spotlight UI remains unexercised; no app instance was launched or stopped for this review.

[kshivang]

@claude Please review the current head 3e8b8a0 for project-index confinement, path spelling correctness, Windows junction handling, and regression coverage.

[claude]

Claude finished @kshivang's task in 5m 16s —— View job

---

Review of 3e8b8a0

• Read the diff at 3e8b8a0
• Confinement logic (isConfined / resolvesInsideRoot / IndexRoot)
• Path spelling correctness (relativeToRoot, absolute vs resolved)
• Windows junction handling (isLink, isOther, attribute-read failure)
• Regression coverage (FileIndexerConfinementTest)

Both defects are real and the fixes are correct. My findings are all test-side or notes; nothing blocks on the production change. I could not run Gradle in this environment (no cached distribution), so I verified by reading - the ktlint/detekt/suite numbers in the description are taken at face value.

---

Confinement: the argument holds

The core claim - "only a link can take an entry out of the directory it was listed in, and the walk only descends through directories isConfined has already approved" - checks out as an induction on real paths, not on spellings, which is the version that is actually sound:

• Base: the walk starts at root.absolutePath, whose toRealPath() is root.realPath by construction (FileIndexer.kt:176-180).
• Step: for an approved link directory L, realpath(L) is inside the root because resolvesInsideRoot said so; for a non-link child, realpath(child) = realpath(parent) + name, inside by hypothesis.

So the gate is not just a performance trade that happens to be safe - it is safe for the stated reason. Two corners I checked that also hold: Path.startsWith on Windows compares whole components case-insensitively (WindowsPath.startsWith → regionMatches(true, …)), and a junction to another volume fails on the root component (D:\ vs C:\) rather than sneaking through a string compare. Equality (child == root) is also true under Path.startsWith, matching resolveFile's explicit resolvedCanon.path == root.path arm.

Windows junctions: the isOther claim is right for the reason that matters, not just empirically. WindowsFileAttributes.isOther() returns true whenever FILE_ATTRIBUTE_REPARSE_POINT is set and isSymbolicLink() is false, and isSymbolicLink() is reparseTag == IO_REPARSE_TAG_SYMLINK - so a junction is isOther && isDirectory && !isSymbolicLink. Both flags from one readAttributes is the right call, and failing true on an unreadable attribute is the right direction.

Path spelling: relativize over two paths in one spelling is correct, and holds even for an un-normalized projectPath (/a/b/../proj), because relativize is lexical and both sides carry the same literal prefix. Keeping IndexedFile.path on the absolute spelling is the right call and for the reason resolveFile:299-304 records - the editor keys open buffers by the path a tab was opened with.

One behaviour change worth confirming is intended (it looks intended): a root-resolution failure now clears the index and stamps _indexedPath, where before the canonicalPath throw propagated to indexProject's catch and left the previous index in place. Since indexProject only short-circuits when _indexedFiles is non-empty, a retry still re-attempts, and the pre-existing "Invalid project path" branch already behaved this way - so this is consistent rather than sticky.

---

1. isSkipped pays a stat before the name test, and the KDoc says otherwise

FileIndexer.kt:232-238

child.name.startsWith(".") ||
    (child.isDirectory && child.name in excludedDirectories) ||
    !isConfined(child, root)

The KDoc above it says "the two cheap name tests run first". child.isDirectory is a filesystem stat, not a name test, and it is paid for every entry in the tree - including every regular file, which can never be in excludedDirectories. Reversing that one conjunct is result-identical and pays the stat only for entries actually named build, node_modules etc.:

(child.name in excludedDirectories && child.isDirectory)

The ordering was the same before this PR, so this is not a regression - but the new KDoc now asserts a property the code does not have, and the PR is making a performance argument about exactly this loop. Fix this →

2. On a OneDrive-synced project, the gate matches every entry

Because isOther() is "any reparse point", a Files-On-Demand cloud placeholder file carries FILE_ATTRIBUTE_REPARSE_POINT and reads as a link. On a project folder under OneDrive - a very common Windows setup - the link gate therefore matches essentially every entry and the walk degrades to the "resolve unconditionally" case the PR measured at 1.1-1.2x the old cost, not the 0.048x headline.

Correctness is unaffected (a placeholder's toRealPath is identity, so it stays confined), so this is not a defect. But the isLink KDoc is the place a future reader will look before re-measuring, and it currently implies the resolve is rare. One sentence there would save that work.

3. linkDirectory uses the bare-catch pattern the sibling test explicitly warns against

FileIndexerConfinementTest.kt:32-37

runCatching { Files.createSymbolicLink(link, target) }.isSuccess || junctionDirectory(link, target)

ResolveFileConfinementTest.kt:63-79, in this same package, narrows its catch to UnsupportedOperationException and FileSystemException with a comment saying why: "A bare catch (e: Exception) swallowed every error, including a link that failed to be created for a different reason - in which case the assertNull below would have 'passed' without testing anything."

Here the failure mode is a green skip rather than a false pass, which is better - but it is the same loss. On Linux and macOS (where these two tests are the ones that actually run in CI, since the junction fallback is Windows-only) any unexpected IOException silently becomes assumeTrue(false) and the confinement assertions never execute. Narrowing the catch the way the neighbour does, and adding an assertTrue(link.exists()) post-condition after a reported success, closes it.

4. A failed mklink skips with no diagnostic, and waitFor() is unbounded

FileIndexerConfinementTest.kt:44-51: redirectErrorStream(true) merges stderr into stdout and redirectOutput(Redirect.DISCARD) then throws the merged stream away - so on the one platform this fallback exists for, a junction that fails to create yields a bare "host supports neither a symlink nor a junction" with the actual reason discarded. Capturing the output into the assumption message costs a line. waitFor() also has no timeout; waitFor(timeout, unit) would keep a wedged cmd from hanging the run.

5. Nothing pins the false-negative direction

All three tests assert exclusion or path shape. None asserts that a link whose target is inside the project is still indexed - so mutating isConfined to !isLink(child) (reject every link outright) passes the entire new suite, while quietly dropping in-project links from Spotlight for every user. Given the PR's own "Known limits" section turns on precisely which links survive, that case is worth pinning.

Related: test 1 covers a directory link only. A file symlink pointing outside the project reaches the index through a different branch (passesExtensionFilter + relativeToRoot at FileIndexer.kt:213-220) than directories do, and that branch is currently unexercised for a link. ResolveFileConfinementTest uses a file symlink (link.txt) for its equivalent case.

A cheaper route to both, and to the sibling-prefix and cross-drive cases: isLink, resolvesInsideRoot and IndexRoot are file-private, so today the only way to reach any of them is a real link on a real filesystem - which is what forces the assumeTrue skips in the first place. Making them internal would let those predicates be pinned directly, unconditionally, on every platform. Fix this →

6. Two smaller notes

• resolveRoot's fail-closed path is new and untested. Hard to provoke portably, so I am noting it rather than asking for it.
• relativeToRoot's IllegalArgumentException catch is unreachable by construction, and does not cover what it might look like it covers: relativize does not throw for a non-prefix path, it returns a ..-prefixed result. Fine as belt-and-braces - just don't read it as a guard against a child outside the root.

7. Adjacent, pre-existing: GlobalSearchService splits relativePath on '/'

GlobalSearchService.kt:334 - file.relativePath.lastIndexOf('/') + 1. relativeToRoot emits File.separator, so on Windows this is -1, fileNameStart is 0, and the path-match highlight ranges are computed against the whole relative path instead of the file name. Not a regression - the old substring produced \ too - and correctly out of scope for this PR. But this PR is the producer, and the new assertion at FileIndexerConfinementTest.kt:111 ("src${File.separatorChar}Main.kt") is what makes the mismatch explicit. Worth a follow-up issue rather than widening this diff.

---

Things done right that I'd have flagged if missing

• Failing closed on an unreadable attribute (isLink → true) and on an unresolvable root, in a file whose whole point is a containment check that used to pass everything.
• Keeping the absolute spelling for path and relativePath, with the tab-keying reason recorded - the resolveFile:299-304 trap (a canonical path making readBuffer miss a buffer under a symlinked checkout, then clobbering it on save) is exactly the mistake available here.
• Removing the three detekt-baseline.xml entries instead of re-keying them to the new signature.
• Naming the TOCTOU window, the broken-link skip and the self-referential-link re-walk in the description up front rather than leaving them for review.
• Not folding this into a shared helper with ContentSearchService.resolveFile while fix(search): prevent lost updates in concurrent replacements #628/Fix/concurrent project replacements silently undo a completed replacement #625 are in flight, with the reason stated. Agreed - and when it happens, resolveFile's two-stage canonical-then-real check is the weaker half (its broken-symlink fallback is documented at ContentSearchService.kt:285-291), so the shared helper should be shaped on this PR's version, not on that one's.

Verdict: the production change reads correct to me. Findings 3, 4 and 5 are the ones I would want addressed before merge, since 5 leaves the fix's own intended behaviour unpinned; 1, 2, 6 and 7 are comments and follow-ups.
• fix/file-index-project-confinement

[kshivang]

Resolved the review findings in c87b61e:

• 1: excluded-directory names are checked before the directory stat.
• 2: documented that other reparse points/cloud placeholders also require resolution, so the fast-path benefit depends on the tree.
• 3: symlink setup now catches only UnsupportedOperationException and FileSystemException, matching the sibling test; successful directory/file links have an existence assertion.
• 4: mklink output is retained for failure diagnostics, and process waiting is bounded to ten seconds with cleanup.
• 5: added end-to-end tests for an internal directory link remaining indexed and an external file symlink being excluded. The linked-root test also pins the original absolute path spelling.

The private helpers remain private: testing them directly would still need real filesystem links and would not eliminate platform setup. The public index output provides stronger integration coverage. Root resolution clearing stale results is intentional and still permits retry; relativeToRoot is display derivation, not the containment boundary. The adjacent Windows highlight separator issue predates this PR and is recorded as cross-group follow-up context, without widening this change.

The contributor's original three tests passed locally, and all three failed against current dev's unchanged FileIndexer on macOS. The new test/ordering/documentation changes are review-agent repairs; Vidhaankhare16 retains credit for both original fixes, the path model, and original regressions. Current-head validation is running; no second Claude request is needed for these bounded responses.

[kshivang]

@Vidhaankhare16 Your change usefully fixes two real Spotlight defects: project-boundary escapes through links and incorrectly sliced display paths. I reproduced all three original regression failures against current dev on macOS, and the original implementation fixes them. Your original commit and attribution are preserved.

I integrated current dev and addressed Claude's requested test/setup improvements in c87b61e. All five focused tests now pass locally with zero skips, and composeApp ktlintCheck/detekt pass. Claude's substantive findings are resolved with evidence in the review response above. No unresolved production correctness/security/regression defect was found in this change.

I am moving this to draft solely because current-head hosted validation has not completed within this review's bounded window: https://github.com/risa-labs-inc/BossConsole/actions/runs/34821641913 . Code quality, pgTAP, shell tests, version and BOSS AI contracts passed; Linux/Windows builds and compatibility remain running, and macOS is queued. This is an infrastructure/validation handoff, not an author defect. The current run is left running; only the superseded-head run was cancelled.

Next steps: wait for that exact-head run to finish; investigate any actual failures with their logs, separating shared-base/infrastructure failures from this diff. Confirm the five FileIndexerConfinementTest cases pass on supported runners (file-symlink setup may legitimately skip without Windows privilege), plus the full hosted required checks. Then mark ready and merge only to dev after refreshing dev/mergeability under the merge lock with the reviewed head match. No additional feature implementation or repeated Claude request is needed for the resolved findings. Optional installed-app visual check: Spotlight should omit outside linked files, retain inside linked files, and show src/Main.kt intact when the project root is reached through a link. That UI check has not been claimed here.

[Vidhaankhare16]

Thanks for the review and for pushing the repairs in c87b61e. The findings are fair. Finding 1 in particular is mine to own: child.isDirectory is a stat, not a name test, and I wrote a KDoc asserting "the two cheap name tests run first" in the exact loop this PR makes a performance argument about. Your reordering is right, and so is the cloud-placeholder note, which is the case my 0.048x measurement does not represent. The two tests you added for the preserved-link direction close the real gap: my suite only pinned exclusion, so a mutation of isConfined to reject every link would have passed it while silently dropping in-project links from Spotlight.

On the current CONFLICTING state, in case the diagnosis saves you a step. The branch is not conflicting on content. It carries the merge commit a15c25b plus a4b91fb and 8e05bf6, and dev has since been rewritten to a3f4b7f, so those commits are no longer on it. I checked that the work in them is not lost: 3c57809 (#649) carries TerminalCommandApprovalQueue.kt, and dev's BossAppEventBusEffects.kt:149 already has the enqueue-before-log ordering from 8e05bf6. So replaying only 3e8b8a0 and c87b61e onto a3f4b7f drops nothing of yours.

I have that replay prepared and verified locally: both commits cherry-pick cleanly onto a3f4b7f with no conflicts, the result is the same three files, and your authorship and empty trailers are preserved on c87b61e.

I have not pushed it, for a reason worth stating rather than guessing at. Every push of mine on this branch (7e2cfac, 838ac0f, 3e8b8a0) landed at action_required and ran no jobs; the green six-job run exists because you pushed c87b61e. Force-pushing now would discard that green run and replace it with one blocked on your approval, while you had validation in flight on the head I would be overwriting. Say the word and I will push the rebase, or take it yourself if that is simpler.

Two smaller replies to the review, neither asking for anything. The relativeToRoot IllegalArgumentException catch is belt-and-braces exactly as you read it, and you are right that relativize returns a ..-prefixed path rather than throwing for a non-prefix input, so it should not be read as a containment guard. And on the shared helper with resolveFile, I agree it should be shaped on this PR's version rather than that one's, for the reason you give.

The GlobalSearchService.kt:334 separator issue is pre-existing and I agree it stays out of this diff. I am happy to open it as a separate issue with the measurement, if you would rather have it tracked than left in this thread.

No merge or auto-merge is authorized.

[Vidhaankhare16]

The exact-head run you were waiting on has finished green: https://github.com/risa-labs-inc/BossConsole/actions/runs/34821641913 is completed / success on c87b61e, all ten jobs, including build-test on windows-latest, macos-latest and ubuntu-latest, plus code-quality, compatibility-test, version-check, pgTAP, shell tests and BOSS AI contracts. That was the sole stated reason for the draft, so I am marking this ready for review.

One thing I could not confirm from here, so I am not claiming it: whether any of the five FileIndexerConfinementTest cases skipped on the hosted runners. The job logs surface Gradle task lines rather than per-test results, and the file-symlink case can legitimately skip without the Windows symlink privilege, which is the reason the junction fallback exists. You already reported zero skips locally; the hosted per-test view needs the test artifacts, which you can reach and I cannot.

I have deliberately not rebased. The branch is still at the head you reviewed and validated, which is what "the reviewed head match" needs, and the dev/mergeability refresh under the merge lock stays yours as you described. The CONFLICTING flag is the stale history described in my previous comment, not a content conflict: replaying only 3e8b8a0 and c87b61e onto current dev applies cleanly, and I have that verified locally if you want it pushed instead.

Worth noting that a push from me cannot help here anyway: every push of mine on this branch landed at action_required and ran no jobs, so rebasing now would trade this green run for one blocked on your approval.

No merge or auto-merge is authorized.

[kshivang]

@Vidhaankhare16 Thank you for the component-aware real-path confinement and walk-relative display paths. I rechecked your revisions and the prior repair: the five regression cases and the accepted-link direction are useful, and all ten checks on c87b61e are now green. The earlier pending-CI finding is closed.

The remaining blocker is integration after dev history changed, not a newly found flaw in your confinement algorithm. The published head currently conflicts in .gitignore, BossBottomBar.kt and McpPolicyEngine.kt. I verified your suggested replay: only 3e8b8a0 and c87b61e apply cleanly onto dev 6004233, producing just the intended three-file search change (local replay 952801d9b). I did not overwrite your branch.

Since that check, #593 has merged into dev as 5dc8211. Comparing the replay with that dev now produces the expected conflicts in FileIndexer.kt and composeApp/detekt-baseline.xml. Concrete next steps: replay the two search commits onto fresh dev, retain #593's suspend scanner signatures and ensureActive() checks at scan entry and each child, retain its mutex/cancellation handling and injectable scan seam, and combine those with your IndexRoot/isConfined/relativeToRoot helpers. The old complexity baseline entries should disappear where your decomposition removes them; do not restore obsolete non-suspend signatures.

Validate the combined result with FileIndexerConfinementTest, FileIndexerTest, SpotlightFileIndexOwnerTest and the full search-package suite, plus ktlint/detekt and required hosted CI on the new head. Previous green CI cannot validate that new integration. This sweep's shared-build-lock attempt did not acquire the lock; no new Gradle result is claimed. Prior local five-test/no-skip evidence remains valid for the old head.

I am returning this to draft for that precise integration/validation handoff under the bounded-review policy. No source changes or force push were made to your branch. #593 session ownership and #670 provider forwarding are complementary, not duplicates. Manual Spotlight checks through a linked root, internal links and external sibling links remain unperformed. Once integrated and green, mark ready for an exact-head, lock-serialized squash merge to dev.

[Vidhaankhare16]

Integrated and pushed. New head is c896ec9858e149d46ace3e762e9aace59bf4bae5, and the PR is back to MERGEABLE: 2 commits, 3 files, +378/-66.

The branch was rewritten to drop the merge commit a15c25b7, which is what made this read as seventeen commits and produced the .gitignore / BossBottomBar.kt / McpPolicyEngine.kt conflicts. Only the two search commits were replayed, which is the replay you verified yourself. Your c87b61e0 is preserved as c896ec98 with your authorship intact; its original SHA is no longer reachable from the head, which is the unavoidable cost of the rewrite rather than anything discarded.

Integrated onto de67e2a8 and rechecked against current dev 7082e2cb: still merges clean, and FileIndexer.kt has not been touched on dev since. The new RememberSpotlightFileIndexer.kt references only the FileIndexer type, not any internal this change moves, so the two compose.

Your integration points, one by one

1. Fix/spotlight project session isolation #593's suspend scanner signatures retained. scanProjectFiles and scanDirectory are both suspend. No non-suspend signature was restored.
2. ensureActive() at scan entry and each child retained. Three call sites: scanProjectFiles entry, scanDirectory entry, and once per child inside the loop.
3. Mutex and cancellation handling retained. indexingMutex.withLock is kept, so requests queue rather than drop. My branch had tryLock, which skips; that behaviour is gone in favour of yours. The CancellationException rethrow ahead of the generic catch is kept.
4. Injectable scan seam retained. The constructor is still FileIndexer(scan: (suspend (String) -> List<IndexedFile>)? = null), and indexProject still prefers scan?.invoke(...) over the real walk.
5. Confinement helpers combined in. IndexRoot, resolveRoot, isSkipped, isConfined, passesExtensionFilter, relativeToRoot, plus file-level isLink and resolvesInsideRoot. toRealPath with component-wise Path.startsWith replaces the junction-blind canonicalPath and the separator-less String prefix.

Baseline entries. Three disappeared, not two: CyclomaticComplexMethod, NestedBlockDepth and LoopWithTooManyJumpStatements, all for the old scanDirectory. Detekt then ran against the integrated source and returned an empty report, so the decomposition genuinely drops all three rather than my having guessed at it. No obsolete signature was re-added.

Validation

Temurin 17, full :composeApp:desktopTest, both runs warm against the same populated build cache:

	tests	failures	skipped
clean dev de67e2a8	5028	2	15
this head c896ec98	5033	2	16
delta	+5	0	+1

The change adds exactly its five tests and one environmental skip; nothing else moves. Both remaining failures are the disclosed symlink-privilege ones, present on clean dev too.

Search package, all green: FileIndexerConfinementTest 5, FileIndexerTest 6, SpotlightFileIndexOwnerTest 4, GlobalSearchIsolationTest 3, SpotlightSearchEffectTest 1, GlobalSearchNewSourcesTest 30, GlobalSearchOrderingTest 5, GlobalSearchTabsTest 11, SearchSourceRegistrarTest 7. That is 72 tests, 0 failures, and it puts #593's cancellation cases and this change's confinement cases in the same passing build. :composeApp:ktlintCheck and :composeApp:detekt both clean.

One correction to my earlier evidence. I previously reported five confinement tests with no skips. On this Windows host that is wrong: a file link outside the project is excluded is skipped, because it needs a file symlink and the mklink /J fallback only makes directory junctions. Four pass, one skips. The three directory-link cases do use the junction fallback. Hosted Linux CI should run all five.

Worth knowing before you re-run anything. A cold run of this suite reports false failures. Mine reported 5: the 2 above plus HeadlessLoggingTest ("logging probe must finish") and two GitCloneProcessTreeTest cases (PID capture, process-tree termination). The identical commit re-run warm reports only the 2. Those three are timing-sensitive and starve when compilation runs alongside them - the cold run took 45m6s against 7m50s warm - and all three pass on this head warm. They are not related to this change: neither test imports anything from the search package, and FileIndexer is referenced only from search and dialog code.

Remaining gate

Required hosted CI on this new head, which is at action_required like every push of mine here. Previous green CI does not cover this integration and I am not offering it as though it did. That approval is the one step I cannot take.

Once it is green I will mark this ready, which is the order you asked for. The manual Spotlight checks through a linked root, internal links and external sibling links remain unperformed and I am not claiming them.

No merge or auto-merge is authorized.

[kshivang]

@Vidhaankhare16 Your c896ec9 follow-up addresses the previous integration handoff: suspend scanning, all three cancellation checkpoints, mutex admission, cancellation propagation and the injected scan seam are retained. Thank you also for correcting the Windows file-symlink skip count.

Current dev was rebased again, so I integrated dev 77c4a40 in normal merge commit 0b7cfda, preserving your two published commits and their author identities. The resulting delta is exactly the intended three-file patch: FileIndexer, its five confinement tests, and removal of three obsolete detekt entries. Its patch is byte-identical to your two-commit delta except for the detekt baseline's old/new blob IDs, reflecting unrelated baseline updates on dev. No new production or test repair is claimed.

Required hosted CI is running on this head: https://github.com/risa-labs-inc/BossConsole/actions/runs/34959783785 . Local search-package tests plus composeApp ktlint/detekt are queued under the shared build lock. Prior Claude findings and their repair response remain applicable because the implementation/test patch is unchanged. Keeping this ready while checks run; pending CI is not an author defect. Final merge, if green, will be an exact-head squash into dev under the shared merge lock. Manual linked-root/internal-link/external-link Spotlight checks remain unperformed.

[kshivang]

Merged into dev as cf21742 after all ten Build & Test jobs passed on exact reviewed head 0b7cfda. CI: https://github.com/risa-labs-inc/BossConsole/actions/runs/34959783785 . The optional Supabase Preview check was skipped.

Downloaded and parsed all three platform test artifacts: Ubuntu, Windows and macOS each report 140 search-package plus SpotlightSearchEffect tests, zero failures/errors/skips, including all five confinement cases and the existing cancellation/session-owner tests. Root detekt and ktlintCheck passed in the code-quality job. The local attempt waited nine minutes for the shared build lock without starting Gradle, so I cancelled only that owned waiter; these are hosted results, not a new local pass.

Under the shared merge lock I refreshed main/dev, rechecked OPEN/non-draft/dev, exact head, clean mergeability, review state and all checks; recomputed the merge tree against dev61200ffe6; and required the complete three-file delta to equal the reviewed patch (ignoring only blob-ID header lines). Used a squash merge matched to the verified head. Concurrent #710, #590 and #640 changes were preserved. Main and integration #713 were untouched.

Original algorithm/decomposition/three tests and your follow-up integration remain credited to @Vidhaankhare16; prior agent test/setup improvements remain separately credited in the description. This batch added only integration, not new contributor implementation. Manual Spotlight/Windows non-admin junction checks remain unperformed; the hosted Windows run executed every confinement test without skips.