Skip to content

Re-send repo paths when repos.json changes under a running daemon - #959

Merged
alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-838-repos-add-live-refresh
Sep 15, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-838-repos-add-live-refresh

Conversation

@alexeyzimarev

@alexeyzimarev alexeyzimarev commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

AI-838 — no GitHub issue exists for this one; the Linear issue predates the GitHub-first convention.

What & why

kcap repos add writes repos.json and exits, and the daemon read that file only at registration and after its own launches, so a repo added from a terminal never reached the launch dialog until the daemon restarted. The daemon now polls a content hash of the file (one read of a small file every 3 s) and re-sends its repo paths through the existing DaemonUpdateRepoPaths hub method when the file differs from the one the last send read. Polling covers every writer — the CLI, the desktop app, the daemon's own launch path, a hand edit — without any of them needing to know whether a daemon is running.

Where to look

The watcher compares against the fingerprint recorded by the last successful send, not a baseline it primes at start. That closes the gap between registration reading the file and the watcher's first tick, and makes a failed send retry on the next tick with no extra state. The fingerprint is taken before the read it pairs with, so a write landing mid-read still reads as a change afterwards. Registration and the update share the connection's existing lock across snapshot-and-send, because the server runs one client's invocations in parallel and two overlapping sends could otherwise land in the opposite order to the fingerprints they record.

Verification

Three mutations against the new tests, each restored afterwards:

Mutation Result
Readiness gate removed from RepoStoreWatcher.TickAsync 1 daemon test failed (Nothing_is_sent_until_the_daemon_is_registered)
Send path stops recording the fingerprint 2 failed (A_send_carries…records_the_file_it_read, The_watcher_sends…exactly_once)
Update path no longer takes the lock 1 failed (Overlapping_sends_run_one_at_a_time)

Full suites on this branch:

Suite Passed Skipped Failed
Capacitor.Cli.Core.Tests.Unit 3296 9 0
Capacitor.Cli.Daemon.Tests.Unit 3204 38 1

The one daemon failure is unrelated to this change: Installed_codex_schema_matches_the_vendored_pin fails because this machine's Codex is 0.154.0 against a pin from 0.147.0.

dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release 2>&1 | grep -E 'IL[23][01][0-9]{2}' — no output.

🤖 Generated with Claude Code

The watcher compares the file against the fingerprint the last successful send read, not a baseline primed at start, so a write between registration and the first tick is not lost and a failed send retries by itself.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 15, 2026

Copy link
Copy Markdown

AI-838

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-15T14:45:06.835674Z 46f3f50 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Refresh daemon repo paths when repos.json changes

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Poll repos.json after daemon registration and republish changed repository paths.
• Track each successful send's fingerprint so failed publications retry automatically.
• Cover fingerprinting, readiness, deduplication, deletion, and retry behavior with tests.
Diagram

graph TD
  Writers["Repo Writers"] --> Store[("repos.json")] --> Watcher["Repo Watcher"] --> Connection["Server Connection"] --> Hub["Update Hub"] --> Dialog["Launch Dialog"]
  Registration["Daemon Registration"] --> Connection
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Filesystem change notifications
  • ➕ Provides near-immediate refreshes without periodic stat calls.
  • ➕ Avoids polling when the file remains unchanged.
  • ➖ Filesystem watcher behavior differs across platforms and editors.
  • ➖ Rename-based atomic saves can produce coalesced or missed event sequences.
  • ➖ Requires additional recovery logic after watcher errors or buffer overflow.
2. Writer-triggered daemon notification
  • ➕ Publishes immediately after a known writer changes the store.
  • ➕ Avoids continuous polling and filesystem event complexity.
  • ➖ Every writer must discover and communicate with the running daemon.
  • ➖ Manual edits and unmodified third-party writers remain invisible.
  • ➖ Couples repository persistence to daemon availability and IPC behavior.

Recommendation: Keep the polling approach. A three-second stat of one small file is inexpensive, covers every writer uniformly, and naturally retries failed publications because the advertised fingerprint advances only after success. Filesystem notifications offer lower latency but introduce substantially more cross-platform and recovery complexity for little user-visible benefit.

Files changed (10) +329 / -2

Enhancement (1) +6 / -0
RepoStoreFingerprint.csDefine immutable repository store identity +6/-0

Define immutable repository store identity

• Introduces a record representing the size and modification timestamp of 'repos.json', allowing atomic replacement and value comparison.

src/Capacitor.Cli.Core/Config/RepoStoreFingerprint.cs

Bug fix (4) +106 / -1
RepoPathStore.csExpose the repository store fingerprint +11/-0

Expose the repository store fingerprint

• Adds a safe fingerprint operation based on file size and UTC modification ticks. Missing or inaccessible files produce a null fingerprint.

src/Capacitor.Cli.Core/Config/RepoPathStore.cs

DaemonRunner.csRegister the repository store watcher +3/-0

Register the repository store watcher

• Adds 'RepoStoreWatcher' as a singleton hosted service so monitoring runs for the daemon lifetime.

src/Capacitor.Cli.Daemon/DaemonRunner.cs

RepoStoreWatcher.csPoll and republish changed repository paths +74/-0

Poll and republish changed repository paths

• Adds a hosted service that checks 'repos.json' every three seconds after registration. It republishes changed paths and leaves failed sends eligible for retry on the next tick.

src/Capacitor.Cli.Daemon/Services/RepoStoreWatcher.cs

ServerConnection.csTrack the repository store behind successful sends +18/-1

Track the repository store behind successful sends

• Captures the store fingerprint before reading paths and records it only after registration or update publication succeeds. Extracts the hub send operation into an overridable method for focused testing.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Tests (3) +202 / -0
RepoPathStoreTests.csTest repository store fingerprint behavior +35/-0

Test repository store fingerprint behavior

• Verifies null fingerprints for missing files, stability without writes, and changes after repository additions or removals.

test/Capacitor.Cli.Core.Tests.Unit/Config/RepoPathStoreTests.cs

RepoStoreWatcherTests.csTest watcher publication and retry decisions +97/-0

Test watcher publication and retry decisions

• Covers unchanged, changed, appearing, and disappearing files, along with registration readiness and failed-publication retries.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/RepoStoreWatcherTests.cs

ServerConnectionRepoPathsTests.csTest successful-send fingerprint integration +70/-0

Test successful-send fingerprint integration

• Verifies sent paths and advertised fingerprints, confirms failures do not advance state, and exercises the production watcher-to-connection wiring for exactly-once publication.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/ServerConnectionRepoPathsTests.cs

Documentation (2) +15 / -1
README.mdDocument live repository-path refreshes +1/-1

Document live repository-path refreshes

• Explains that a running daemon notices repository additions and removals within a few seconds, without requiring a restart.

README.md

CHANGES.mdRecord the repos.json watcher design +14/-0

Record the repos.json watcher design

• Documents the stale launch-dialog problem, polling strategy, successful-send fingerprint semantics, race handling, and rationale for avoiding writer-driven notifications.

docs/CHANGES.md

@qodo-code-review

qodo-code-review Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Rapid edits can remain invisible ✓ Resolved 🐞 Bug ≡ Correctness
Description
Fingerprint identifies repository-store content solely through file length and
LastWriteTimeUtc.Ticks, although equal values do not prove equal content. A same-length write
within the filesystem’s timestamp resolution, such as re-adding an existing repository and changing
only its LastUsed value, compares equal and leaves the server’s launch ordering stale until
another detectable write or reconnect.
Code

src/Capacitor.Cli.Core/Config/RepoPathStore.cs[R114-118]

+    public RepoStoreFingerprint? Fingerprint() {
+        try {
+            var info = new FileInfo(StorePath);
+            return info.Exists ? new RepoStoreFingerprint(info.Length, info.LastWriteTimeUtc.Ticks) : null;
+        } catch {
Relevance

●● Moderate

Plausible correctness issue, but no closely matching historical precedent for filesystem fingerprint
collisions.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Adding an already-known repository rewrites only its LastUsed field, saves the complete JSON
through atomic replacement, and repository paths are subsequently ordered by that field. The watcher
suppresses publication whenever the new size-and-mtime record equals the advertised record, so a
fingerprint collision directly preserves stale launch-dialog ordering.

src/Capacitor.Cli.Core/Config/RepoPathStore.cs[53-71]
src/Capacitor.Cli.Core/Config/RepoPathStore.cs[95-101]
src/Capacitor.Cli.Core/Config/RepoPathStore.cs[104-120]
src/Capacitor.Cli.Daemon/Services/RepoStoreWatcher.cs[47-56]
src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[747-761]
README.md[1852-1855]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
File size and modification time can remain unchanged across a supported repository-store rewrite, causing the watcher to miss changed content. Re-adding an existing repository updates its recency and ordering but commonly preserves serialized length, so timestamp granularity can make the old and new files indistinguishable.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Config/RepoPathStore.cs[53-71]
- src/Capacitor.Cli.Core/Config/RepoPathStore.cs[95-101]
- src/Capacitor.Cli.Core/Config/RepoPathStore.cs[112-120]
- src/Capacitor.Cli.Daemon/Services/RepoStoreWatcher.cs[47-56]

## Recommended Fix
Replace the size-and-mtime identity with a collision-resistant content fingerprint, such as a hash computed from the complete file bytes, while retaining the before-read comparison strategy needed to detect writes during publication. Add a test using two distinct same-length contents with the same modification timestamp and verify that the watcher republishes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Repo lists can permanently revert ✓ Resolved 🐞 Bug ≡ Correctness
Description
UpdateRepoPathsAsync snapshots, sends, and assigns _advertisedRepoStore without serializing
those steps against another update or daemon registration. When the existing launch-triggered update
overlaps the new watcher and the older server call lands last while the newer continuation records
its fingerprint last, the server retains older paths while the watcher sees the current fingerprint
as advertised and never retries.
Code

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[R850-853]

+            var repoStore = FingerprintRepoStore();
            var repoPaths = await MergeRepoPathsAsync();
-            await _hub.InvokeAsync("DaemonUpdateRepoPaths", repoPaths, cancellationToken: _ct);
+            await SendRepoPathsAsync(repoPaths);
+            _advertisedRepoStore = repoStore;
Relevance

●● Moderate

Concurrency ordering concerns are accepted historically, but this specific race lacks decisive
repository precedent.

PR-#880
PR-#497

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new update path records a fingerprint only after its asynchronous send, while the pre-existing
agent launch path starts updates in fire-and-forget tasks and registration independently sends
another repository snapshot. Because no shared lock couples each send to its fingerprint assignment,
task continuations can assign fingerprints in a different order from the server calls, producing a
stable false match that suppresses subsequent watcher sends.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[649-705]
src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[737-761]
src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[847-860]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[4298-4306]
src/Capacitor.Cli.Daemon/Services/RepoStoreWatcher.cs[50-59]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Concurrent repository-path publications can complete and resume out of order, leaving the server with old paths while `_advertisedRepoStore` records the newest fingerprint. The watcher then considers the file synchronized and does not repair the stale server state.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[649-705]
- src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[847-860]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[4298-4306]

## Recommended Fix
Add a shared asynchronous lock that serializes each fingerprint-read, path-read, hub-send, and advertised-fingerprint assignment as one operation. Use the same serialization for registration’s repository snapshot and the standalone update path, then add an overlapping-send test that forces reverse completion and verifies server state and the advertised fingerprint remain paired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. A test divider repeats its subject ✗ Dismissed 📘 Rule violation ⚙ Maintainability
Description
The // ── Fingerprint ── divider merely labels the fingerprint tests whose adjacent method names
already identify that subject. When tests are added or reorganized, this ornamental heading adds
another label to maintain without preserving any non-obvious constraint or rationale.
Code

test/Capacitor.Cli.Core.Tests.Unit/Config/RepoPathStoreTests.cs[275]

+    // ── Fingerprint ──────────────────────────────────────────────────────────
Relevance

●●● Strong

Redundant section comments are routinely removed when they duplicate nearby names or add no
behavioral rationale.

PR-#433

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2762993 permits comments only when they document non-obvious, behavior-critical
constraints. The added divider at line 275 is solely a section label, while every following test
name already starts with Fingerprint_.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.Cli.Core.Tests.Unit/Config/RepoPathStoreTests.cs[275-275]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly added fingerprint section divider only repeats the subject expressed by the adjacent test names and does not document a non-obvious constraint.

## Fix Focus Areas
- test/Capacitor.Cli.Core.Tests.Unit/Config/RepoPathStoreTests.cs[275-275]

## Recommended Fix
Remove the ornamental fingerprint divider while leaving the grouped tests and their descriptive names unchanged.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (sha: 9cf4bbe1)
Review mode: ⚖️ Balanced: This changes daemon lifecycle behavior, file polling, concurrent registration/update state, and hub communication across multiple production paths, making it meaningfully risky but not dense enough to warrant redundant extended review.

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread test/Capacitor.Cli.Core.Tests.Unit/Config/RepoPathStoreTests.cs
Comment thread src/Capacitor.Cli.Daemon/Services/ServerConnection.cs
Comment thread src/Capacitor.Cli.Core/Config/RepoPathStore.cs

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 46f3f50f3c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

public RepoStoreFingerprint? Fingerprint() {
try {
var info = new FileInfo(StorePath);
return info.Exists ? new RepoStoreFingerprint(info.Length, info.LastWriteTimeUtc.Ticks) : null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Hash repo contents to detect same-size writes

On filesystems with coarse timestamp resolution, such as some NFS or FAT-backed home directories, two writes between the watcher's three-second polls can have the same LastWriteTimeUtc; replacing one path with an equal-length path also preserves the file size. The fingerprints then compare equal, so TickAsync skips the update indefinitely even though repos.json changed. Include content identity, such as a hash, rather than relying solely on size and mtime.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 89034ef. The fingerprint is now a SHA-256 of the file's bytes, read with FileShare.Delete so a save in another process can still rename over it mid-read. Fingerprint_TellsApartSameLengthWritesWithTheSameTimestamp writes two same-length contents with an identical LastWriteTimeUtc and asserts the fingerprints differ; Fingerprint_IgnoresATouchThatLeavesTheContentAlone pins the other direction, so a bare touch no longer triggers a send.

The server runs one client's hub invocations in parallel, so an older repo list can be processed after a newer one while the newer fingerprint is recorded last; and a same-length rewrite inside the filesystem's timestamp resolution reads as no change under size and mtime.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit 2ca4a53 into main Sep 15, 2026
8 checks passed
@alexeyzimarev
alexeyzimarev deleted the alexeyzimarev/ai-838-repos-add-live-refresh branch September 15, 2026 15:15
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