fix: persist add-reminder under the reminder API field (re-implements upstream #58 by @nico-fioretti) - #4
Merged
Conversation
An absolute reminder must be sent under the reminder key. The TaskReminder type calls it reminder_date, but the server expects reminder and otherwise stores a zero (0001-01-01) reminder that never fires. Reads/writes the reminder field and preserves existing reminders.
PR #58 changes add-reminder to persist under the 'reminder' API field instead of 'reminder_date'. Update the two existing add-reminder unit tests in tasks-reminders.test.ts to assert the new payload shape and to model existing reminders as Vikunja v2.3.0 returns them (reminder field). These two tests were the only net-new failures introduced by the field change; with them updated the branch matches main's test baseline exactly (no regressions).
This was referenced Jul 16, 2026
This was referenced Jul 18, 2026
netadvanced
added a commit
that referenced
this pull request
Jul 18, 2026
Battle-campaign friction #4 (q3-offsite-kanban / subtask-breakdown transcripts): agents made 6 one-by-one set-bucket calls and 3 one-by-one create-subtask calls despite readily using bulk-create/bulk-update elsewhere in the same run. Adds two composite-first (docs/ROADMAP.md decision 4) subcommands on vikunja_tasks, following PR #95's honest partial-reporting shape: - bulk-set-bucket (also exposed on vikunja_task_bulk, mirroring the existing bulk-create/update/delete dual registration): resolves the project + Kanban view ONCE, then applies the per-task bucket move sequentially (SQLite lock discipline), collecting failedIds on a partial result instead of aborting the whole batch. - bulk-create-subtasks: resolves the parent's project ONCE, then reuses create-subtask's own create -> label -> assign -> relate -> bucket -> verify CompositeOperation sequence per subtask spec, sequentially, each with its own independent rollback scope. atomic:true rolls back only the failing subtask, never earlier successes. Response lists which subtasks were created/related/failed. Both subcommands are classified 'write' in the global read-only classification table and covered by tool annotations. Updates the hard-coded vikunja_tasks/vikunja_task_bulk description-string assertions, docs/TOOLS.md, and adds mcp-e2e.ts checks (not run live). Gates: lint clean, typecheck clean, test:coverage 145 suites / 3148 tests passing (up from 143/3123 on unmodified main — zero net new regressions), aggregate coverage 89.84/80.81/78.45/90.13 (stmts/branches/funcs/lines), above the 89/80/77/89 ratcheted gate. Claude-Session: https://claude.ai/code/session_01Jn8SF2doyvho8vM9fwwFeJ
This was referenced Jul 20, 2026
netadvanced
added a commit
that referenced
this pull request
Jul 22, 2026
…overage (#159) Adds an `ensure` subcommand to vikunja_labels that get-or-creates a label by title in one idempotent call: GET /labels?s=<title> narrows candidates, a case-insensitive exact-title match is picked client-side (first match wins on duplicates), and only on a miss does it fall through to the same PUT /labels `create` uses. Reports clearly whether the label was reused or created, mirroring applyLabels' idempotent messaging style. Campaign evidence (tracking issue #28, friction #4): with no create-or-reuse primitive, agents were listing all labels, matching by title, then creating — costing both models 6 calls vs an optimal 3 (2x overage) for "attach a label by name". This collapses that flow to ensure + apply-label (2 calls). Also points vikunja_labels' and vikunja_task_labels' tool descriptions at the new ensure subcommand for the by-name-attach flow, classifies `ensure` as a 'write' subcommand in the read-only gate, documents the composite in docs/API-COVERAGE.md, and adds an mcp-e2e check that ensures the same label twice (plus a third differently-cased call) and confirms every call reuses the same id after the first create. Unit tests cover reuse-hit, create-miss, case-insensitive match, dedupe against multiple/substring search candidates, a null search response, and read-only-mode rejection. Gates: lint clean, typecheck clean, test:coverage 147/147 suites passing, 3324/3324 tests passing (net +7 vs the 3317-test unmodified base, 0 new failures). Claude-Session: https://claude.ai/code/session_014BfGcJGUddi6mnTe6QYLgs
netadvanced
added a commit
that referenced
this pull request
Jul 23, 2026
PR #159's `vikunja_labels ensure` subcommand cut existing-label-reuse friction (#28 #4) but a battle re-check showed weak agents still didn't adopt it: discovering `ensure`, calling it, then threading its id into a *separate* apply-label call was too many hops. This folds get-or-create directly into `vikunja_task_labels apply-label` via an optional `labelTitles: string[]` field, alongside the existing numeric `labels` field — either or both may be given. - Extract the get-or-create-by-title logic out of `ensure` into a shared `ensureLabelByTitle` helper (src/utils/label-ensure.ts). `ensure` now calls the helper instead of duplicating the logic; its existing test suite passes unchanged. - `applyLabels` resolves each `labelTitles` entry via the helper (case-insensitive title cache to avoid redundant lookups), merges the resolved ids with `labels`, dedupes, and attaches all of them through the existing idempotent apply path. The response reports which titles were created vs reused, alongside the pre-existing "already present" messaging. - Updated `vikunja_task_labels`/`vikunja_labels` tool descriptions and docs/TOOLS.md to point agents at the one-call `labelTitles` path; `ensure` remains documented for get-or-create-without-attaching. - Added a live labelTitles create/reuse/attach check to scripts/mcp-e2e.ts (not run — shared local stack is in use). Gates: lint clean, typecheck clean, test:coverage 149/149 suites, 3341/3341 tests (baseline origin/main: 147/147, 3324/3324 — net +2 suites/+17 tests, zero regressions). Claude-Session: https://claude.ai/code/session_014BfGcJGUddi6mnTe6QYLgs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-implements upstream PR #58 by @nico-fioretti (democratize-technology#58) — "fix: persist add-reminder date (send under
reminder, notreminder_date)" — as a fork PR. Full credit for the fix goes to the original author.What this fixes
An absolute reminder must be sent to Vikunja under the
reminderkey.node-vikunja's typedTaskRemindermodel calls the fieldreminder_date, so sending that key makes Vikunja (v2.3.0) silently store a zero (0001-01-01) reminder that never fires. This change reads and writes the actual APIreminderfield and preserves any existing reminders (absolute or relative) verbatim.Changes
src/tools/tasks/reminders.ts— cherry-picked verbatim from upstream PR Wave D: direct project sharing with users & teams + link-share REST migration #58 (@nico-fioretti).addRemindernow maps existing reminders through the realreminderfield and appends the new reminder as{ reminder: <date> }.tests/reminder-persistence.test.ts— new test from the upstream PR verifying the persisted payload usesreminder(passing).tests/tools/tasks-reminders.test.ts— follow-up commit updating the two existingadd-reminderunit tests to assert the newreminderpayload shape and to model existing reminders the way Vikunja v2.3.0 returns them. These two tests were the only net-new failures introduced by the field rename.Verification
npm run lint— passesnpm run typecheck— passestasks-reminders.test.ts+reminder-persistence.test.ts) — 22/22 passingmain(24 suites / 227 tests). The fix introduces zero net-new failures / zero regressions.Known limitation (pre-existing, out of scope)
npm run test:coveragedoes not pass repo-wide becausemainitself currently has ~227 failing tests with coverage below the configured thresholds, from long-standing source-vs-test drift unrelated to reminders (e.g.SecureErrorHandlerstring handling, removed default-subcommand branches, circuit-breaker test isolation). This drift predates PR #58 and is tracked separately in the upstream-adoption triage. This PR is intentionally scoped to the reminder fix only and does not attempt to repair that unrelated debt.