Send POST and PUT parameters in the request body #241

[chenghung]

#241

TL;DR

POST and PUT parameters now travel in a JSON request body instead of the query string, so long descriptions and comments stop failing with HTTP 414. Background and the API verification are in the linked issue.

It also pins @oclif/core to 4.13.3. CI is red on main for an unrelated reason, and no PR can go green without it. That change is a separate commit and can be dropped on its own.

Behaviour change

	Before	After
card:update --description with 10,014 bytes of CJK	exits 1 with 414; description unchanged	exits 0; all 3,338 characters stored
Practical CJK description limit	around 900 characters	Trello's own field limit
POST/PUT parameters	query string	JSON body
GET, DELETE, multipart upload, key/token	—	unchanged
Flags, output formats, exit codes	—	unchanged

23 commands issue POST or PUT, so all of them move to the new transport. Five were failing on long text: card:update --description, card:create --description, card:comment --text, board:create --description, and the TUI's multi-line description editor.

The @oclif/core pin

Why CI is red, independent of this branch:

• The workflow pins pnpm 8, but pnpm-lock.yaml is lockfileVersion: '9.0'. pnpm 8 logs WARN Ignoring not compatible lockfile and re-resolves every dependency from its semver range, so CI has never installed the locked versions.
• @oclif/core: ^4.8.0 therefore floats. 4.13.4 ships a module format Jest cannot parse under the current CommonJS setup, so 29 of the 42 suites fail with Jest encountered an unexpected token before any assertion runs.

Three runs on clean checkouts with pnpm 8, matching what CI installs:

Tree	@oclif/core	Suites failed	Tests failed	Tests total
main at ffb7c21	floats to 4.13.5	29	135	303
This branch	floats to 4.13.5	29	135	316
This branch	4.13.3	0	0	316

Rows one and two reproduce CI's numbers exactly and are identical — that is what identifies the breakage as pre-existing. The 13-test difference is this PR's own tests, all passing.

The boundary came from a binary search over all 35 releases between 4.8.0 and 4.13.5, with both sides re-run explicitly: 4.13.3 passes 42 suites, 4.13.4 fails 29. 4.13.5 is the newest 4.x, so 4.13.3 is the newest version that works.

One more run checked the pin as it actually ships — only the package.json change, no pnpm.overrides, clean pnpm 8 install. One copy of @oclif/core@4.13.3 is installed and the suite is green. Pinning the direct dependency suffices because @oclif/test takes @oclif/core as a peer and the three @oclif/plugin-* ranges accept 4.13.3.

The pin is temporary. It can go once either cause is addressed: raise the workflow's pnpm to match the lockfile, or teach Jest to handle the newer module format. Two caveats if you take the first route: posttest: "yarn lint" has never run in CI, because pnpm 8 skips post scripts, and it currently reports 132 errors — all in .tsx files, which eslint.config.mjs leaves out of its rule overrides. And Version Packages PRs opened by GITHUB_TOKEN get no checks at all.

Risk and reviewer focus

1. The pin is a policy call, not a bug fix. An exact pin stops consumers receiving patch releases. If you would rather fix the pnpm/lockfile mismatch, or use a range like >=4.8.0 <4.13.4, say so — the 414 fix does not depend on it.

2. The wrapper reaches into a dependency's request pipeline. It replaces sendRequest on the TrelloClient instance, so it is coupled to that method's signature and to two fields of RequestConfig. Upstream is not an alternative here: trello.js 2.1.6 still puts desc in the query string, is ESM-only, and requires Node >= 22.

3. Every POST/PUT changes transport, not just the broken ones. 17 write endpoints were checked against the live API with all parameters — including required ones — sent only in the body; all applied correctly. Only two of them document body support. The gap is PUT /notifications/{id}, which returns 401 for body and query string on every notification available for testing, so notification:read and notification:unread are unverified rather than confirmed.

4. Values now travel as native JSON types instead of percent-encoded strings. Arrays, comma-joined strings, booleans, numbers, ISO dates, and the native Date string chrono-node produces for --due were each verified to parse to the same result as before. Object-valued parameters were inconclusive; no command sends one, and a code comment says so.

5. Instance wrapping instead of subclassing is deliberate. jest.mock("trello.js") returns a plain object from the constructor, which discards a subclass prototype and makes an overridden sendRequest unreachable. Wrapping keeps all 28 mocked test files unchanged.

Walkthrough

src/paramsInBody.ts

moveParamsIntoBody(config) is pure: untouched config unless the method is POST or PUT, data is absent, and at least one parameter is non-empty; otherwise a new config with params emptied and the survivors as data. undefined and null are dropped, matching the query serialiser — updateCard passes all 17 fields on every call, most of them undefined. params becomes {} rather than being deleted, because BaseClient.sendRequest spreads it and appends key and token afterwards, so credentials stay in the query string without the wrapper knowing them.

sendParamsInBody(client) installs the transform as an own property on the instance. The API groups hold a reference to the client and call this.client.sendRequest(...), so one own property covers every call site. The cast exists only because sendRequest is overloaded.

src/BaseCommand.ts, src/commands/interactive.tsx

The only two places that call new TrelloClient(...) — BaseCommand.init for regular commands, and interactive.tsx for the TUI. Both wrap at construction, which is why the TUI is covered.

test/paramsInBody.test.ts — 13 tests

Eleven for the transform: POST and PUT relocation, a long CJK description staying out of the query string, lowercase methods, GET and DELETE untouched, a request that already carries a body untouched, empty-value stripping, the no-usable-params and no-params cases, and no mutation of the input.

Two for the installer: that sendRequest is routed through the transform and its result returned, and that the callback argument passes through.

package.json, pnpm-lock.yaml

@oclif/core moves from ^4.8.0 to 4.13.3, lockfile regenerated to match. The lockfile diff is confined to that dependency.

Conclusion

The 414 fix is transport-only: no flag, output format or exit code moves. Full suite green — 42 suites / 316 tests, tsc --noEmit clean, no new eslint errors — under both pnpm 10 and pnpm 8.

🤖 Generated with Claude Code

[chenghung]

@mheap — whenever you have a moment, would you mind casting an eye over this? No urgency at all.

One thing worth flagging up front so it doesn't look odd: the second commit pins @oclif/core to 4.13.3, which has nothing to do with the fix itself. CI was already failing on main — the workflow's pnpm 8 can't read the lockfileVersion: '9.0' lockfile, so it floats @oclif/core to a release Jest can't parse. I only included the pin so the checks here would mean something, and kept it as its own commit so you can drop it if you'd rather fix the pnpm/lockfile mismatch instead. The control run that shows the breakage is pre-existing is in the description.

The fix itself is the first commit; background and the API verification behind it are in #241. Checks are green on all four Node versions.

Happy to split this up, rework it, or drop the pin — whichever suits you best. Thanks for maintaining this.