-
Notifications
You must be signed in to change notification settings - Fork 0
CLI Build and Deployment Setup #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add build:binary and build:binary:release scripts to package.json - Create scripts/build-release.sh for platform-specific local builds - Add *.bun-build to gitignore
- Lint & typecheck on ubuntu - Tests on ubuntu, macos, windows - Binary builds for all 5 platform targets - Uploads artifacts for each platform
- Release workflow triggered by tdn-cli-v* tags - Builds archives with checksums for all 5 platforms - Creates GitHub Release with auto-generated notes - Triggers Homebrew tap update for stable releases - Add curl-installable install.sh script
|
Warning Rate limit exceeded@dannysmith has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 58 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds CI and release GitHub workflows, new CLI build/install/prepare scripts, package scripts and .gitignore entry, a Windows-tolerant disk-sync tweak in Rust writer, vault-path temp-dir acceptance, and several test/helper updates to centralize archive-path detection. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (4)
tdn-cli/scripts/install.sh (1)
62-69: Fragile JSON parsing withoutjq.Parsing JSON with
grepandsedis brittle and may break if GitHub changes their API response formatting. Consider addingjqas an optional dependency with a fallback, or document this limitation.🔎 Proposed improvement using jq with fallback
if [ "$VERSION" = "latest" ]; then info "Fetching latest version..." - # Filter for tdn-cli releases specifically - VERSION=$(curl -sL "https://api.github.com/repos/${REPO}/releases" | \ - grep '"tag_name":' | \ - grep 'tdn-cli-v' | \ - head -1 | \ - sed -E 's/.*"tdn-cli-v([^"]+)".*/\1/') + # Filter for tdn-cli releases specifically + RELEASES_JSON=$(curl -sL "https://api.github.com/repos/${REPO}/releases") + if command -v jq >/dev/null 2>&1; then + VERSION=$(echo "$RELEASES_JSON" | jq -r '[.[] | select(.tag_name | startswith("tdn-cli-v"))][0].tag_name // empty' | sed 's/^tdn-cli-v//') + else + VERSION=$(echo "$RELEASES_JSON" | \ + grep '"tag_name":' | \ + grep 'tdn-cli-v' | \ + head -1 | \ + sed -E 's/.*"tdn-cli-v([^"]+)".*/\1/') + fi if [ -z "$VERSION" ]; then error "Failed to fetch latest version. Check your internet connection." fi fitdn-cli/scripts/prepare-release.js (1)
146-148: Add error handling for unhandled promise rejection.The script calls
prepareRelease()without catching errors, which could lead to unhandled promise rejection warnings in Node.js.🔎 Proposed fix
-prepareRelease() +prepareRelease().catch(error => { + console.error('Unexpected error:', error.message) + process.exit(1) +})tdn-cli/scripts/build-release.sh (1)
7-7: VERSION is unused in the output filename.The
VERSIONvariable is set but only used in the echo message, not in the actual output filename. This might be intentional for the build script (version is added during release packaging), but consider adding a comment to clarify this design decision..github/workflows/release-cli.yml (1)
109-109: Static analysis hint: Consider quoting the variable expansion.While the ref value is unlikely to contain spaces, quoting
${GITHUB_REF#refs/tags/tdn-cli-v}is a good defensive practice.🔎 Proposed fix
- name: Extract version id: version - run: echo "version=${GITHUB_REF#refs/tags/tdn-cli-v}" >> $GITHUB_OUTPUT + run: echo "version=${GITHUB_REF#refs/tags/tdn-cli-v}" >> "$GITHUB_OUTPUT"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/tasks-todo/task-x-cli-distribution-and-releases.mdis excluded by!**/*.mdtdn-cli/docs/developer/releases.mdis excluded by!**/*.md
📒 Files selected for processing (7)
.github/workflows/ci-cli.yml.github/workflows/release-cli.ymltdn-cli/.gitignoretdn-cli/package.jsontdn-cli/scripts/build-release.shtdn-cli/scripts/install.shtdn-cli/scripts/prepare-release.js
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
Applied to files:
tdn-cli/scripts/prepare-release.js.github/workflows/ci-cli.ymltdn-cli/package.json
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Clean build artifacts (*.node, index.js, index.d.ts) before rebuilding if experiencing build failures
Applied to files:
tdn-cli/scripts/build-release.shtdn-cli/package.json
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Applies to tdn-cli/bindings/** : Never manually edit files in the `bindings/` directory as it is auto-generated by NAPI-RS
Applied to files:
tdn-cli/.gitignoretdn-cli/package.json
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Applies to tdn-cli/src/**/*.{ts,tsx} : Use path aliases in TypeScript imports: `@/*` maps to `src/*` (e.g., `import { foo } from '@/commands/list'`) and `bindings` maps to `bindings/` (e.g., `import { parseTask } from 'bindings`)
Applied to files:
tdn-cli/.gitignoretdn-cli/package.json
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Run `bun run fix` before `bun run check` when editing multiple TypeScript or Rust files to avoid formatting errors failing the check
Applied to files:
tdn-cli/package.json
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Regenerate TypeScript bindings by running `bun run build` after modifying any `#[napi]` function signatures, structs, or enums in Rust code
Applied to files:
tdn-cli/package.json
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Use `bun` (not pnpm/npm) for all JavaScript/TypeScript operations
Applied to files:
tdn-cli/package.json
📚 Learning: 2025-12-28T12:22:10.779Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:10.779Z
Learning: Use `bun` instead of `pnpm` or `npm` for JavaScript/TypeScript projects
Applied to files:
tdn-cli/package.json
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Applies to tdn-cli/crates/**/*.rs : Rust code must use the `#[napi]` macro to expose functions that will be auto-generated as TypeScript bindings
Applied to files:
tdn-cli/package.json
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Use bun as the package manager and task runner for this project, not npm or pnpm
Applied to files:
tdn-cli/package.json
📚 Learning: 2025-12-28T12:22:26.336Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-rust/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:26.336Z
Learning: Applies to archived-projects/taskdn-rust/{src/**/*.rs,tests/**/*.rs} : Place unit tests in the same file as code using #[cfg(test)] modules, integration tests in tests/ directory
Applied to files:
tdn-cli/package.json
📚 Learning: 2025-12-28T17:09:11.509Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-desktop/AGENTS.md:0-0
Timestamp: 2025-12-28T17:09:11.509Z
Learning: Use bun only for package management and running scripts (not npm or pnpm)
Applied to files:
tdn-cli/package.json
🪛 actionlint (1.7.9)
.github/workflows/release-cli.yml
109-109: shellcheck reported issue in this script: SC2086:info:1:53: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-cli.yml
91-91: shellcheck reported issue in this script: SC1001:info:1:2: This \d will be a regular 'd' in this context
(shellcheck)
91-91: shellcheck reported issue in this script: SC1012:warning:1:7: \t is just literal 't' here. For tab, use "$(printf '\t')" instead
(shellcheck)
🔇 Additional comments (8)
tdn-cli/.gitignore (1)
9-9: LGTM!The new
.bun-buildpattern correctly excludes Bun's build artifacts, aligning with the release build scripts introduced in this PR.tdn-cli/scripts/install.sh (2)
1-2: LGTM!Good use of strict mode with
set -euo pipefailfor robust error handling.
111-118: LGTM!Extraction and installation logic is straightforward with proper directory creation and executable permissions.
tdn-cli/package.json (1)
25-28: LGTM!The new build and release scripts are well-structured. Using
--sourcemapfor release builds aids in debugging production issues while keeping the binary minified.tdn-cli/scripts/build-release.sh (1)
21-25: LGTM!The build sequence correctly ensures NAPI bindings are built before compiling the standalone binary.
.github/workflows/ci-cli.yml (2)
89-91: Static analysis false positive - Windows path syntax is correct.The shellcheck warnings (SC1001/SC1012) about
\dand\tare false positives. This step runs on Windows where backslashes are valid path separators. The syntax.\dist\tdn-...is correct for PowerShell/cmd.
49-66: LGTM!The build matrix covers all major platforms with appropriate runners. Using
macos-13for x64 builds (Intel) andmacos-latestfor arm64 (Apple Silicon) is the correct approach..github/workflows/release-cli.yml (1)
120-127: Verify the Homebrew tap token secret exists.The workflow references
secrets.HOMEBREW_TAP_TOKENwhich must be configured in the repository settings. Ensure this secret is created with appropriate permissions for the target repository.
- Split upload artifact step for Unix/Windows file types - Fix working directory for release job steps - Improve checksum verification to not falsely report success - Quote variable expansion in version extraction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.github/workflows/release-cli.yml (2)
77-95: LGTM! Conditional uploads resolve past artifact issue.The conditional upload steps correctly separate Unix (tar.gz) and Windows (zip) artifacts, ensuring each target only uploads the archive type it produces. This addresses the previous review concern about mixed file patterns.
110-116: LGTM! Working directory overrides fix path issues.Using
working-directory: .correctly overrides the defaulttdn-cliworking directory, ensuring paths are resolved from the repository root. This addresses previous review concerns.tdn-cli/scripts/install.sh (1)
89-110: LGTM! Checksum verification is no longer misleading.The improved logic correctly handles the case when no hash tool is available:
- Warning is printed (line 101)
- "Checksum verified" message only appears when verification actually succeeds (line 105)
This addresses the concern from previous review about silent bypass being misleading.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/tasks-todo/task-x-cli-distribution-and-releases.mdis excluded by!**/*.md
📒 Files selected for processing (3)
.github/workflows/ci-cli.yml.github/workflows/release-cli.ymltdn-cli/scripts/install.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Applies to tdn-cli/src/**/*.{ts,tsx} : Use path aliases in TypeScript imports: `@/*` maps to `src/*` (e.g., `import { foo } from '@/commands/list'`) and `bindings` maps to `bindings/` (e.g., `import { parseTask } from 'bindings`)
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
Applied to files:
.github/workflows/ci-cli.yml
🪛 actionlint (1.7.9)
.github/workflows/release-cli.yml
120-120: shellcheck reported issue in this script: SC2086:info:1:57: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ci-cli.yml
91-91: shellcheck reported issue in this script: SC1001:info:1:2: This \d will be a regular 'd' in this context
(shellcheck)
91-91: shellcheck reported issue in this script: SC1012:warning:1:7: \t is just literal 't' here. For tab, use "$(printf '\t')" instead
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Binary - windows-x64
- GitHub Check: Test - windows-latest
- GitHub Check: Lint & Typecheck
- GitHub Check: Test - ubuntu-latest
- GitHub Check: Test - macos-latest
🔇 Additional comments (13)
.github/workflows/ci-cli.yml (5)
1-21: LGTM! Well-configured CI trigger and environment.The workflow appropriately scopes triggers to
tdn-cli/**paths and sets a working directory default, which is ideal for a monorepo structure.
23-48: LGTM! Comprehensive testing strategy.Testing across Ubuntu, macOS, and Windows ensures the CLI works reliably on all major platforms. Building NAPI bindings before tests is the correct order of operations.
85-91: Shellcheck warnings are false positives.The static analysis warnings on line 91 (
SC1001,SC1012) are false positives. The Windows path syntax.\dist\is correct for PowerShell, which is the default shell for Windows runners in GitHub Actions.
93-98: LGTM! Artifact upload path is correct.The artifact path
tdn-cli/dist/tdn-${{ matrix.target }}*is correctly specified relative to the repository root, sincewith:parameters in actions don't inherit the workflow'sworking-directory.
49-66: Theubuntu-24.04-armrunner is a valid, officially available GitHub-hosted runner for arm64 architecture on Ubuntu 24.04, announced by GitHub on June 24, 2024. The workflow configuration is correct and requires no changes..github/workflows/release-cli.yml (2)
1-36: LGTM! Proper release workflow configuration.The tag trigger pattern
tdn-cli-v*correctly scopes to CLI releases, andcontents: writepermission is necessary for creating GitHub releases.
55-66: LGTM! Platform-appropriate archive creation.The conditional use of
sha256sum(Linux) vsshasum(macOS) correctly handles platform differences in Unix systems.tdn-cli/scripts/install.sh (6)
1-34: LGTM! Excellent error handling and user experience setup.The use of
set -euo pipefailensures strict error handling, and the terminal detection for colors provides a good user experience both in interactive and non-interactive contexts.
35-56: LGTM! Robust platform detection.The platform and architecture detection handles common variations (e.g.,
x86_64vsamd64) and provides clear error messages for unsupported platforms.
58-71: LGTM! Version resolution handles monorepo releases correctly.Filtering for
tdn-cli-vprefixed tags ensures the script selects the correct release in a monorepo context where multiple packages may have releases.
73-87: LGTM! Proper temporary file handling.The use of
trap 'rm -rf "$TMPDIR"' EXITensures cleanup happens even if the script fails, preventing leftover temporary files.
112-127: LGTM! Installation includes verification.Running the installed binary with
--versionprovides immediate feedback that the installation succeeded and the binary is functional.
129-142: LGTM! Helpful PATH guidance.The PATH check provides clear, actionable instructions for users to add the installation directory to their shell profile, improving the installation experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci-cli.yml (2)
23-33: Consider caching dependencies for faster CI runs.The lint job setup is correct, but adding dependency caching could significantly improve CI performance.
🔎 Suggested caching additions
Add these steps after the tool setup and before
bun install:- uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + with: + workspaces: tdn-cli + - uses: actions/cache@v4 + with: + path: ~/.bun/install/cache + key: ${{ runner.os }}-bun-${{ hashFiles('tdn-cli/bun.lockb') }} - run: bun install
85-92: Consider more comprehensive binary smoke tests.The
--versioncheck verifies the binary starts, but consider adding a simple functionality test to catch runtime issues earlier.🔎 Example additional smoke test
Add after the version test steps:
- name: Smoke test binary (Unix) if: matrix.target != 'windows-x64' run: | ./dist/tdn-${{ matrix.target }} --help # Add a simple command test if applicable # ./dist/tdn-${{ matrix.target }} <some-safe-command> - name: Smoke test binary (Windows) if: matrix.target == 'windows-x64' run: | .\dist\tdn-${{ matrix.target }}.exe --help # Add a simple command test if applicable
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci-cli.yml.github/workflows/release-cli.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/release-cli.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
Applied to files:
.github/workflows/ci-cli.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Binary - windows-x64
- GitHub Check: Test - ubuntu-latest
- GitHub Check: Test - macos-latest
- GitHub Check: Test - windows-latest
🔇 Additional comments (5)
.github/workflows/ci-cli.yml (5)
1-21: LGTM! Clean workflow configuration.The trigger paths, default working directory, and environment setup are well-structured for a monorepo CLI project.
34-47: Good cross-platform test coverage.The test matrix appropriately covers the major platforms. The same caching optimization mentioned for the lint job would benefit this job as well.
71-84: LGTM! Binary build steps properly handle platform differences.The conditional Unix/Windows build steps correctly handle the
.exeextension requirement and output naming conventions.
93-98: LGTM! Artifact upload configuration is correct.The artifact path correctly references the repository root (not the working directory), and the wildcard pattern properly handles both Unix and Windows binary names. The
if-no-files-found: errorsetting ensures build failures are caught.
49-70: No actionable issues identified. The runner labelsmacos-15-intelandubuntu-24.04-armare both valid and publicly available GitHub-hosted runners. The matrix configuration is correct and will work without workflow failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tdn-cli/crates/core/src/writer.rs (1)
194-214: Consider adding debug logging for Windows sync failures.While making
sync_allnon-fatal on Windows is a pragmatic solution to temp directory permission issues, silently ignoring the error at line 203 (let _ = e;) may hide legitimate I/O problems during development or debugging.🔎 Suggested enhancement
Consider adding debug-level logging when sync fails on Windows:
#[cfg(target_os = "windows")] { - // Silently ignore sync errors on Windows - let _ = e; + // Log but don't fail on Windows sync errors (common in temp dirs) + eprintln!("Warning: Failed to sync file (non-fatal on Windows): {}", e); }This would help diagnose unexpected I/O issues while still allowing the operation to proceed on Windows.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/ci-cli.ymltdn-cli/crates/core/src/writer.rstdn-cli/src/config/index.tstdn-cli/tests/e2e/list.test.tstdn-cli/tests/e2e/modify.test.tstdn-cli/tests/helpers/cli.tstdn-cli/tests/unit/bindings.test.tstdn-cli/tests/unit/config-security.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
tdn-cli/crates/**/*.rs
📄 CodeRabbit inference engine (tdn-cli/CLAUDE.md)
Rust code must use the
#[napi]macro to expose functions that will be auto-generated as TypeScript bindings
Files:
tdn-cli/crates/core/src/writer.rs
tdn-cli/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (tdn-cli/CLAUDE.md)
Use path aliases in TypeScript imports:
@/*maps tosrc/*(e.g.,import { foo } from '@/commands/list') and@bindingsmaps tobindings/(e.g.,import { parseTask } from '@bindings)
Files:
tdn-cli/src/config/index.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Applies to tdn-cli/src/**/*.{ts,tsx} : Use path aliases in TypeScript imports: `@/*` maps to `src/*` (e.g., `import { foo } from '@/commands/list'`) and `bindings` maps to `bindings/` (e.g., `import { parseTask } from 'bindings`)
Applied to files:
tdn-cli/tests/helpers/cli.tstdn-cli/tests/unit/config-security.test.tstdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/modify.test.tstdn-cli/tests/e2e/list.test.tstdn-cli/src/config/index.ts
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Applies to archived-projects/taskdn-ts/**/index.d.ts : Ensure TypeScript types in index.d.ts exactly match the Taskdn specification and maintain type fidelity with the Rust SDK API
Applied to files:
tdn-cli/tests/helpers/cli.tstdn-cli/tests/unit/config-security.test.tstdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/modify.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Applies to archived-projects/taskdn-ts/src/lib.rs : Implement all file operations as synchronous in the TypeScript SDK to match the Rust SDK design
Applied to files:
tdn-cli/crates/core/src/writer.rs
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Applies to tdn-cli/**/*.test.*/**/* : Fixture files for tests must conform to the S1 specification
Applied to files:
tdn-cli/tests/unit/config-security.test.tstdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/modify.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Follow TDD workflow for CLI features: write failing E2E test, add fixture files conforming to S1 spec, review test against specs, get user confirmation, implement until green, refactor, add unit tests (Rust for parsing, TS for formatters/utilities), run checks, update docs
Applied to files:
tdn-cli/tests/unit/config-security.test.tstdn-cli/tests/e2e/list.test.ts.github/workflows/ci-cli.yml
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Use snapshot testing via api-snapshot.test.ts to verify that generated TypeScript type definitions (index.d.ts) have not unintentionally changed
Applied to files:
tdn-cli/tests/unit/config-security.test.tstdn-cli/tests/e2e/modify.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Applies to archived-projects/taskdn-ts/**/index.d.ts : Use tagged unions with a 'type' field for file references (possible values: 'wikilink', 'relativePath', 'filename') in the TypeScript API
Applied to files:
tdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/modify.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Applies to archived-projects/taskdn-ts/**/*.{ts,tsx,js} : Use camelCase for JavaScript/TypeScript identifiers (e.g., createdAt) while maintaining snake_case internally in Rust code
Applied to files:
tdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/modify.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T12:22:26.336Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-rust/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:26.336Z
Learning: Applies to archived-projects/taskdn-rust/{src/**/*.rs,tests/**/*.rs} : Place unit tests in the same file as code using #[cfg(test)] modules, integration tests in tests/ directory
Applied to files:
tdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Applies to archived-projects/taskdn-ts/**/index.d.ts : Expose Rust status enums as TypeScript string literal unions (e.g., TaskStatus = 'ready' | 'completed') rather than as objects
Applied to files:
tdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Update the api-snapshot.test.ts snapshot file when intentionally changing the NAPI bindings API using 'bun test --update-snapshots'
Applied to files:
tdn-cli/tests/unit/bindings.test.tstdn-cli/tests/e2e/modify.test.ts
📚 Learning: 2025-12-28T12:22:45.546Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-cli/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:45.546Z
Learning: Use `bun` (not pnpm/npm) for all JavaScript/TypeScript operations
Applied to files:
tdn-cli/tests/e2e/modify.test.ts
📚 Learning: 2025-12-28T12:22:37.372Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: archived-projects/taskdn-ts/CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:37.372Z
Learning: Use bun as the package manager and task runner for this project, not npm or pnpm
Applied to files:
tdn-cli/tests/e2e/modify.test.tstdn-cli/tests/e2e/list.test.ts
📚 Learning: 2025-12-28T17:09:11.509Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: tdn-desktop/AGENTS.md:0-0
Timestamp: 2025-12-28T17:09:11.509Z
Learning: Use bun only for package management and running scripts (not npm or pnpm)
Applied to files:
tdn-cli/tests/e2e/modify.test.ts
📚 Learning: 2025-12-28T12:22:10.779Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:10.779Z
Learning: Use `bun` instead of `pnpm` or `npm` for JavaScript/TypeScript projects
Applied to files:
tdn-cli/tests/e2e/modify.test.ts
📚 Learning: 2025-12-28T12:22:10.779Z
Learnt from: CR
Repo: dannysmith/taskdn PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T12:22:10.779Z
Learning: Never make changes to files in tdn-specs/ without clearly confirming with the user first, as specification changes affect all implementations
Applied to files:
tdn-cli/src/config/index.ts
🧬 Code graph analysis (5)
tdn-cli/crates/core/src/writer.rs (1)
tdn-cli/crates/core/src/error.rs (1)
write_error(89-97)
tdn-cli/tests/unit/config-security.test.ts (1)
tdn-cli/src/config/index.ts (1)
validateVaultPath(44-83)
tdn-cli/tests/unit/bindings.test.ts (1)
tdn-cli/tests/helpers/cli.ts (1)
isArchivePath(101-104)
tdn-cli/tests/e2e/modify.test.ts (1)
tdn-cli/tests/helpers/cli.ts (1)
isArchivePath(101-104)
tdn-cli/tests/e2e/list.test.ts (1)
tdn-cli/tests/helpers/cli.ts (2)
isArchivePath(101-104)runCli(43-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test - windows-latest
- GitHub Check: Build Binary - darwin-x64
- GitHub Check: Lint & Typecheck
- GitHub Check: Test - ubuntu-latest
- GitHub Check: Build Binary - windows-x64
- GitHub Check: Test - macos-latest
🔇 Additional comments (16)
tdn-cli/tests/helpers/cli.ts (1)
97-104: LGTM! Clean cross-platform archive path detection.The implementation correctly handles both Windows and Unix path separators using a simple regex pattern. The function is well-documented and serves as a centralized utility for archive path detection across the test suite.
tdn-cli/src/config/index.ts (1)
71-74: LGTM! Improved cross-platform temp directory handling.The addition of Linux
/tmp/support alongside the existing macOS/var/folders/exception makes the validation logic more robust across platforms. The introduction of theisTempDirvariable improves code clarity..github/workflows/ci-cli.yml (2)
86-111: LGTM! Well-structured multi-platform build job.The build job properly:
- Separates Unix and Windows binary compilation steps
- Includes smoke tests for each platform to validate basic functionality
- Uploads artifacts with appropriate naming
The conditional execution based on target platform is handled correctly.
55-72: No action required. Both runner labels are valid and available in GitHub Actions as of December 2025:
macos-15-intelis the correct label for Intel-based macOS 15 runnersubuntu-24.04-armis the correct label for Ubuntu 24.04 ARM runnersThe build matrix configuration is correct.
tdn-cli/tests/unit/bindings.test.ts (2)
10-10: LGTM! Importing centralized archive path helper.Good refactor to use the shared
isArchivePathutility from test helpers.
138-143: LGTM! Improved archive path detection.Using
isArchivePath(t.path)instead of a string match for'archive/'makes the test more robust and cross-platform compatible (handles both forward slashes and backslashes).tdn-cli/tests/e2e/modify.test.ts (2)
2-2: LGTM! Importing cross-platform path helper.Good addition of the
isArchivePathutility for consistent archive path detection.
385-387: LGTM! Improved cross-platform archive path validation.The test now properly:
- Uses
isArchivePath(output.to)to detect archive directories across platforms- Validates the filename suffix separately with a regex
This separation of concerns makes the test more maintainable and cross-platform compatible.
tdn-cli/tests/e2e/list.test.ts (4)
2-2: LGTM! Importing cross-platform path helper.Good addition of the
isArchivePathutility for consistent archive path detection across the test suite.
904-904: LGTM! Cross-platform archive path detection.Using
isArchivePath(t.path)instead of a string substring check properly handles both Unix and Windows path separators.
910-910: LGTM! Consistent use of archive path helper.The negated form
!isArchivePath(t.path)correctly validates that archived tasks are excluded by default.
920-920: LGTM! Proper validation for --only-archived flag.Correctly uses
isArchivePath(t.path)to ensure all returned tasks are from archive directories.tdn-cli/tests/unit/config-security.test.ts (4)
3-4: LGTM! Improved imports with static named imports.Using explicit named imports from
osandpathmodules is better than dynamic requires and provides better type safety.
36-42: LGTM! Cross-platform path construction.Using
path.join()for path construction is more robust and cross-platform compatible than string concatenation.
133-146: LGTM! Improved test coverage for temp directory handling.The new tests properly validate:
/tmppaths are accepted without warnings (Linux temp directory)- Non-temp, outside-home paths still trigger warnings (e.g.,
/opt/tasks)This aligns with the expanded temp directory logic in
tdn-cli/src/config/index.tsthat now accepts both/var/folders/(macOS) and/tmp/(Linux).
149-178: LGTM! Platform-aware path resolution tests.The updated tests properly handle platform differences:
- Windows: expects drive letters (e.g.,
C:\)- Unix: expects leading slash
- Uses
path.join()for cross-platform path constructionThe test at lines 171-178 correctly validates that paths with
..are properly resolved to their canonical form.
Rust tests were using format!("{}/file.md", dir) which creates mixed path
separators on Windows (e.g., C:\Users\...\tasks/task1.md). Changed to use
Path::new().join() which uses the correct separator for each platform.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary by CodeRabbit
New Features
Chores
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.