[kridaydave]

triggerNow now reports skip result honestly.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c31dbfba-9c68-4698-8ba0-2a5c3e1f8969

📥 Commits

Reviewing files that changed from the base of the PR and between 77e64ad and 90c87ff.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/extensions/scheduler/auto-organize.service.ts
• tests/unit/services/auto-organize.test.ts

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

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[kridaydave]

Reviewed locally on k5/debt-trigger-result. Built + npm test auto-organize (21 pass), tsc clean, lint clean. No blockers found. Change is contained (no external triggerNow callers). Summary: runOrganization/triggerNow now report PassOutcome (ran vs skipped not-watched/in-flight); guard semantics unchanged. Suggested nits: document raw-direct echo on not-watched; note new exported scheduler types in docs if they become public API. Approve-equivalent: no changes requested.

[kridaydave]

Review

Verdict: approve. No blockers found.

Correctness

• runOrganization returns { status: "ran" } only after the finally has released the in-flight key, and the skip branch returns before this.inFlight.set(...), so a skip can never clear a directory that is genuinely in flight. src/extensions/scheduler/auto-organize.service.ts:330 and :402.
• triggerNow is now a pure pass-through of runOrganization's outcome. Verified locally by restoring the old unconditional-success shape: 6 of 21 tests go red, so the new assertions are load-bearing rather than vacuous.
• not-watched echoes the caller's own string rather than a validated path. That is the right call (nothing was validated, and inventing one would be a lie) and the comment says so — worth keeping in mind if this value is ever put on an error path, where sanitizeErrorMessage rules apply.

API / contract

• triggerNow/runOrganization went Promise<boolean> → Promise<PassOutcome>. I grepped every caller: the cron retry loop and _runMissedSchedulesInternal both discard the return value, watch-cli.ts never calls it, and src/index.ts exports nothing, so there is no consumer outside this class and its tests. No wire-visible contract (no tool, no API.md, no inputSchema/outputSchema) is touched, so docs:check stays green — confirmed, it passes.

Tests

• The corrected "already in flight" test no longer stubs its way past the guard it names, and the new no-stub test observes the refusal three ways (outcome, sweep not reached, seeded counter survives). The getStatus().runningDirectories assertion is exactly the kind that catches a finally leaking into the skip path — good.
• beforeEach/afterEach mutating CONFIG.paths.customAllowed is acceptable here: it is restored in afterEach, the test file is the only mutator, and Jest isolates module registries per file. Slightly unusual, but the comment explains why it exists.

Verification I ran

• npm test tests/unit/services/auto-organize.test.ts — 21/21 pass
• npm run build, npm run typecheck:tests — clean
• npx eslint on both changed files — clean
• npm run docs:check — passes

Residual risk (not blocking)

• The two loops that call runOrganization still ignore the outcome, so a cron tick that loses its directory to a manual trigger produces only a debug line. The PR body's comment documents why the retry is skipped; whether a skip deserves a warn is a logging policy call, not a correctness one.
• The new behavior is proven at the unit level only. It is not reachable through the stdio harness (verify-file-organizer has no scheduler capability), so there is no real-server proof available for this class of change — noting it so the gap is visible rather than implied to be covered.