Repository navigation
task text: one 20000-character limit, and a counter where the text is cut - #132
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a82872d7c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { StringDecoder } = require('string_decoder'); | ||
| // Same limit the HTTP API enforces; advertising a smaller one here makes the model | ||
| // shorten a task for no reason. | ||
| const { MAX_TASK_TEXT } = require('./task-text-limit'); |
There was a problem hiding this comment.
In packaged Electron builds, server.js launches this helper from app.asar.unpacked using plain system Node, but electron-builder.yml only unpacks mcp-*.js, hooks/**, and bots.js. The new task-text-limit.js dependency remains inside app.asar, so this require throws MODULE_NOT_FOUND and the task-manager MCP server cannot start, removing its tools from scheduled task execution. Add task-text-limit.js to asarUnpack alongside the helper. The existing test/asar-helpers.test.js dependency-coverage check also fails on this change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0896dca. task-text-limit.js is now listed in asarUnpack next to bots.js, the helper requires it as ./task-text-limit.js (the coverage check in test/asar-helpers.test.js resolves the extension literally), and that suite is green on the new head: 9 passed, 0 failed.
A task description is a prompt, and 2000 characters is shorter than a useful one: the textarea and every server write cut the end off with no message, so a long prompt reached the worker with its last instructions missing. MAX_TASK_TEXT now lives in task-text-limit.js and is used by all ten task-text writes in server.js, by the MCP schema an agent reads, and by the description and notes fields in public/schedule.html and public/kanban.html. Both pages are single static files with no build step, so each carries its own copy of the literal and test/task-text-limit.test.js fails if a copy drifts. mcp-task-manager.js is spawned as a plain node child from the unpacked part of the desktop build, so it requires the module with its .js extension and electron-builder.yml unpacks task-text-limit.js next to it - the same trap that once left bots.js inside app.asar (test/asar-helpers.test.js pins both). The kanban group-form test now takes the limit from the page instead of a copy. The prose fields now show a counter that turns red at the limit - a maxlength cut is silent otherwise - and the schedule card shows three lines of a description instead of one. The server still truncates at the limit rather than returning 400: failing loudly is a behaviour change for API callers and the reporter was asked to weigh it, so that part stays open.
a82872d to
0896dca
Compare
Closes #129.
A task description is a prompt, and 2000 characters is shorter than a useful one. The field cut the end off with no message — once in the browser
maxlength, and again in every server write — so a long prompt reached the worker with its last instructions missing.What changed
task-text-limit.jsholdsMAX_TASK_TEXT = 20000.server.jsuses it in all ten task-text writes (the four task INSERTs, the update, both plan importers and the task-manager endpoint), andmcp-task-manager.jsadvertises the same number to the agent instead of a smaller one.public/schedule.htmlandpublic/kanban.htmlput the limit on every description and notes field (1 and 7 places). Each page keeps its own copy of the literal — they are single static files with no build step — andtest/task-text-limit.test.jsfails if a copy drifts from the module, which is what happened when the limit existed in four unrelated places.1234 / 20000and turn red at the limit. Amaxlengthattribute truncates a paste silently; the counter is the only warning the user gets, and it reaches the limit exactly where the text was cut.-webkit-line-clamp:3), because a longer field is only useful if some of it is visible on the board.mcp-task-manager.jsis spawned as a plainnodechild fromapp.asar.unpacked, so it requires the module as./task-text-limit.jsandelectron-builder.ymlunpacks that file next to it — the same trap that once leftbots.jsinside the archive.Deliberately not changed: the server still truncates at the limit instead of returning
400. Whether an over-long request should fail loudly is a behaviour change for API callers — today those calls lose text silently — and the reporter was asked to weigh it. This PR does the part that does not depend on that answer.Evidence
test/task-text-limit.test.js(new, 22 checks) against the pre-change tree: 4 passed, 18 failed —a 5000-character description is stored whole — expected 5000, got 2000on the HTTP API, on an update, and through the task-manager endpoint an agent posts to; 25000 characters capped at 2000 instead of the limit; both pages still carryingmaxlength="2000". After the change: 22 passed, 0 failed.The suite is driven the way an agent drives it: it starts a real server against a throwaway
APP_DIR, posts{action, taskId, ...args}with the task-manager bearer token, and creates everything asbacklogor a year out, so nothing is queued.Three existing guards caught real mistakes in the first head, all fixed here:
test/env-load-order.test.js— the new localrequiresat above the.envloader; it moved below it.test/asar-helpers.test.jsand Codex's P1 — the unpacked helper would have failed withMODULE_NOT_FOUNDin a packaged build; fixed by theasarUnpackentry above.test/kanban-group-button.test.js—buildChainFormis rendered in a bare sandbox that now takes the limit from the page instead of a copy of the number.Local full suite on Node 24.22.1, run suite by suite (90 suites): 89 pass, 1 skipped.
test/task-backlog.test.jsis the skip: it fails identically on a pristineorigin/mainon this machine (its second server never answers the 20 s health probe), and it passes on CI, which is where it belongs.CI on the merged head:
test (20)andtest (24)both success, GitGuardian success.Diff: 9 files, +261 / −21.