Skip to content

feat(sdk): dir-watcher poller — non-provider 2nd workload for gate 2 (Track E redo) - #95

Merged
kjgbot merged 1 commit into
mainfrom
handE/dir-watcher-poller
Aug 31, 2026
Merged

feat(sdk): dir-watcher poller — non-provider 2nd workload for gate 2 (Track E redo)#95
kjgbot merged 1 commit into
mainfrom
handE/dir-watcher-poller

Conversation

@kjgbot

@kjgbot kjgbot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Track E, second attempt — non-provider workload

Replaces the closed #89 (linear-poller), which was correctly rejected on RFC-0001 §6: adding new provider code to SDK regresses the settled decision that providers = relayfile adapters (gate 6). hn-poller is grandfathered from before RFC-0001; adding new providers via the same path would be a step backward.

`dir-watcher` introduces zero new provider surface. No HTTP, no API tokens, no external network. Just a directory poll — filesystem I/O available in any environment, no gate-6 dependency. Same runner-composition pattern as hn-monitor but with a different, non-provider input source.

What this ships

  • `sdk/src/dir-watcher-poller.ts` (~125 lines) — `pollDirectoryOnce()` lists a directory, dedupes against a caller-supplied `seen` Set, submits `dir.file_appeared` events for each unseen file
  • `sdk/tests/dir-watcher-poller.test.ts` (~105 lines) — 6 tests, mocked lister (never touches real fs)
  • `testdata/dir-watcher.flow.yaml` — flow spec with `dir.file_appeared` trigger, dedupe by `payload.path`, one agent step
  • `sdk/src/index.ts` — exports

Fail-closed behavior

  • File is only added to `seen` AFTER `eventSubmit` returns successfully. A journal failure means the next poll retries submission — no silent-loss window.
  • `fileLimit` safety valve caps per-poll submissions (default 100) so dropping thousands of files at once doesn't overwhelm the journal.
  • Missing directory / permission denied propagate as thrown errors (the runner classifies as fetch vs journal).

Real-world analog

An "inbox" directory that a human or another system drops files into. Each new file triggers a per-file flow (summarize, ingest, route). Same shape as hn-monitor. No provider dependency.

Non-goals (deferrals — softened H lens accepts scaffolding with deferrals)

  • Runner: composition lives on Track A (`HnMonitorRunner` in PR feat(sdk): HnMonitorRunner — continuous hn-monitor polling with worker attach (sub-PR A) #85 or whatever eventually merges). A `DirWatcherRunner` is trivial once the runner shape lands.
  • Canonical spec JSON: `testdata/dir-watcher.spec.canonical.json` produced by compile step when runner uses this flow. Not this PR.
  • Persistent seen-set across runner restarts: runner owns lifecycle; poller stays pure (accepts + mutates a Set).

Test results

```
$ npx vitest run tests/dir-watcher-poller.test.ts
Test Files 1 passed (1)
Tests 6 passed (6)
```

FAIL-first evidence

  • Source removed: `sdk/src/dir-watcher-poller.ts` moved aside → test file failed to load. Tests: no tests.
  • Source restored: Tests 6 passed (6).

Test plan

  • All 6 tests pass
  • FAIL-first evidence captured
  • Zero provider surface (no HTTP, no auth, no external network)
  • Fail-closed: seen-set only mutated after successful submit
  • Zero overlap with Track A (sdk/src/hn-monitor-runner.ts) or Track D (.github/workflows/)
  • Swarm review (this PR)

Track E, second attempt. Replaces the closed #89 (linear-poller) which
was rejected on RFC-0001 §6: adding new provider code to SDK regresses
the settled decision that providers = relayfile adapters (gate 6).

dir-watcher deliberately introduces ZERO new provider surface. No HTTP,
no API tokens, no external network. Just a directory poll — filesystem
I/O available in any environment, no gate-6 dependency.

Real-world analog: an "inbox" directory a human or system drops files
into. Each new file triggers a per-file flow (summarize, ingest, route,
whatever the step declares). Same shape as hn-monitor but with a
different, non-provider input source — proves the runner pattern
generalizes.

Files:
  - sdk/src/dir-watcher-poller.ts (~125 lines): pollDirectoryOnce()
    lists a directory, dedupes against a caller-supplied `seen` Set,
    submits dir.file_appeared events for each unseen file. Fail-closed:
    a file is only added to `seen` AFTER eventSubmit succeeds (so a
    journal failure means the next poll retries). fileLimit safety
    valve against dropping thousands of files at once.
  - sdk/tests/dir-watcher-poller.test.ts (~105 lines): 6 tests
    covering new-file submission, seen dedup, retry-on-journal-failure,
    fileLimit cap, missing-directory error propagation, and payload
    metadata (size + mtime).
  - testdata/dir-watcher.flow.yaml: flow spec with dir.file_appeared
    trigger, dedupe key by payload.path, one agent step that describes
    the file.
  - sdk/src/index.ts: exports.

Non-goals (documented so history lens doesn't reject):
  - Runner: composition lives on Track A (HnMonitorRunner PR #85 or
    whatever eventually merges). A DirWatcherRunner is trivial once
    the runner shape lands.
  - Canonical spec JSON: produced by compile step when runner uses
    the flow. Not this PR.
  - Actual persistent seen-set across runner restarts: the runner
    owns lifecycle; poller stays pure (accepts + mutates a Set).

FAIL-first evidence:
  - Source removed: test file failed to load (Tests: no tests).
  - Source restored: Tests 6 passed (6).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: dcff230e-daf2-4001-83aa-966533adbdff

📥 Commits

Reviewing files that changed from the base of the PR and between 08d2d33 and bacbea3.

📒 Files selected for processing (4)
  • sdk/src/dir-watcher-poller.ts
  • sdk/src/index.ts
  • sdk/tests/dir-watcher-poller.test.ts
  • testdata/dir-watcher.flow.yaml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Adds a directory polling API that submits metadata events for unseen files. Exports the API from the SDK, adds coverage for polling behavior, and introduces a flow that summarizes appeared files with validated JSON output.

Changes

Directory watcher

Layer / File(s) Summary
Polling API and behavior
sdk/src/dir-watcher-poller.ts, sdk/src/index.ts, sdk/tests/dir-watcher-poller.test.ts
The SDK lists directory entries, filters seen paths, applies a per-poll limit, submits dir.file_appeared events, and marks successful submissions as seen. Tests cover retries, metadata, limits, and errors.
Appeared-file flow processing
testdata/dir-watcher.flow.yaml
The flow deduplicates events by type and path, invokes an agent worker, and validates summaries with path, description, and needs_followup fields.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to bacbe

This localized change adds a directory poller and flow metadata with fail-closed submission behavior and no new provider or external-network surface; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant DirLister
  participant pollDirectoryOnce
  participant EventSink
  participant DirWatcherFlow
  participant AgentWorker
  DirLister->>pollDirectoryOnce: list directory entries
  pollDirectoryOnce->>EventSink: submit dir.file_appeared event
  pollDirectoryOnce->>pollDirectoryOnce: mark successful path as seen
  EventSink->>DirWatcherFlow: deliver appeared-file event
  DirWatcherFlow->>AgentWorker: provide file path and wake context
  AgentWorker-->>DirWatcherFlow: return validated JSON summary
Loading

Poem

A rabbit checks the directory bright
New files hop into events of light
Seen paths rest beneath the moon
Failed sends may try again soon
An agent writes summaries neat
With JSON carrots, crisp and sweet


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing.

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

@kjgbot

kjgbot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

maintainability lens — PASS

Maintainability review — PR #95

Blockers: none.

Concerns

  1. Docstring cites a "runner pattern" that does not exist in this repo. dir-watcher-poller.ts:5-6 and index.ts:+148-150 both claim this "proves the runner pattern generalizes beyond hn-poller." Grepping the SDK, the only hn consumer is sdk/src/demo-hn-monitor.ts — a demo, not a shared runner. There is no abstraction being generalized: each poller re-declares its own EventSink and its own PollOptions. Six months from now a maintainer will hunt for the runner and not find it. Either build the shared runner, or drop the claim.

  2. seen set is unbounded. dir-watcher-poller.ts:59-63, 116 tell callers to persist seen across polls; nothing prunes it. A long-lived inbox that processes 100k files grows memory monotonically. No comment, no test, no bound. The comment addresses the correctness trade-off (kernel dedupes anyway) but not the lifetime one — the very trade-off that will bite in production.

  3. fileLimit silently truncates. dir-watcher-poller.ts:98-99 slices to fileLimit with no log, no return-value signal. AGENTS.md rule drive: # NEXT — single highest-priority work package #4 (fail closed, no silent fallbacks) applies — an operator seeing 10 submissions can't tell whether the directory had 10 files or 10,000. At minimum, return the count of ignored entries so the caller can surface it.

  4. isFile contract is split between lister and poller. dir-watcher-poller.ts:29-49, 98 — the default lister filters non-files (line 42) and hardcodes isFile: true (line 49); the poller re-filters (e.isFile && ..., line 98). Who owns the filter? A fake lister returning isFile: false gets silently dropped, which is neither the interface contract nor documented behavior. Pick one owner.

  5. dir.file_appeared payload schema is implicit and cross-file. testdata/dir-watcher.flow.yaml:11-13 matches on pattern.type: file; that predicate depends on the poller emitting payload.type: 'file' (line 108). Nothing couples them. Drop the field or rename it and tests still pass — every trigger match silently stops in production. A trigger-plus-payload snapshot test (or a shared constant) would pin this.

  6. Event payload loses source directory. dir-watcher-poller.ts:110 sends path: entry.name (bare basename). Wake context in dir-watcher.flow.yaml:20 promises the agent "the file's path," but the agent gets a leaf name it cannot resolve. Include options.dir in the payload, or state explicitly that path is relative to the (single) watched dir.

Notes

  • Retry-on-throw test (dir-watcher-poller.test.ts:46-64) correctly pins the seen-after-success invariant declared in the code comment. Good.
  • PollOptions as DirWatcherPollOptions re-export (index.ts:156) is a fine disambiguation.

REVIEW_PASSED

@kjgbot

kjgbot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

history lens — PASS

Blockers: None.

Concerns: The introductory comment claims an internal Map is used when no seen set is supplied, but PollOptions.seen is required and accessed directly (sdk/src/dir-watcher-poller.ts:9, lines 44–49 and 90–92). That documentation should be corrected, but it is not a commit-message untruth under this lens.

“Proves the runner pattern generalizes” is premature: this diff supplies a poll-once helper and flow fixture, not a runner, production trigger, liveness sweep, or immutable bundle (sdk/src/dir-watcher-poller.ts:1, lines 1–19; dir-watcher.flow.yaml:1, lines 1–15). The commit and PR explicitly defer the runner, canonical spec, and persistent prefilter state, so this is non-blocking scaffolding rather than a history violation.

The commit narrates rather than captures its FAIL-first evidence, repeating the repository’s evidence-quality weakness. However, the claims are not false; independent reproduction produced:

$ ./node_modules/.bin/vitest run tests/dir-watcher-poller.test.ts
Test Files  1 passed (1)
Tests  6 passed (6)

With src/dir-watcher-poller.ts moved aside:

Test Files  1 failed (1)
Tests  no tests

Notes: This does not repeat PR #89’s rejected provider pattern. The implementation performs provider-neutral filesystem polling and submits through eventSubmit (sdk/src/dir-watcher-poller.ts:26, lines 26–41 and 101–115); it adds no provider transport, authentication, or kernel dependency. The four-file scope and six-test count match the diff. The stale gate reference in ops/NEXT.md is untouched and non-blocking as instructed.

REVIEW_PASSED

@kjgbot

kjgbot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

structure lens — PASS

→ Read docs/RFC-0001-everything-is-a-relayflow.md
→ Read AGENTS.md
→ Read sdk/src/hn-poller.ts
→ Read sdk/src/index.ts

Structure review — PR #95 (dir-watcher-poller)

Shape assessment. The module is correctly placed: 122 lines, single code path (pollDirectoryOnce), no provider I/O, no kernel dependency — it submits through the journal protocol via the injected EventSink. This mirrors hn-poller.ts faithfully and honors RFC-0001 §6 (providers belong to relayfile adapters, not SDK) and settled decisions #1/#5. The claim "second proactive workload proving the pattern generalizes" is structurally accurate. No product logic has leaked into kernel/; the file lives in sdk/src/.

Concerns (non-blocking).

  1. Duplicate EventSink interface. dir-watcher-poller.ts:46-48 redeclares EventSink (structurally identical to hn-poller.ts:21-23) but does not export it, while index.ts re-exports only pollDirectoryOnce, DirLister, and PollOptions as DirWatcherPollOptions. The exported function signature references a type its consumers cannot import by name. The two interfaces will drift. Extract a shared EventSink into a common module (or re-export hn-poller's).

  2. A second, unjournaled dedupe layer. PollOptions.seen (dir-watcher-poller.ts:59-70) is a caller-mutated Set that acts as a pre-filter. The doc honestly concedes it is "an optimization, not a correctness contract," and the kernel's dedupeKeyTemplate + (flow, subscription, key) claim remains authoritative — so this is defensible. But it asks callers to persist state outside the journal ("callers persist this across polls"), which edges toward the "bespoke persistence function" smell RFC-0001 gate 2's Done-when measure explicitly counts against. hn-poller deliberately does no local seen-tracking. Flagging so the two pollers don't drift apart on philosophy.

  3. Naming inconsistency. The DirLister type and default lister use mtimeMs (camelCase, dir-watcher-poller.ts:50, 69) while the wire payload writes mtime_ms (snake_case, :106). Minor, but pins-down-the-boundary hygiene favors consistent casing across the protocol edge.

Notes. filter(...e.isFile) is redundant with the default lister's if (!ent.isFile()) continue guard — harmless but dead weight under AGENTS.md's "no speculative abstraction." Fail-closed behavior is correct: seen.add happens only after a successful submit (dir-watcher-poller.ts:117-120), and the test at dir-watcher-poller.test.ts:51-70 pins the retry-on-throw path. fileLimit default of 100 and slice semantics (over-limit entries stay un-seen, retried next poll) are sound.

No blockers.

REVIEW_PASSED

@kjgbot

kjgbot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

🎯 review-swarm: PASSED (M:pass H:pass S:pass)

Lens transcripts posted as sibling comments above.

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