Skip to content

Fix findings from the nightly PR-review and docs-sync issues - #494

Merged
Sev7eNup merged 9 commits into
mainfrom
fix/review-findings-2026-10-10
Oct 10, 2026
Merged

Sev7eNup merged 9 commits into
mainfrom
fix/review-findings-2026-10-10

Conversation

@Sev7eNup

Copy link
Copy Markdown
Owner

Works through the open PR-review and docs-sync issues (#472, #473, #475, #476, #478, #483, #491, #493).

Code

  • Folder tree lock: readers (hub joins, live-event flush, dashboard scope) share it, mutations and the subscription revalidation stay exclusive, waiters are served FIFO. The notifier waits at most BatchTimeout for the lock and drops the batch if a mutation outlasts it.
  • Backup restore keeps child workflow references it cannot resolve (Guid.Empty from an import, or a deleted workflow) as stored instead of aborting. A warning would not help here: any restore warning aborts the restore.
  • Flat FileWatcher scans skip links and files that vanish mid-scan instead of failing. Recursive watches still reject reparse points.
  • CI runs six offline deployment/test-suite scripts that nothing called. Verified under Windows PowerShell 5.1 only; PowerShell 7 was not available locally.
  • Electron 44.5.1 (same Node 24.21.0 and Chromium 152 as 44.4.5). Replaces Bump the npm-minor-patch group in /src/nodepilot-desktop with 2 updates #479; the lockfile only carries the electron and @types/node entries.

Guards and docs

  • DocumentationCountsTests pins the e2e catalogue part count; the e2e README says 86 (same fix as docs: sync after #466 (E2E catalogue Teile count) #474).
  • Test that the gate.py UI source hash ignores test files.
  • Docs site: daylight saving behaviour of schedule triggers, masked contract defaults, provider-atomic config lists, and the duplicate custom-activity-key upgrade note (the startup error now names it).

Known and not changed here: test_gate.py::test_catalog_is_complete_and_drift_checked fails on main because coverage-review.json lags the UI sources.

Tests run locally: Data.Tests (lock, custom activity key) 17, Api.Tests (restore, notifier, hub, dashboard, settings docs) 265, Engine.Tests (FileWatcher) 67, Mcp.Tests (DocumentationCountsTests) 29, docs-ui 221, desktop package contract 8.

Closes #472, closes #473, closes #475, closes #476, closes #478, closes #483, closes #491, closes #493

🤖 Generated with Claude Code

Sev7eNup and others added 9 commits October 10, 2026 15:28
Hub joins, the live-event flush and the dashboard scope took the same
exclusive semaphore as folder mutations, so a backup restore or a move
stalled every live event and join. The lock is now a FIFO reader/writer
lock: readers share it, mutations and the subscription revalidation stay
exclusive, and a waiting mutation is not overtaken by new readers.

The notifier bounds its wait for the lock by BatchTimeout and drops the
batch if a mutation outlasts it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A startWorkflow/forEach reference that is neither in the backup nor in
the database made the whole restore fail. The import writes Guid.Empty
for such references on purpose, so a backup of a system holding one could
not be restored. The reference now stays as stored.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The reparse-free walk threw on any reparse entry and on any failure to
read attributes, also when the watch does not recurse. A junction in the
drop folder, a dedup/cloud placeholder or a file deleted mid-scan marked
the source unhealthy, so the trigger never fired. A flat scan now skips
those entries; recursive watches still reject reparse points.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
They only use fakes or the script AST, but nothing called them.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
44.5.1 embeds the same Node (24.21.0) and Chromium (152) as 44.4.5, so
the embedded-Node note stays valid. The pin guard, the note and the
changelog follow the new version. The lockfile only carries the electron
and @types/node entries.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
DocumentationCountsTests now pins the part count the e2e README quotes
(86). Also explains that the manual E2E checklist leaves out
automated-only parts, and adds a test that gate.py's UI source hash
ignores test files.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Quartz 4 fires a skipped time at the end of the gap and fires hourly or
more frequent schedules again in the repeated hour. Also lists the new
cron parsing rules.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contract defaults read as *** for callers without Edit on the folder.
Several allow/bypass/role lists are read whole from the highest-priority
provider instead of being merged by index. The redactor fast-path row
in performance-improvements.md now matches the current pass structure.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The startup error pointed at docs/custom-activity-key-migration.md,
which nothing linked and installations don't ship. The conflict query
and procedure are now on the Production deployment page of the
documentation website, custom-activities.md links the guide and states
the uniqueness rule, and the error message names the website page.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
{
DeleteDirectoryLink(dirLink);
try { File.Delete(fileLink); } catch { }
try { Directory.Delete(stage, recursive: true); } catch { }
{
DeleteDirectoryLink(dirLink);
try { File.Delete(fileLink); } catch { }
try { Directory.Delete(stage, recursive: true); } catch { }
[WindowsFact]
public async Task Execute_ManualFlatScan_SkipsLinksInsteadOfFailing()
{
var stage = Path.Combine(Path.GetTempPath(), "nodepilot-fw-flat-link-" + Guid.NewGuid().ToString("N"));
public async Task Execute_ManualFlatScan_SkipsLinksInsteadOfFailing()
{
var stage = Path.Combine(Path.GetTempPath(), "nodepilot-fw-flat-link-" + Guid.NewGuid().ToString("N"));
var watched = Path.Combine(stage, "watched");
{
var stage = Path.Combine(Path.GetTempPath(), "nodepilot-fw-flat-link-" + Guid.NewGuid().ToString("N"));
var watched = Path.Combine(stage, "watched");
var outside = Path.Combine(stage, "outside");
var fileLink = Path.Combine(watched, "file-link.log");
Directory.CreateDirectory(watched);
Directory.CreateDirectory(outside);
await File.WriteAllTextAsync(Path.Combine(watched, "real.log"), "x");
Directory.CreateDirectory(watched);
Directory.CreateDirectory(outside);
await File.WriteAllTextAsync(Path.Combine(watched, "real.log"), "x");
await File.WriteAllTextAsync(Path.Combine(outside, "outside-secret.log"), "secret");
try
{
Directory.CreateSymbolicLink(dirLink, outside);
File.CreateSymbolicLink(fileLink, Path.Combine(outside, "outside-secret.log"));
finally
{
DeleteDirectoryLink(dirLink);
try { File.Delete(fileLink); } catch { }
finally
{
DeleteDirectoryLink(dirLink);
try { File.Delete(fileLink); } catch { }
@Sev7eNup
Sev7eNup merged commit fd63443 into main Oct 10, 2026
17 checks passed
@Sev7eNup
Sev7eNup deleted the fix/review-findings-2026-10-10 branch October 10, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment