[Sprint 16] Clear tsc + patch 0.1.1 - #4
Conversation
Align S3/Archive/cron/gzip APIs; add missing/prepend to contract; patch 0.1.1. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe package version was bumped to 0.1.1. Filesystem contracts gained visibility and write operations, adapters and decorators were updated, S3 handling was refined, archives moved to in-memory Bun APIs, scheduling was centralized, and TypeScript configuration was adjusted. ChangesFilesystem API and adapters
Runtime behavior
Project tooling
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/adapters/s3-adapter.ts (1)
78-95: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftApply
defaultAclin the regular upload path.
S3AdapterConfig.aclis documented as the default ACL, butput()writes withclient.file(key).write(contents)and never uses this value. Bun’s S3 write API supportsacl, so passthis.defaultAclinto the write options when present, including forappend()/prepend()upload paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/s3-adapter.ts` around lines 78 - 95, Update the S3Adapter upload flows in put() and the append()/prepend() paths to pass this.defaultAcl through Bun’s S3 write options when it is defined, while preserving existing behavior when no ACL is configured.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/local-adapter.ts`:
- Around line 129-132: Update LocalAdapter.prepend to serialize the get-and-put
sequence per targetPath, reusing the existing per-path write coordination used
by other write operations if available. Ensure concurrent prepends to the same
path execute sequentially so both prefixes are retained, while preserving
independent concurrency for different paths and the current return behavior.
In `@src/adapters/s3-adapter.ts`:
- Line 652: Update the directory-prefix computation in the S3 listing logic
around dirName so nested keys preserve their full directory path relative to the
requested directory, rather than using only parts[0]. Ensure keys like
foo/bar/file.txt emit foo/bar, while retaining the existing undefined guard and
avoiding prefixes outside the requested directory.
- Around line 467-468: Update the URL construction in the S3 adapter method
containing endpoint and bucket interpolation to normalize trailing slashes on
endpoint and URL-encode each segment of key while preserving its slash
separators. Keep the bucket and key path structure intact so special characters
cannot alter the URL path or become query/fragment delimiters.
- Line 627: Update scanDirectory() to paginate S3 list responses: process each
page’s result.contents, then reuse result.nextContinuationToken as
continuationToken while result.isTruncated is true. Preserve the existing
object-processing behavior so files(), allFiles(), and directory discovery
include every page.
In `@src/contracts/filesystem.ts`:
- Around line 155-158: Update S3Adapter.getVisibility() to use the provided
path, check whether the object exists, and retrieve its S3 ACL so it returns the
actual "private" or "public" visibility instead of defaultAcl. Update
S3Adapter.setVisibility() to avoid reporting success while performing no ACL
change; either apply the requested S3 ACL or return the established
unsupported-operation result.
In `@src/decorators/compressed-adapter.ts`:
- Around line 106-110: The string-based FilesystemDisk read corrupts binary data
before decompression or archiving. Add a byte-exact read method to the disk
contract and its adapters, then update CompressedAdapter.get() in
src/decorators/compressed-adapter.ts, archive.createFromDirectory() at
src/utils/archive.ts:61-71, and archive.createBlob() at
src/utils/archive.ts:108-118 to use it when reading compressed files or archive
entries.
- Around line 3-7: Restrict the GzipLevel type in compressed-adapter.ts to the
valid zlib compression levels -1 through 9, removing 10, 11, and 12 so level
options passed to Bun.gzipSync and put() cannot represent unsupported values.
In `@src/decorators/watched-adapter.ts`:
- Around line 130-154: Update the watcher logic around the debounceTimer in the
watch setup to maintain separate debounce timers for each watched path and
filename, so events for different files do not replace one another. When
unwatching a path or all watchers, clear and remove the associated pending
timers before removing callbacks/watchers, preventing delivery to removed
callbacks.
In `@src/utils/scheduler.ts`:
- Around line 215-224: Update the backup method’s catch block to log the caught
failure with the scheduler’s existing logging mechanism, then rethrow it so
backup failures remain observable to scheduled maintenance callers; do not leave
the error silently swallowed.
- Around line 127-167: Update registerJob to reject unsupported cron expressions
when cronToInterval returns no interval instead of returning an inactive
CronJob, and wrap Bun.cron invocation failures for invalid expressions with a
clear error. Also ensure the project either constrains engines.bun and
packageManager to Bun >=1.3.12 or replaces the 2-argument Bun.cron call with a
compatible API path.
---
Outside diff comments:
In `@src/adapters/s3-adapter.ts`:
- Around line 78-95: Update the S3Adapter upload flows in put() and the
append()/prepend() paths to pass this.defaultAcl through Bun’s S3 write options
when it is defined, while preserving existing behavior when no ACL is
configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a766af8-a8de-4691-bf36-3f280be4e4a7
📒 Files selected for processing (13)
jsr.jsonpackage.jsonsrc/adapters/local-adapter.tssrc/adapters/s3-adapter.tssrc/contracts/filesystem.tssrc/decorators/compressed-adapter.tssrc/decorators/scoped-adapter.tssrc/decorators/watched-adapter.tssrc/manager.tssrc/testing/helpers.tssrc/utils/archive.tssrc/utils/scheduler.tstsconfig.json
| async prepend(targetPath: string, data: string): Promise<boolean> { | ||
| try { | ||
| const existing = (await this.get(targetPath)) ?? ""; | ||
| return await this.put(targetPath, data + existing); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize prepend writes to prevent lost updates.
get() followed by put() is not atomic. Concurrent prepend() calls can read the same old contents, then the last write silently discards the other prefix. Add per-path serialization shared with other write operations, or use an atomic filesystem primitive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/local-adapter.ts` around lines 129 - 132, Update
LocalAdapter.prepend to serialize the get-and-put sequence per targetPath,
reusing the existing per-path write coordination used by other write operations
if available. Ensure concurrent prepends to the same path execute sequentially
so both prefixes are retained, while preserving independent concurrency for
different paths and the current return behavior.
| const endpoint = this.endpoint || `https://s3.${this.region}.amazonaws.com`; | ||
| return `${endpoint}/${this.bucket}/${key}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching s3-adapter:"
fd -a 's3-adapter\.(ts|js)$' . || true
file="$(fd 's3-adapter\.(ts|js)$' . | head -n 1)"
if [ -n "${file:-}" ]; then
echo "Selected: $file"
wc -l "$file"
echo "Outline:"
ast-grep outline "$file" || true
echo "Relevant lines 430-490:"
sed -n '430,490p' "$file" | nl -ba -v430
fi
echo
echo "S3 key/url related usages:"
rg -n "getObjectUrl|endpoint|bucket|/\\$\\{key\\}|url|sign|presign" "$file" 2>/dev/null || true
echo
echo "Search URL encoding across repo:"
rg -n "encodeURI|encodeURIComponent|URLSearchParams|object keys|objectKey|S3" -S . || trueRepository: nino-ts/filesystem
Length of output: 1017
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="./src/adapters/s3-adapter.ts"
echo "Relevant lines 430-490:"
sed -n '430,490p' "$file"
echo
echo "All occurrences of generated URL expression:"
rg -n "endpoint.*bucket|bucket.*key|getObjectUrl|temporary|presigned|\\$\\{key\\}|\\$\\{endpoint\\}" "$file"
echo
echo "Read-only verifier: extract key interpolation expressions and detect simple URI segment encoding pattern"
python3 - <<'PY'
from pathlib import Path
import re
p = Path('./src/adapters/s3-adapter.ts').read_text()
m = re.search(r'const endpoint = this\.endpoint \|\| `https://s3\.\$\{this\.region\}\.amazonaws\.com`;\s*return `[^`]*(?:\$\{\w+\})+`', p)
print("Found endpoint/url expression:", bool(m))
if m:
start = max(0, p.rfind('\n', 0, m.start()) + 1)
line_no = p.count('\n', 0, m.start()) + 1
print(f"line {line_no}: {m.group(0)}")
PYRepository: nino-ts/filesystem
Length of output: 2738
URL-encode S3 object keys.
S3 keys may contain spaces, ?, #, and other characters requiring URL encoding; interpolating key directly can address a different object or create a query/fragment instead. Encode each path segment while preserving /, and normalize endpoint slashes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/s3-adapter.ts` around lines 467 - 468, Update the URL
construction in the S3 adapter method containing endpoint and bucket
interpolation to normalize trailing slashes on endpoint and URL-encode each
segment of key while preserving its slash separators. Keep the bucket and key
path structure intact so special characters cannot alter the URL path or become
query/fragment delimiters.
Source: MCP tools
| const results: string[] = []; | ||
|
|
||
| for (const obj of result.objects || []) { | ||
| for (const obj of result.contents ?? []) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Paginate truncated S3 listings.
result.contents is only the first 1,000-object page. scanDirectory() currently ignores isTruncated/nextContinuationToken, so files(), allFiles(), and directory discovery can silently omit later objects. Loop with continuationToken = response.nextContinuationToken until isTruncated is false.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/s3-adapter.ts` at line 627, Update scanDirectory() to paginate
S3 list responses: process each page’s result.contents, then reuse
result.nextContinuationToken as continuationToken while result.isTruncated is
true. Preserve the existing object-processing behavior so files(), allFiles(),
and directory discovery include every page.
| if (parts.length > 1) { | ||
| const dirName = parts[0]; | ||
| if (!results.includes(dirName)) { | ||
| if (dirName !== undefined && !results.includes(dirName)) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve nested directory paths.
The new undefined guard is safe, but dirName = parts[0] still emits only the first key component. For example, foo/bar/file.txt yields foo, not foo/bar; recursive listings therefore omit nested directories and scoped listings return incorrect paths. Emit directory prefixes relative to the requested directory.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/s3-adapter.ts` at line 652, Update the directory-prefix
computation in the S3 listing logic around dirName so nested keys preserve their
full directory path relative to the requested directory, rather than using only
parts[0]. Ensure keys like foo/bar/file.txt emit foo/bar, while retaining the
existing undefined guard and avoiding prefixes outside the requested directory.
| /** | ||
| * Get the visibility of a file (public/private). | ||
| */ | ||
| getVisibility(path: string): Promise<string | null>; | ||
| getVisibility(path: string): Promise<FileVisibility | null>; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate filesystem-related files =="
git ls-files | rg 'src/contracts/filesystem\.ts$|filesystem|storage|s3|adapter|storage' | head -200
echo
echo "== outline src/contracts/filesystem.ts =="
if [ -f src/contracts/filesystem.ts ]; then
ast-grep outline src/contracts/filesystem.ts || true
wc -l src/contracts/filesystem.ts
fi
echo
echo "== relevant contract section =="
if [ -f src/contracts/filesystem.ts ]; then
sed -n '120,190p' src/contracts/filesystem.ts
fi
echo
echo "== search getVisibility/setVisibility implementations/usages =="
rg -n "getVisibility|setVisibility|FileVisibility|setPublic|SetPublic|FilesystemDisk" src || trueRepository: nino-ts/filesystem
Length of output: 10735
🏁 Script executed:
# The previous shell was a capability request only; results are not present.Repository: nino-ts/filesystem
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== S3 adapter visibility methods =="
sed -n '360,450p' src/adapters/s3-adapter.ts
echo
echo "== Memory adapter visibility methods =="
sed -n '260,320p' src/adapters/memory-adapter.ts
echo
echo "== Local adapter visibility methods =="
sed -n '220,255p' src/adapters/local-adapter.ts
echo
echo "== S3 tests visibility-related content =="
rg -n -C 5 "getVisibility|setVisibility|visibility|acl|aclObject|ACL" tests/unit/s3-adapter.test.ts
echo
echo "== package deps relevant =="
if [ -f package.json ]; then jq '.dependencies, .devDependencies | {aws, s3}' package.json; fiRepository: nino-ts/filesystem
Length of output: 7713
🏁 Script executed:
# The previous shell was a capability request only; results are not present.Repository: nino-ts/filesystem
Length of output: 156
Make S3 visibility methods honor the visibility contract.
S3Adapter.getVisibility() ignores the requested path and defaults every object to the configured defaultAcl; it should check object existence and read/report the object’s ACL before returning "private" or "public". Also avoid returning success from setVisibility() when it only documents a no-op and does not update S3 ACL state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/contracts/filesystem.ts` around lines 155 - 158, Update
S3Adapter.getVisibility() to use the provided path, check whether the object
exists, and retrieve its S3 ACL so it returns the actual "private" or "public"
visibility instead of defaultAcl. Update S3Adapter.setVisibility() to avoid
reporting success while performing no ACL change; either apply the requested S3
ACL or return the established unsupported-operation result.
| /** | ||
| * Bun gzip compression level (zlib / libdeflate). | ||
| */ | ||
| export type GzipLevel = -1 | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
curl -fsSL https://bun.sh/docs/runtime/utils | grep -F 'Must be between `-1` and `9`'
rg -n -C2 'type GzipLevel|gzipSync' src/decorators/compressed-adapter.tsRepository: nino-ts/filesystem
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant occurrences =="
rg -n -C3 'type GzipLevel|gzipSync|gunzipSync|level.*GzipLevel|GzipLevel' src/decorators/compressed-adapter.ts
echo
echo "== targeted docs search =="
curl -fsSL https://bun.sh/docs/runtime/utils | sed -n '/gzipSync/,+60p' | sed -n '1,120p'
echo
echo "== file section =="
sed -n '1,90p;120,170p' src/decorators/compressed-adapter.tsRepository: nino-ts/filesystem
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant occurrences =="
rg -n -C3 'type GzipLevel|gzipSync|gunzipSync|level.*GzipLevel|GzipLevel' src/decorators/compressed-adapter.ts
echo
echo "== targeted docs search =="
curl -fsSL https://bun.sh/docs/runtime/utils | sed -n '/gzipSync/,+80p' | sed -n '1,160p'
echo
echo "== file section =="
sed -n '1,100p;120,180p' src/decorators/compressed-adapter.tsRepository: nino-ts/filesystem
Length of output: 50375
🌐 Web query:
Bun gzipSync level option must be between -1 and 9 Bun.gzipSync docs
💡 Result:
The level option for Bun.gzipSync must be between -1 and 9 [1][2][3]. According to the official Bun documentation, the compression level is defined within the ZlibCompressionOptions interface as follows: - A value of -1 uses the default compression level (currently 6) [1][2][3]. - A value of 0 provides no compression [1][2][3]. - A value of 1 offers the least compression and the fastest speed [1][2][3]. - A value of 9 offers the best compression and the slowest speed [1][2][3]. The valid range for the level option is -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, or 9 [1][3].
Citations:
- 1: https://bun.com/reference/bun/gzipSync
- 2: https://bun.com/reference/bun/ZlibCompressionOptions
- 3: https://bun.com/docs/runtime/utils
Restrict GzipLevel to -1..9.
Bun.gzipSync only accepts zlib levels -1 through 9; including 10..12 makes invalid call options available through level?: GzipLevel, and these values can fail put() via the try/catch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/decorators/compressed-adapter.ts` around lines 3 - 7, Restrict the
GzipLevel type in compressed-adapter.ts to the valid zlib compression levels -1
through 9, removing 10, 11, and 12 so level options passed to Bun.gzipSync and
put() cannot represent unsupported values.
| const data = await this.adapter.get(compressedPath); | ||
| if (data) { | ||
| try { | ||
| const decompressed = await Bun.gunzip(data); | ||
| const decompressed = Bun.gunzipSync(data); | ||
| return new TextDecoder().decode(decompressed); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files around candidate names:"
fd -a 'compressed-adapter|archive|filesystem' . | sed 's#^\./##' | head -200
echo
echo "Inspect compressed-adapter outline:"
ast-grep outline src/decorators/compressed-adapter.ts --view expanded || true
echo
echo "Compressed-adapter relevant lines:"
cat -n src/decorators/compressed-adapter.ts | sed -n '1,160p'
echo
echo "Archive outline:"
ast-grep outline src/utils/archive.ts --view expanded || true
echo
echo "Archive relevant lines:"
cat -n src/utils/archive.ts | sed -n '1,150p'Repository: nino-ts/filesystem
Length of output: 14360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Filesystem contract:"
cat -n src/contracts/filesystem.ts | sed -n '1,260p'
echo
echo "Search for FilesystemDisk get/getBytes/getBytes/bytes implementations and related adapters:"
rg -n "interface FilesystemDisk|class .*FilesystemDisk|implements FilesystemDisk|get\\(|getBytes\\(|bytes\\(|async get\\(|Promise\\.Uint8Array|Uint8Array|string \\| null" src -S
echo
echo "Try detecting static type shape:"
python3 - <<'PY'
from pathlib import Path
for p in Path("src").rglob("*.ts"):
s=p.read_text()
if "class " in s or "interface FilesystemDisk" in s:
print(f"\n--- {p} ---")
for i,line in enumerate(s.splitlines(),1):
if "get(" in line or "interface FilesystemDisk" in line or "implements FilesystemDisk" in line or "bytes(" in line or "getBytes(" in line or "Promise<" in line:
print(f"{i}: {line[:220]}")
PY
echo
echo "Node text binary probe matching compressor flow:"
node - <<'JS'
const zlib = require('zlib');
const data = new Uint8Array([0xCA, 0xFE, 0xBA, 0xBE, 0xFF]);
const compressed = zlib.gzipSync(data);
const text = Buffer.from(compressed).toString('utf8');
const input = Buffer.from(text, 'utf8');
const decompressed = zlib.gunzipSync(input);
console.log(JSON.stringify({
original: Array.from(data),
textBytes: Array.from(input),
decompressed: Array.from(decompressed),
equalBufferToStringBuffer: Buffer.compare(compressed, input) === 0,
equalOriginalToDecompressed: Buffer.compare(data, decompressed) === 0,
}));
JSRepository: nino-ts/filesystem
Length of output: 36635
Add a byte-exact filesystem read path. FilesystemDisk.get(): Promise<string | null> is used for binary round-trips, so compressed bytes survive UTF-8 text encoding/decoding and gunzipSync sees corrupted output. Add a byte API to the contract/adapters and use it at CompressedAdapter.get(), archive.createFromDirectory(), and archive.createBlob() before decompressing or building archive entries.
📍 Affects 2 files
src/decorators/compressed-adapter.ts#L106-L110(this comment)src/utils/archive.ts#L61-L71src/utils/archive.ts#L108-L118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/decorators/compressed-adapter.ts` around lines 106 - 110, The
string-based FilesystemDisk read corrupts binary data before decompression or
archiving. Add a byte-exact read method to the disk contract and its adapters,
then update CompressedAdapter.get() in src/decorators/compressed-adapter.ts,
archive.createFromDirectory() at src/utils/archive.ts:61-71, and
archive.createBlob() at src/utils/archive.ts:108-118 to use it when reading
compressed files or archive entries.
| let debounceTimer: ReturnType<typeof setTimeout> | null = null; | ||
|
|
||
| const watcher = watch(path, { recursive: this.recursive }, (event, filename) => { | ||
| if (!filename) { | ||
| return; | ||
| } | ||
|
|
||
| // Debounce | ||
| const callbacks = this.callbacks.get(path); | ||
| if (!callbacks) { | ||
| return; | ||
| } | ||
|
|
||
| const eventType = event === "change" ? "change" : "rename"; | ||
|
|
||
| // Call all callbacks | ||
| for (const callback of callbacks) { | ||
| try { | ||
| callback(eventType, filename); | ||
| } catch (_error) {} | ||
| if (debounceTimer !== null) { | ||
| clearTimeout(debounceTimer); | ||
| } | ||
|
|
||
| debounceTimer = setTimeout(() => { | ||
| for (const callback of callbacks) { | ||
| try { | ||
| callback(eventType, filename); | ||
| } catch (_error) {} | ||
| } | ||
| }, this.debounce); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Debounce per filename and cancel pending delivery on unwatch.
One timer per watched directory drops earlier events when different files change within the debounce window. It also survives unwatch()/unwatchAll(), so removed callbacks can still run. Track timers by watched path and filename, then clear them when the watcher is removed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/decorators/watched-adapter.ts` around lines 130 - 154, Update the watcher
logic around the debounceTimer in the watch setup to maintain separate debounce
timers for each watched path and filename, so events for different files do not
replace one another. When unwatching a path or all watchers, clear and remove
the associated pending timers before removing callbacks/watchers, preventing
delivery to removed callbacks.
| private registerJob(jobId: string, cronExpression: string, callback: CronJobCallback): CronJob { | ||
| let running = false; | ||
| let bunJob: import("bun").CronJob | null = null; | ||
| let timerId: ReturnType<typeof setInterval> | null = null; | ||
|
|
||
| const job: CronJob = { | ||
| isRunning: () => running, | ||
| start: () => { | ||
| if (running) { | ||
| return; | ||
| } | ||
|
|
||
| if (typeof Bun.cron === "function") { | ||
| bunJob = Bun.cron(cronExpression, callback); | ||
| running = true; | ||
| this.jobs.set(jobId, job); | ||
| return; | ||
| } | ||
|
|
||
| const interval = this.cronToInterval(cronExpression); | ||
| if (interval) { | ||
| timerId = setInterval(callback, interval); | ||
| running = true; | ||
| this.jobs.set(jobId, job); | ||
| } | ||
| }, | ||
| stop: () => { | ||
| bunJob?.stop(); | ||
| bunJob = null; | ||
| if (timerId !== null) { | ||
| clearInterval(timerId); | ||
| timerId = null; | ||
| } | ||
| running = false; | ||
| this.jobs.delete(jobId); | ||
| }, | ||
| }; | ||
|
|
||
| job.start(); | ||
| return job; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check declared Bun version/engine constraints across the repo
fd -a 'package.json' -d 1 --exec cat {}
fd -a 'bunfig.toml' --exec cat {}
fd -a '.bun-version' --exec cat {}
rg -n '"bun"' package.json 2>/dev/nullRepository: nino-ts/filesystem
Length of output: 1783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files | rg '(^|/)src/utils/scheduler\.ts$|scheduler'
printf '\n--- scheduler outline ---\n'
ast-grep outline src/utils/scheduler.ts --view compact || true
printf '\n--- relevant scheduler lines ---\n'
cat -n src/utils/scheduler.ts | sed -n '1,240p'
printf '\n--- Bun cron parse/overload mentions in repo ---\n'
rg -n 'Bun\.cron|cronToInterval|scheduleCleanup|scheduleBackup|schedule\(' src tests || true
printf '\n--- package runtime deps/types ---\n'
node - <<'JS'
const fs=require('fs');
const p=require('./package.json');
console.log(JSON.stringify({engines:p.engines,packageManager:p.packageManager,dependencies:p.dependencies,devDependencies:p.devDependencies}, null, 2));
JSRepository: nino-ts/filesystem
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- files ---'
git ls-files | rg '(^|/)src/utils/scheduler\.ts$|scheduler' || true
printf '%s\n' ''
printf '%s\n' '--- scheduler outline ---'
ast-grep outline src/utils/scheduler.ts --view compact || true
printf '%s\n' ''
printf '%s\n' '--- relevant scheduler lines ---'
cat -n src/utils/scheduler.ts | sed -n '1,240p' || true
printf '%s\n' ''
printf '%s\n' '--- Bun cron parse/overload mentions in repo ---'
rg -n 'Bun\.cron|cronToInterval|scheduleCleanup|scheduleBackup|schedule\(' src tests || true
printf '%s\n' ''
printf '%s\n' '--- package runtime deps/types ---'
node - <<'JS'
const fs=require('fs');
const p=require('./package.json');
const keys = ['engines','packageManager','dependencies','devDependencies'];
console.log(JSON.stringify(keys.reduce((o,k)=>({...o,[k]:p[k]}),{}), null, 2));
JSRepository: nino-ts/filesystem
Length of output: 10439
🌐 Web query:
Bun cron API Bun.cron.parse Bun.cron throws TypeError invalid expression Bun.cron overload Bun.cron(path, expression, title) Bun.cron(expression, callback)
💡 Result:
The Bun.cron API allows for in-process cron scheduling and the registration of OS-level cron jobs [1][2][3]. API Overloads Bun.cron uses the following main overloads: 1. In-process scheduling: Bun.cron(schedule, handler) [1][2] - schedule: A string representing a 5-field cron expression or a predefined nickname (e.g., "@hourly") [1][4]. - handler: A callback function executed on each fire [1][2]. It returns a CronJob object synchronously [2][5]. 2. OS-level job registration: Bun.cron(path, schedule, title) [1][2] - path: Path to the script (resolved relative to the caller) [1][2]. - schedule: Cron expression or nickname [2]. - title: A unique alphanumeric identifier (can include hyphens and underscores) [1][2]. - This returns a Promise [2][6][7]. Bun.cron.parse The Bun.cron.parse(expression, relativeDate?) function parses a cron expression and returns the next matching UTC Date (or null if no match exists within approximately 4 years) [2][8][3]. Error Handling Bun.cron throws a TypeError if the provided cron expression is invalid (e.g., syntactically incorrect) or represents a time that will never occur (e.g., "0 0 30 2 *") [2][5]. For OS-level jobs, if a provided cron pattern is incompatible with the host operating system's constraints, the function may reject with an error message [5].
Citations:
- 1: https://bun.com/docs/runtime/cron
- 2: https://bun.sh/docs/runtime/cron
- 3: https://bun.com/blog/release-notes/bun-v1.3.11
- 4: https://bun.com/reference/bun/CronWithAutocomplete
- 5: https://github.com/oven-sh/bun/blob/88417471/docs/runtime/cron.mdx
- 6: https://github.com/oven-sh/bun/blob/7e57e529/src/bun.js/api/cron.zig
- 7: feat(cron): add Bun.cron API for OS-level cron jobs and expression parsing oven-sh/bun#26999
- 8: https://github.com/oven-sh/bun/blob/7abe6c38/docs/runtime/cron.mdx
🌐 Web query:
Bun cron callback overload added version Bun.cron(Bun.script(...), expression, title) schedule handler 1.3.12 1.3.13
💡 Result:
The in-process callback overload for Bun.cron was introduced in Bun v1.3.12 [1][2][3]. This feature allows you to run a function on a cron schedule directly within your application process, sharing state (such as database pools, caches, and module-level variables) with the rest of your code [1][3]. It complements the existing OS-level Bun.cron(path, schedule, title) API, which handles persistent, external cron jobs via system schedulers like crontab, launchd, or Task Scheduler [1][4]. Key characteristics of the in-process Bun.cron(schedule, handler) overload include: No-overlap guarantee: The next execution is scheduled only after the handler (and any returned Promise) has settled [1][4]. If a task takes longer than its scheduled interval, it will not trigger concurrent runs; instead, it will resume at the next available interval after completion [4][5]. Error handling: Synchronous errors in the handler emit uncaughtException, and rejected Promises emit unhandledRejection [1][6]. The job will reschedule itself after an error, provided the process does not exit [1][7]. Lifecycle Management: The function returns a CronJob object which can be managed using.ref or.unref, and supports the Disposable interface (e.g., using job = Bun.cron(...)) to automatically stop the job when the scope is exited [1][4]. Compatibility: It is --hot safe, meaning in-process cron jobs are automatically cleared before the module graph is re-evaluated, preventing timer leaks during hot reloads [1][6]. Time Zones: While initially scheduled in UTC, subsequent updates (following v1.3.12) added support for interpreting schedules in the system's local time zone or specific IANA time zones via an options argument [8][5]. The API signature is Bun.cron(schedule: string, handler: (this: CronJob) => unknown) [4][9]. As of version 1.3.13, this functionality remains core to the Bun.cron runtime API [1][2].
Citations:
- 1: https://bun.com/blog/release-notes/bun-v1.3.12
- 2: https://bun.sh/blog/bun-v1.3.12
- 3: https://typescript.news/articles/2026-04-13-bun-1-3-12-webview-browser-automation-using-await-using
- 4: https://bun.sh/docs/runtime/cron
- 5: https://bun.com/docs/runtime/cron
- 6: https://bun.com/blog/bun-v1.3.12
- 7: Implement in-process callback-style Bun.cron oven-sh/bun#28701
- 8: cron: interpret Bun.cron.parse() and in-process schedules in local time; add { tz } option oven-sh/bun#35122
- 9: https://bun.com/reference/bun/cron
Avoid scheduling unsupported expressions silently and constrain Bun.cron versions.
registerJob can return a valid-looking CronJob that never runs when Bun’s interval fallback hits an unsupported cronToInterval case, and if Bun.cron is present on older Bun engines the 2-argument callback overload may actually be the 3-argument OS-level signature instead, causing cronExpression to be treated as a script path. Raise a clear error for unsupported/pending fallbacks, wrap Bun.cron(...) invalid-expression cases, and tighten engines.bun/packageManager to Bun >=1.3.12 or stop calling the 2-argument overload.
[low_effort和high_reward]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/scheduler.ts` around lines 127 - 167, Update registerJob to reject
unsupported cron expressions when cronToInterval returns no interval instead of
returning an inactive CronJob, and wrap Bun.cron invocation failures for invalid
expressions with a clear error. Also ensure the project either constrains
engines.bun and packageManager to Bun >=1.3.12 or replaces the 2-argument
Bun.cron call with a compatible API path.
| private async backup(source: string, destination: string): Promise<void> { | ||
| try { | ||
| // Import ArchiveUtils dynamically to avoid circular dependency | ||
| const { ArchiveUtils } = await import("./archive"); | ||
|
|
||
| const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); | ||
| const backupPath = `${destination}_${timestamp}.tar`; | ||
|
|
||
| const success = await ArchiveUtils.createFromDirectory(backupPath, source, this.adapter); | ||
|
|
||
| if (success) { | ||
| } else { | ||
| } | ||
| await ArchiveUtils.createFromDirectory(backupPath, source, this.adapter); | ||
| } catch (_error) {} | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
backup() silently swallows all failures.
The empty catch (_error) {} means a failed backup (bad source, archive write failure, permissions, etc.) produces no signal whatsoever — no log, no thrown error, no return value to inspect. Combined with cleanup()'s identical pattern (line 209), scheduled maintenance jobs can silently stop working indefinitely. Bun's own cron guidance in the community docs notes that Swallowing errors with an empty catch makes production debugging very difficult. Re-throwing after logging is the right pattern.
🛠️ Suggested minimal fix
- } catch (_error) {}
+ } catch (error) {
+ console.error(`Backup failed for "${source}" -> "${destination}":`, error);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async backup(source: string, destination: string): Promise<void> { | |
| try { | |
| // Import ArchiveUtils dynamically to avoid circular dependency | |
| const { ArchiveUtils } = await import("./archive"); | |
| const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); | |
| const backupPath = `${destination}_${timestamp}.tar`; | |
| const success = await ArchiveUtils.createFromDirectory(backupPath, source, this.adapter); | |
| if (success) { | |
| } else { | |
| } | |
| await ArchiveUtils.createFromDirectory(backupPath, source, this.adapter); | |
| } catch (_error) {} | |
| } | |
| private async backup(source: string, destination: string): Promise<void> { | |
| try { | |
| const { ArchiveUtils } = await import("./archive"); | |
| const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); | |
| const backupPath = `${destination}_${timestamp}.tar`; | |
| await ArchiveUtils.createFromDirectory(backupPath, source, this.adapter); | |
| } catch (error) { | |
| console.error(`Backup failed for "${source}" -> "${destination}":`, error); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/scheduler.ts` around lines 215 - 224, Update the backup method’s
catch block to log the caught failure with the scheduler’s existing logging
mechanism, then rethrow it so backup failures remain observable to scheduled
maintenance callers; do not leave the error silently swallowed.
Summary
tsc --noEmiterrors under TS7 / Bun 1.3Test plan
bunx tsc --noEmitexit 0bun test102 pass@ninots/filesystem@0.1.1Summary by CodeRabbit