fix(daemon): re-parse same-second workflow edits via source_hash by lab1207 · Pull Request #445 · mvschwarz/openrig

lab1207 · 2026-10-02T07:16:50Z

What a user gets

Fixes #415. Before: editing a workflow (or breaking its YAML) within the same timestamp second as the cached scan stayed invisible — the second-resolution mtime check reported the file skipped. After: same-second buckets fall back to the stored SHA-256 source_hash, so edited files re-parse while unchanged files keep skipping.

How you verified it

npx tsc --noEmit in packages/daemon: clean.
npx vitest run test/workflow-spec-folder-scanner.test.ts: 15/16 pass, including 3 new tests (same-second edit re-parses with updated purpose, unchanged same-second file still skips with cached_at untouched, same-second malformed YAML becomes a diagnostic). The 1 failure (scans invalid YAML expects row name bad.yaml, gets the full path) fails identically with and without this change — pre-existing Windows path issue, untouched by this PR.
Could not run: full npm test / npm run lint — better-sqlite3 has no Windows build here and the repo eslint config errors locally (missing migration); repo CI covers both.

Anything you were unsure about

On hash-read failure (file vanished between stat and read) the code falls through to re-parse rather than skip; readThrough then records a diagnostic, same as the pre-existing race behavior.
A stored empty source_hash can never equal a computed SHA-256 hex digest, so legacy diagnostic rows always re-evaluate instead of skipping.
One concern per PR; no version bump; no CHANGELOG.md edit
Tests added or updated where the change is testable
I listed the checks I ran, their results, and any checks I could not run

Summary by CodeRabbit

Bug Fixes
- Workflow specifications that change within the same timestamp second are now detected and reparsed. Unchanged files continue to be skipped, while unreadable or malformed changes are reported as errors.

coderabbitai · 2026-10-02T07:17:19Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5bfaf47b-995d-46e4-9532-006faf5a84f5

📥 Commits

Reviewing files that changed from the base of the PR and between 5887d3b and d72ed80.

📒 Files selected for processing (2)

packages/daemon/src/domain/spec-library-workflow-scanner.ts
packages/daemon/test/workflow-spec-folder-scanner.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Walkthrough

The workflow folder scanner now checks file content when cached and filesystem timestamps fall within the same second. It skips unchanged content and reparses changed or unreadable files. Tests cover changed valid content, unchanged content, and changed malformed content.

Changes

Workflow folder scanning

Layer / File(s)	Summary
Verify content before skipping cached files `packages/daemon/src/domain/spec-library-workflow-scanner.ts`, `packages/daemon/test/workflow-spec-folder-scanner.test.ts`	The scanner retrieves the cached source hash and skips same-second files only when their current content matches. Tests verify that changed content is reparsed, unchanged content is skipped, and changed malformed content produces an error.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: mvschwarz

Merge Risk: ⚪ Minimal · up to d72ed

Same-second workflow edits are checked against cached content. The additional file reads may affect Library list performance, but no material impact requiring a pre-merge fix is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d72ed

Content checks correctly detect previously missed edits, but Library requests now synchronously read and hash unchanged cached workflow files. This can increase daemon-wide resource pressure under repeated requests. The practical impact depends on workflow-folder size and API reachability.

Retained concerns

Medium · security · inferred: Existing Library GET requests now synchronously read and hash every timestamp-eligible cached workflow file, including older unchanged files. Repeated admitted requests can amplify filesystem and hashing work on the daemon's request-processing thread. The traced caller and scanner provide no content-work budget or request coalescing; actual availability impact remains workload- and deployment-dependent.

Security review details

Security Blast Radius

inferred — The evidenced resource-pressure scope is the daemon servicing the configured workflow folder and Library requests. Request parameters do not choose the scan folder in this path. Cross-tenant, cross-service, and deployed network exposure were not established.

Security Findings and Attack Paths

inferred — A client admitted to the Library API can repeatedly trigger full synchronous reads and hashes of timestamp-eligible cached files without changing those files. The introduced amplification is supported by the call path, but a practical denial-of-service outcome and its threshold have not been verified.

Trust Boundaries and Controls

observed — The Library routes sit behind existing Host and browser-Origin checks. For an accepted target Host, those checks can admit an Origin-less request without a bearer token; they are not universal client authentication or work throttling. This control behavior predates the scanner change, and network reachability remains deployment-dependent.

Resilience and Maintainability Implications

inferred — The Library caller catches scan failures, preserving response fallback, but exception handling does not bound successful synchronous reads and hashing. Matching hashes avoid reparsing, which limits some work but does not restore the former content-read-free skip.

Hardening Proposals

proposed — Consider coalescing content validation across requests or moving bounded validation off the request-processing thread while preserving same-second and backdated-edit detection.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: re-parsing workflow edits made within the same second by using source_hash.
Linked Issues check	✅ Passed	Issue [`#415`] requires same-second content edits to be parsed and unchanged files to remain skipped. The scanner now reads `source_hash`, hashes the current file when the cached and file timestamps sha…
Out of Scope Changes check	✅ Passed	The pull request changes only the workflow scanner logic and its focused tests. The scanner change directly implements issue [`#415`]. The tests verify the required changed, unchanged, and malformed-con…

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

fix(daemon): re-parse same-second workflow edits via source_hash

d72ed80

lab1207 mentioned this pull request Oct 2, 2026

fix(daemon): recover repaired diagnostic rows in place #451

Open

3 tasks done

openrig-review mentioned this pull request Oct 2, 2026

fix(workflow): detect content changes in the cached timestamp second #435

Merged

3 tasks done

Navigation Menu

fix(daemon): re-parse same-second workflow edits via source_hash - #445

fix(daemon): re-parse same-second workflow edits via source_hash#445
lab1207 wants to merge 1 commit into
mvschwarz:mainfrom
lab1207:fix/scan-same-second-edits

lab1207 commented Oct 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lab1207 commented Oct 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What a user gets

How you verified it

Anything you were unsure about

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lab1207 commented Oct 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading