Skip to content

feat: Add Biome linting, upgrade to Node 20, and add CI workflows - #3

Merged
scottlovegrove merged 1 commit into
mainfrom
feat/biome-linting-node20-ci
Jan 22, 2026
Merged

feat: Add Biome linting, upgrade to Node 20, and add CI workflows#3
scottlovegrove merged 1 commit into
mainfrom
feat/biome-linting-node20-ci

Conversation

@scottlovegrove

Copy link
Copy Markdown
Collaborator

Summary

This PR modernizes the codebase with comprehensive linting, formatting, and CI/CD improvements. It adds Biome linting (matching todoist-ai configuration), upgrades to Node 20, and establishes robust CI pipelines for code quality assurance.

Key Changes

🎯 Biome Linting Setup

  • Add @biomejs/biome 2.3.9 with exact todoist-ai configuration
  • 4-space indentation migration for all TypeScript/JavaScript/JSON files
  • Hybrid approach: Biome for code files, Prettier for markdown/YAML/CSS
  • Unified format scripts: npm run format handles both tools seamlessly

🚀 Node.js Upgrade

  • Upgrade from Node 18 to Node 20
  • Update package.json engines and add .nvmrc
  • All workflows now use Node 20 consistently

Type Safety Improvements

  • Eliminate all any usage (100% type-safe codebase)
  • Replace non-null assertions with safe undefined checks
  • Add Node.js import protocol prefixes (node: imports)
  • Fix TypeScript compilation issues
  • Remove unused imports and variables

🛠 Git Hooks & Automation

  • Update lefthook configuration for file-specific linting
  • Biome handles *.{ts,js,json} files in pre-commit
  • Prettier handles *.{md,yml,yaml,css} files in pre-commit
  • Auto-stage fixed files for seamless developer experience

🤖 CI/CD Pipeline

  • Separate workflows for better organization:
    • lint.yml - Type checking, linting, and formatting validation
    • test.yml - Build verification and test execution
  • Both workflows trigger on pushes to main and pull requests
  • npm cache enabled for faster CI builds
  • 10-minute timeouts for efficient resource usage

Code Quality Metrics

Before vs After

  • Linting Issues: Multiple → 0 (100% resolution)
  • Type Safety: Multiple any usage → 0 any usage
  • Test Coverage: 84 tests passing (maintained)
  • Build Time: Optimized with npm cache

Verification

  • ✅ All 84 tests pass
  • ✅ TypeScript compilation successful
  • ✅ Zero linting errors or warnings
  • ✅ Perfect formatting compliance
  • ✅ CLI functionality verified

Migration Details

Breaking Changes

  • 4-space indentation: Large diff due to formatting migration
  • Stricter linting: New rules may catch previously undetected issues

Developer Experience

  • Unified commands: npm run format handles everything
  • Clear separation: Biome for code quality, Prettier for documentation
  • Fast feedback: Separate CI jobs provide granular status reporting
  • Modern standards: Follows current best practices for TypeScript projects

Testing

All functionality verified:

  • Build process works correctly
  • All existing tests continue to pass
  • CLI commands function as expected
  • Git hooks operate smoothly
  • CI workflows execute successfully

🤖 Generated with Claude Code

- Add @biomejs/biome 2.3.9 with 4-space indentation for all files
- Upgrade from Node 18 to Node 20 (.nvmrc and package.json engines)
- Hybrid approach: Biome for code/JSON, Prettier for markdown/YAML/CSS
- Add separate lint.yml and test.yml CI workflows
- Update lefthook for file-specific linting (biome + prettier)
- Fix Node.js import protocols, eliminate unused imports/variables
- Achieve 100% type safety (zero any usage)
- Update format scripts for unified Biome/Prettier workflow

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this Jan 22, 2026
@scottlovegrove
scottlovegrove requested a review from gnapse January 22, 2026 15:43
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label Jan 22, 2026
@scottlovegrove
scottlovegrove merged commit 57f802e into main Jan 22, 2026
2 checks passed
@scottlovegrove
scottlovegrove deleted the feat/biome-linting-node20-ci branch January 22, 2026 15:43
github-actions Bot pushed a commit that referenced this pull request Jan 22, 2026
# 1.0.0 (2026-01-22)

### Bug Fixes

* add UTF-8 charset to OAuth callback HTML responses ([#8](#8)) ([fcb2d49](fcb2d49))

### Features

* Add Biome linting, upgrade to Node 20, and add CI workflows ([#3](#3)) ([57f802e](57f802e))
* add loading animations with global API proxy integration ([#7](#7)) ([bf53bfb](bf53bfb))
* Add OAuth authentication with dynamic client registration ([#6](#6)) ([2845ebd](2845ebd)), closes [#5](#5)
* add semantic release automation for NPM publishing ([#10](#10)) ([c756324](c756324))
* Refactor login command to auth with status/logout subcommands ([#4](#4)) ([59e6493](59e6493))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gnapse gnapse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lmjabreu added a commit that referenced this pull request May 27, 2026
The Twist v4 workspace_users/get endpoint returns both active and
removed members, with a `removed: boolean` field that the CLI was
ignoring. In workspace 1585 this meant `tw users` listed 554 entries
when only 146 were actual members, and `resolveUserRefs` would
happily match a removed user by name/email, leading to confusing
HTTP 403 "User N is not part of workspace" errors from downstream
mutations like `tw channel add`.

Filter removed users in `getWorkspaceUsers` by default, and add an
`--include-removed` flag on `tw users` for the rare audit case
where you actually want to see them. When that flag is set, the
human-readable output appends a red [removed] chip; `removed` is
exposed in the curated JSON essentials so callers can distinguish
the two states.

Follow-up to side-discovery #3 in #244.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lmjabreu added a commit that referenced this pull request May 27, 2026
## Summary

`tw users` was listing users who had been **removed** from the
workspace. In `Doist` (ws:1585) this meant 554 entries returned for what
is actually a 146-member workspace, polluting `tw users [--json]` output
and silently breaking `resolveUserRefs` — any command that takes a
name/email user ref (`tw channel add`, `tw groups add-user`, etc.) could
resolve to a tombstoned user and then 403 from the backend with `User N
is not part of workspace`.

Root cause: Twist's v4 `workspace_users/get?id=<wsid>` endpoint returns
both active and removed members with a `removed: boolean` flag. The
SDK's `WorkspaceUserSchema` already exposes that field — the CLI just
wasn't reading it. The curated `--json` formatter also stripped it, so
the staleness was invisible.

Reproducer (before the fix):

```
$ tw users 1585 --json --full | jq '[.[] | select(.removed)] | length'
408
$ tw users 1585 --json | jq '. | map(select(.id == 2342 or .id == 2343))'
[
  { "id": 2342, "name": "Test User1",  "userType": "GUEST" },
  { "id": 2343, "name": "Test User 2", "userType": "USER"  }
]
$ # Both above are removed members; adding them to a channel returns 403
$ # "User 2342 is not part of workspace 1585"
```

After the fix:

```
$ tw users 1585 --json | jq '. | length'
146
$ tw users 1585 --json --include-removed | jq '. | length'
554
$ tw users 1585 --include-removed --search "Test User1"
id:2342  Test User1 <test1@doist.io> [GUEST] [removed]
```

## Changes

| Layer | File | Notes |
|---|---|---|
| API helper | `src/lib/api.ts` | `getWorkspaceUsers(workspaceId, {
includeRemoved? })` filters `removed === true` by default. One filter,
fixes `tw users`, `resolveUserRefs`
([src/lib/refs.ts](https://github.com/Doist/twist-cli/blob/lmjabreu/confident-lalande-769bec/src/lib/refs.ts)),
and `resolveNotifyIds`
([src/commands/thread/helpers.ts](https://github.com/Doist/twist-cli/blob/lmjabreu/confident-lalande-769bec/src/commands/thread/helpers.ts))
in one shot. |
| Command | `src/commands/user.ts` | New `--include-removed` flag. Human
output appends a red `[removed]` chip. |
| JSON | `src/lib/output.ts` | Added `removed` to
`USER_ESSENTIAL_FIELDS` so callers can distinguish the two states
without `--full`. |
| Tests | `src/commands/user.test.ts` | Two new tests: default omits
removed users, flag passes through and chip renders. |

`getUserById`-based callers (e.g. `tw groups view`) were already correct
because they only fetch ids that the group's `userIds` list references —
removed users aren't there.

## Why a flag, not always filter

Audit/recovery cases occasionally do want the historical roster (e.g.
"who used to be in here?"). `--include-removed` covers that without
making the common case noisy.

## Side discoveries

None. Scope was kept tight to the diagnosed bug.

## Checks

- [x] `npm run type-check`
- [x] `npm run lint:check`
- [x] `npm test` — 653/653 (2 new)
- [x] Manual end-to-end against workspace 1585 (Doist) — counts above

Follow-up to side-discovery #3 in #244.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doist-release-bot Bot added a commit that referenced this pull request May 27, 2026
## [2.44.1](v2.44.0...v2.44.1) (2026-05-27)

### Bug Fixes

* **deps:** update dependency @doist/twist-sdk to v2.8.1 ([#259](#259)) ([a00bc9e](a00bc9e))
* **users:** filter removed members from `tw users` by default ([#245](#245)) ([c921a19](c921a19)), closes [#3](#3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released 👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants