From cc5cef951cd5cd356094d4a75ddf4ffcea5e4f70 Mon Sep 17 00:00:00 2001 From: ipreuss Date: Tue, 14 Apr 2026 11:37:49 +0200 Subject: [PATCH 1/3] feat: add update command for non-interactive field updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `tk update --field=value` for modifying YAML frontmatter fields without opening an editor. Useful for AI agents and scripts. Features: - Update single or multiple fields in one command - JSON array syntax with jq: --tags='["a", "b"]' → tags: [a, b] - Graceful fallback when jq unavailable - Proper error handling for invalid arguments/JSON Also fixes update_yaml_field to work on macOS/BSD by using awk instead of GNU-specific sed syntax for inserting new fields. Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 5 +++ README.md | 2 + features/ticket_update.feature | 58 ++++++++++++++++++++++++++++ ticket | 69 ++++++++++++++++++++++++++++++++-- 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 features/ticket_update.feature diff --git a/CHANGELOG.md b/CHANGELOG.md index f25558fc..1855ece6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,15 @@ ## [Unreleased] +### Fixed +- `update_yaml_field` now works on macOS/BSD (awk instead of GNU-specific sed for field insertion) + ### Changed - Extracted `edit`, `ls`, `query`, and `migrate-beads` commands to plugins (ticket-extras) ### Added +- `update` command for non-interactive YAML field updates (`tk update --field=value`) +- JSON array syntax support in update command (requires jq): `--tags='["a", "b"]'` - Plugin system: executables named `tk-` or `ticket-` in PATH are invoked automatically - `super` command to bypass plugins and run built-in commands directly - `TICKETS_DIR` and `TK_SCRIPT` environment variables exported for plugins diff --git a/README.md b/README.md index 2d7017e5..99346cba 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,8 @@ Commands: close Set status to closed reopen Set status to open status Update status (open|in_progress|closed) + update --field=value Update YAML field(s) non-interactively + Arrays: --field='["a", "b"]' (JSON syntax) dep Add dependency (id depends on dep-id) dep tree [--full] Show dependency tree (--full disables dedup) dep cycle Find dependency cycles in open tickets diff --git a/features/ticket_update.feature b/features/ticket_update.feature new file mode 100644 index 00000000..fcca897a --- /dev/null +++ b/features/ticket_update.feature @@ -0,0 +1,58 @@ +Feature: Ticket Update Command + As a user or automation script + I want to update ticket fields non-interactively + So that I can modify tickets without opening an editor + + Background: + Given a clean tickets directory + And a ticket exists with ID "test-0001" and title "Test ticket" + + Scenario: Update existing field + When I run "ticket update test-0001 --priority=1" + Then the command should succeed + And the output should be "Updated 1 field(s) on test-0001" + And ticket "test-0001" should have field "priority" with value "1" + + Scenario: Update multiple fields + When I run "ticket update test-0001 --priority=1 --assignee=alice" + Then the command should succeed + And the output should be "Updated 2 field(s) on test-0001" + And ticket "test-0001" should have field "priority" with value "1" + And ticket "test-0001" should have field "assignee" with value "alice" + + Scenario: Add new custom field + When I run "ticket update test-0001 --custom_field=myvalue" + Then the command should succeed + And the output should be "Updated 1 field(s) on test-0001" + And ticket "test-0001" should have field "custom_field" with value "myvalue" + + Scenario: Update with JSON array (requires jq) + When I run "ticket update test-0001 '--tags=[\"bug\",\"urgent\"]'" + Then the command should succeed + And the output should be "Updated 1 field(s) on test-0001" + And ticket "test-0001" should have field "tags" with value "[bug, urgent]" + + Scenario: Invalid JSON array + When I run "ticket update test-0001 '--tags=[\"unclosed]'" + Then the command should fail + And the output should contain "Error: invalid JSON for tags" + + Scenario: Unknown argument without --field=value format + When I run "ticket update test-0001 badarg" + Then the command should fail + And the output should contain "Error: unknown argument 'badarg'" + + Scenario: Missing field arguments + When I run "ticket update test-0001" + Then the command should fail + And the output should contain "Usage:" + + Scenario: Update non-existent ticket + When I run "ticket update nonexistent --priority=1" + Then the command should fail + And the output should contain "Error: ticket 'nonexistent' not found" + + Scenario: Update with partial ID + When I run "ticket update 0001 --priority=0" + Then the command should succeed + And ticket "test-0001" should have field "priority" with value "0" diff --git a/ticket b/ticket index 0aea72c1..ba12c800 100755 --- a/ticket +++ b/ticket @@ -148,9 +148,12 @@ update_yaml_field() { _sed_i "$file" "s/^${field}:.*/${field}: ${value}/" else # Insert after first --- (beginning of frontmatter) - _sed_i "$file" "0,/^---$/ { /^---$/a\\ -${field}: ${value} -}" + # Using awk for BSD/GNU portability (sed 0,/pat/ and a\ differ) + local tmp="${file}.tmp.$$" + awk -v field="$field" -v value="$value" ' + NR==1 && /^---$/ { print; print field ": " value; next } + { print } + ' "$file" > "$tmp" && mv "$tmp" "$file" fi } @@ -1280,6 +1283,8 @@ Commands: close Set status to closed reopen Set status to open status Update status (open|in_progress|closed) + update --field=value Update YAML field(s) non-interactively + Arrays: --field='["a", "b"]' (JSON syntax) dep Add dependency (id depends on dep-id) dep tree [--full] Show dependency tree (--full disables dedup) dep cycle Find dependency cycles in open tickets @@ -1317,6 +1322,63 @@ Supports partial ID matching (e.g., '$cmd show 5c4' matches 'nw-5c46') EOF } +cmd_update() { + if [[ $# -lt 2 ]]; then + echo "Usage: $(basename "$0") update --field=value [--field2=value2 ...]" >&2 + return 1 + fi + + local file + file=$(ticket_path "$1") || return 1 + shift + + local updated=0 + while [[ $# -gt 0 ]]; do + case "$1" in + --*=*) + local arg="${1#--}" + local field="${arg%%=*}" + local value="${arg#*=}" + + # Handle JSON array syntax -> YAML flow array + if [[ "$value" =~ ^\[.*\]$ ]]; then + if command -v jq &>/dev/null; then + local yaml_val + yaml_val=$(echo "$value" | jq -r ' + if type == "array" then + "[" + (map(tostring) | join(", ")) + "]" + else + . + end + ' 2>/dev/null) || { + echo "Error: invalid JSON for $field" >&2 + return 1 + } + value="$yaml_val" + fi + # If no jq, pass value as-is (user's responsibility) + fi + + update_yaml_field "$file" "$field" "$value" + ((updated++)) + shift + ;; + *) + echo "Error: unknown argument '$1'" >&2 + echo "Usage: $(basename "$0") update --field=value" >&2 + return 1 + ;; + esac + done + + if [[ $updated -gt 0 ]]; then + echo "Updated $updated field(s) on $(basename "$file" .md)" + else + echo "No fields specified" >&2 + return 1 + fi +} + # Main dispatch # Handle 'super' to bypass plugins @@ -1373,6 +1435,7 @@ case "${1:-help}" in blocked) shift; cmd_blocked "$@" ;; closed) shift; cmd_closed "$@" ;; show) shift; cmd_show "$@" ;; + update) shift; cmd_update "$@" ;; add-note) shift; cmd_add_note "$@" ;; help|--help|-h) cmd_help ;; *) From f4774ea6153673ffe201aeace33eea79f1caac59 Mon Sep 17 00:00:00 2001 From: ipreuss Date: Tue, 21 Apr 2026 20:52:15 +0200 Subject: [PATCH 2/3] fix(update): address PR #59 review findings 1-6 - Exit 0 on success (updated=$((updated+1)) instead of ((updated++))) - awk-based value replacement in both branches of update_yaml_field (eliminates sed metacharacter hazards: & / \) - Hybrid field policy via validate_field_update helper: - immutable: id, created - routed: status, deps, links - body-backed (rejected): title, description, design, acceptance - validated: priority (0-4), parent (must exist), type (known values) - freeform: everything else with [A-Za-z][A-Za-z0-9_-]* name - Reject array items with embedded commas (jq + no-jq paths) - Add 24 regression scenarios Ticket: M3R-7iue --- features/ticket_update.feature | 118 +++++++++++++++++++++++++++ ticket | 143 ++++++++++++++++++++++++++++++--- 2 files changed, 251 insertions(+), 10 deletions(-) diff --git a/features/ticket_update.feature b/features/ticket_update.feature index fcca897a..bc413f7f 100644 --- a/features/ticket_update.feature +++ b/features/ticket_update.feature @@ -56,3 +56,121 @@ Feature: Ticket Update Command When I run "ticket update 0001 --priority=0" Then the command should succeed And ticket "test-0001" should have field "priority" with value "0" + + Scenario: Exit code is 0 on successful single update (set -e regression) + When I run "ticket update test-0001 --priority=1" + Then the command should succeed + + Scenario: Exit code is 0 on successful multiple updates + When I run "ticket update test-0001 --priority=1 --assignee=alice" + Then the command should succeed + + Scenario: Value containing ampersand is stored literally (sed & metachar) + When I run "ticket update test-0001 --assignee='A & B'" + Then the command should succeed + And ticket "test-0001" should have field "assignee" with value "A & B" + + Scenario: Value containing forward slash is stored literally (sed / separator) + When I run "ticket update test-0001 --external-ref='https://example.com/path'" + Then the command should succeed + And ticket "test-0001" should have field "external-ref" with value "https://example.com/path" + + Scenario: Cannot update id field (immutable) + When I run "ticket update test-0001 --id=hacker" + Then the command should fail + And the output should contain "Error: field 'id' is immutable" + + Scenario: Cannot update created field (immutable) + When I run "ticket update test-0001 --created=2020-01-01" + Then the command should fail + And the output should contain "immutable" + + Scenario: Cannot update status (routes to tk status) + When I run "ticket update test-0001 --status=closed" + Then the command should fail + And the output should contain "use 'tk status" + + Scenario: Cannot update deps (routes to tk dep) + When I run "ticket update test-0001 '--deps=[\"other\"]'" + Then the command should fail + And the output should contain "use 'tk dep" + + Scenario: Cannot update links (routes to tk link) + When I run "ticket update test-0001 '--links=[\"other\"]'" + Then the command should fail + And the output should contain "use 'tk link" + + Scenario: Cannot update title (body content, not YAML) + When I run "ticket update test-0001 --title=Renamed" + Then the command should fail + And the output should contain "markdown body" + + Scenario: Cannot update description (body content, not YAML) + When I run "ticket update test-0001 --description=text" + Then the command should fail + And the output should contain "markdown body" + + Scenario: Cannot update design (body content, not YAML) + When I run "ticket update test-0001 --design=text" + Then the command should fail + And the output should contain "markdown body" + + Scenario: Cannot update acceptance (body content, not YAML) + When I run "ticket update test-0001 --acceptance=text" + Then the command should fail + And the output should contain "markdown body" + + Scenario: Priority must be 0-4 + When I run "ticket update test-0001 --priority=99" + Then the command should fail + And the output should contain "priority must be 0-4" + + Scenario: Priority accepts valid range + When I run "ticket update test-0001 --priority=0" + Then the command should succeed + And ticket "test-0001" should have field "priority" with value "0" + + Scenario: Type must be one of known values + When I run "ticket update test-0001 --type=banana" + Then the command should fail + And the output should contain "must be one of: bug, feature, task, epic, chore" + + Scenario: Parent must reference existing ticket + When I run "ticket update test-0001 --parent=nonexistent" + Then the command should fail + And the output should contain "parent ticket 'nonexistent' not found" + + Scenario: Parent can reference an existing ticket (positive path) + Given a ticket exists with ID "test-0002" and title "Parent ticket" + When I run "ticket update test-0001 --parent=test-0002" + Then the command should succeed + And ticket "test-0001" should have field "parent" with value "test-0002" + + Scenario: Parent can be unset with empty value + When I run "ticket update test-0001 --parent=" + Then the command should succeed + + Scenario: Invalid field name syntax is rejected (regex injection guard) + When I run "ticket update test-0001 '--foo.bar=baz'" + Then the command should fail + And the output should contain "invalid field name" + + Scenario: Field-name validation precedes JSON validation (ordering) + When I run "ticket update test-0001 '--foo.bar=[1,2,3]'" + Then the command should fail + And the output should contain "invalid field name" + + Scenario: Reject array items with embedded commas (ecosystem limitation) + When I run "ticket update test-0001 '--tags=[\"urgent, internal\"]'" + Then the command should fail + And the output should contain "must not contain commas" + + Scenario: Comma-in-item error is specific, not generic (jq error coupling) + When I run "ticket update test-0001 '--tags=[\"a, b\"]'" + Then the command should fail + And the output should contain "must not contain commas" + + Scenario: Custom field names are allowed (hybrid policy, freeform category) + When I run "ticket update test-0001 --sprint=42" + Then the command should succeed + And ticket "test-0001" should have field "sprint" with value "42" diff --git a/ticket b/ticket index ba12c800..7736f4d2 100755 --- a/ticket +++ b/ticket @@ -139,24 +139,114 @@ yaml_field() { } # Update YAML field +# +# Both branches use awk with -v to pass field/value as variables. This avoids +# sed's s/// metacharacter hazards (&, /, \) that would corrupt values or +# break the separator. First-match-wins: for well-formed frontmatter with a +# single definition per field, behavior is identical to a global replace. +# For accidentally duplicated keys, the canonical first entry stays +# authoritative and the duplicate remains visible as an artifact rather than +# being silently mirrored. update_yaml_field() { local file="$1" local field="$2" local value="$3" + local tmp="${file}.tmp.$$" if _grep -q "^${field}:" "$file"; then - _sed_i "$file" "s/^${field}:.*/${field}: ${value}/" + # Replace existing field — exit 2 if no match found, to prevent + # silently writing an unchanged file (defense against encoding/ + # race issues that slip past the _grep gate). + awk -v field="$field" -v value="$value" ' + !done && $0 ~ "^" field ":" { print field ": " value; done=1; next } + { print } + END { if (!done) exit 2 } + ' "$file" > "$tmp" && mv "$tmp" "$file" else # Insert after first --- (beginning of frontmatter) - # Using awk for BSD/GNU portability (sed 0,/pat/ and a\ differ) - local tmp="${file}.tmp.$$" awk -v field="$field" -v value="$value" ' - NR==1 && /^---$/ { print; print field ": " value; next } + NR==1 && /^---$/ { print; print field ": " value; done=1; next } { print } + END { if (!done) exit 2 } ' "$file" > "$tmp" && mv "$tmp" "$file" fi } +# Classify and validate a proposed field update. Categories: +# immutable — id, created: breaks file-name coupling or audit trail +# routed — status, deps, links: has dedicated command with validation +# body-backed — title, description, design, acceptance: live in markdown +# body (not YAML frontmatter); writing them to YAML +# creates desync (cmd_create emits them as body sections) +# validated — priority, parent, type: YAML-backed, value must match rules +# freeform — anything else, as long as the key name is parser-safe +# +# Returns 0 if the update is allowed, 1 with a clear error otherwise. +validate_field_update() { + local field="$1" + local value="$2" + + # Guard: field-name syntax — prevents regex-injection into update_yaml_field + # (field is interpolated into awk's regex "^" field ":") + if [[ ! "$field" =~ ^[A-Za-z][A-Za-z0-9_-]*$ ]]; then + echo "Error: invalid field name '$field' (allowed: [A-Za-z][A-Za-z0-9_-]*)" >&2 + return 1 + fi + + case "$field" in + # immutable + id|created) + echo "Error: field '$field' is immutable (would break file-name coupling / audit trail)" >&2 + return 1 + ;; + # routed to dedicated commands + status) + echo "Error: use 'tk status ' to update status (validates allowed values)" >&2 + return 1 + ;; + deps) + echo "Error: use 'tk dep ' to add dependencies (validates existence)" >&2 + return 1 + ;; + links) + echo "Error: use 'tk link ' to link tickets (maintains symmetric backlinks)" >&2 + return 1 + ;; + # body-backed (not YAML frontmatter) + title|description|design|acceptance) + echo "Error: '$field' is stored in the ticket's markdown body, not frontmatter. Use 'tk edit ' to modify body sections" >&2 + return 1 + ;; + # validated + priority) + if [[ ! "$value" =~ ^[0-4]$ ]]; then + echo "Error: priority must be 0-4 (got '$value')" >&2 + return 1 + fi + ;; + type) + case "$value" in + bug|feature|task|epic|chore) ;; + *) + echo "Error: type must be one of: bug, feature, task, epic, chore (got '$value')" >&2 + return 1 + ;; + esac + ;; + parent) + # Parent must be an existing ticket (empty value allowed to unset) + if [[ -n "$value" ]]; then + if ! ticket_path "$value" >/dev/null 2>&1; then + echo "Error: parent ticket '$value' not found" >&2 + return 1 + fi + fi + ;; + # freeform custom fields — pass through + esac + return 0 +} + cmd_create() { ensure_dir @@ -1340,27 +1430,60 @@ cmd_update() { local field="${arg%%=*}" local value="${arg#*=}" + # Validate field-name syntax, classify category, validate value. + # Runs BEFORE JSON array handling so that "invalid field name" + # errors take priority over "invalid JSON" errors — specific + # diagnosis beats generic. + validate_field_update "$field" "$value" || return 1 + # Handle JSON array syntax -> YAML flow array if [[ "$value" =~ ^\[.*\]$ ]]; then if command -v jq &>/dev/null; then + # jq exits non-zero on invalid JSON. It also exits + # non-zero via error() if any array item contains a + # comma — comma-in-item would corrupt round-trip + # through consumers that split on ", *" (ticket-query, + # ticket-ls, internal awk splits). local yaml_val - yaml_val=$(echo "$value" | jq -r ' + yaml_val=$(echo "$value" | jq -er ' if type == "array" then - "[" + (map(tostring) | join(", ")) + "]" + if any(.[] | tostring; test(",")) then + error("array item contains comma") + else + "[" + (map(tostring) | join(", ")) + "]" + end else . end - ' 2>/dev/null) || { - echo "Error: invalid JSON for $field" >&2 + ' 2>&1) || { + if echo "$yaml_val" | grep -q "array item contains comma"; then + echo "Error: array items for '$field' must not contain commas. The ticket ecosystem stores arrays as comma-separated values and has no quote-aware parsing in consumers (ticket-query, ticket-ls). Use dashes or underscores instead." >&2 + else + echo "Error: invalid JSON for $field" >&2 + fi return 1 } value="$yaml_val" + else + # This branch is exercised only when jq is absent; + # manual verification required on a jq-less system. + # Still guard the contract: reject items that contain + # commas, since downstream consumers split on commas. + local inner="${value#[}" + inner="${inner%]}" + # Require 1+ non-quote chars on each side of the comma: + # that anchors the comma INSIDE a quoted string and + # excludes the separator case '","' between items. + if echo "$inner" | grep -qE '"[^"]+,[^"]+"'; then + echo "Error: array items for '$field' must not contain commas (ecosystem limitation)" >&2 + return 1 + fi + # Value passes through as-is (user's responsibility beyond comma check) fi - # If no jq, pass value as-is (user's responsibility) fi update_yaml_field "$file" "$field" "$value" - ((updated++)) + updated=$((updated + 1)) shift ;; *) From eb531bf97a156a2bdab08a5bf7a646ab0aefde27 Mon Sep 17 00:00:00 2001 From: ipreuss Date: Sun, 26 Apr 2026 04:27:27 +0200 Subject: [PATCH 3/3] fix(update): emit valid YAML flow arrays with quoted values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the comma-rejection guard (added during PR #59 review) with structural quoting via jq @json. Values with commas, quotes, or backslashes now roundtrip losslessly as YAML 1.2 flow sequences (also valid JSON arrays) for non-iterated freeform fields. The 'tags' field stays comma-rejecting because tk ls -T, cmd_ready, cmd_blocked, and cmd_show iterate tag values via comma-split. The new error message names the readers explicitly. release_notes and other freeform array fields accept commas via storage roundtrip honored by tk query. Backward compatibility for existing .tickets/*.md: - Tags/deps/links readers strip quotes via widened gsub char class ([\[\] ] -> [\[\] "]); legacy unquoted and new quoted forms both parse to bare CSV for existing split() consumers. - ticket-query detects quoted form via /^\\[[[:space:]]*\"/ and emits verbatim (already valid JSON); legacy falls through to existing split-on-comma branch. cmd_update now requires jq for JSON-array values. Previously, jq absence triggered a regex-based fallback that accepted commas inconsistently. Fail-fast aligns with the rest of the codebase. update_yaml_field switched from awk -v to ENVIRON for the value, preserving JSON escape sequences (\", \\, \\n) byte-exact. Without this, awk -v's C-style escape processing would corrupt stored YAML when arrays contain quoted strings. 5 new behave scenarios: - Comma-in-value roundtrip (release_notes) - ticket-query on new quoted form - ticket-query on legacy unquoted form (regression guard) - ls -T on new quoted tags - ls -T on legacy unquoted tags (regression guard) Plus 3 scenarios codifying the tags-vs-freeform asymmetry: - tags array items rejected with comma (reader limitation) - comma-in-tag error names the reason - comma-in-value allowed for non-tag freeform fields Test framework: should-have-field-with-value and output-should-contain steps now support \" escapes in expected values, mirroring the existing 'I run' step. Two existing scenarios deleted (Reject array items with embedded commas, Comma-in-item error is specific) — their contract is replaced by the tags-specific case in validate_field_update. Full suite: 163/163 (was 157 baseline). --- CHANGELOG.md | 7 ++ features/steps/ticket_steps.py | 11 ++- features/ticket_listing.feature | 13 ++++ features/ticket_query.feature | 13 ++++ features/ticket_update.feature | 17 ++++- plugins/ticket-ls | 4 +- plugins/ticket-query | 22 ++++-- ticket | 126 +++++++++++++++++--------------- 8 files changed, 137 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1855ece6..b9229a20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,15 @@ ### Fixed - `update_yaml_field` now works on macOS/BSD (awk instead of GNU-specific sed for field insertion) +- `update` command for non-iterated freeform array fields (e.g. `release_notes`) no longer rejects values whose items contain commas. Storage uses structural quoting via `jq @json`, which roundtrips losslessly as valid YAML 1.2 flow sequences (also valid JSON arrays). `tk query` returns the values verbatim. +- `update_yaml_field` now uses `ENVIRON` instead of `awk -v` for the value, preserving JSON escape sequences (`\"`, `\\`, `\n`) byte-exact. Previously, `awk -v` would unescape those sequences and corrupt stored YAML when arrays contained quoted strings. ### Changed +- `update` command now emits valid YAML flow sequences with JSON-escaped values (via `jq @json`). Previously the output was bare CSV with no quoting, which lost value boundaries when items contained commas. +- `update` command now requires `jq` for JSON-array values (`--field='[…]'`). Previously, `jq` absence triggered a regex-based fallback that accepted commas-in-items inconsistently. The fail-fast error directs users to install `jq`. Scalar updates are unaffected. +- `ticket-query` now detects the new quoted form (`["bug", "urgent"]`) and emits it verbatim as already-valid JSON. Legacy unquoted `tickets/*.md` (`[bug, urgent]`) keep working via the existing comma-split branch — no migration required. +- Tag/deps/links readers in `cmd_ready`, `cmd_closed`, `cmd_blocked`, `cmd_show`, `cmd_dep_tree`, `cmd_dep_cycle`, and `plugins/ticket-ls` now widen the gsub character class from `[\[\] ]` to `[\[\] "]`, accepting both quoted and unquoted inline arrays. +- `update` command continues to reject comma-in-item for `tags` specifically (reader limitation: `tk ls -T`, `cmd_ready`, `cmd_blocked`, `cmd_show` split tag values on comma). The new error message names the reader limitation explicitly. Other freeform array fields are unaffected. - Extracted `edit`, `ls`, `query`, and `migrate-beads` commands to plugins (ticket-extras) ### Added diff --git a/features/steps/ticket_steps.py b/features/steps/ticket_steps.py index 54d3ab3f..15550496 100644 --- a/features/steps/ticket_steps.py +++ b/features/steps/ticket_steps.py @@ -356,16 +356,18 @@ def step_output_empty(context): assert context.stdout == '', f"Expected empty output but got: {context.stdout}" -@then(r'the output should contain "(?P[^"]+)"') +@then(r'the output should contain "(?P(?:[^"\\]|\\.)+)"') def step_output_contains(context, text): """Assert output contains text.""" + text = text.replace('\\"', '"').replace('\\\\', '\\') output = context.stdout + context.stderr assert text in output, f"Expected output to contain '{text}'\nActual output: {output}" -@then(r'the output should not contain "(?P[^"]+)"') +@then(r'the output should not contain "(?P(?:[^"\\]|\\.)+)"') def step_output_not_contains(context, text): """Assert output does not contain text.""" + text = text.replace('\\"', '"').replace('\\\\', '\\') output = context.stdout + context.stderr assert text not in output, f"Expected output to NOT contain '{text}'\nActual output: {output}" @@ -456,9 +458,12 @@ def step_created_ticket_has_timestamp(context): f"No valid created timestamp found\nContent: {content}" -@then(r'ticket "(?P[^"]+)" should have field "(?P[^"]+)" with value "(?P[^"]+)"') +@then(r'ticket "(?P[^"]+)" should have field "(?P[^"]+)" with value "(?P(?:[^"\\]|\\.)*)"') def step_ticket_has_field_value(context, ticket_id, field, value): """Assert ticket has a field with specific value.""" + # Unescape \" to " (and \\ to \) in the expected value, mirroring `I run` + value = value.replace('\\"', '"').replace('\\\\', '\\') + ticket_path = Path(context.test_dir) / '.tickets' / f'{ticket_id}.md' content = ticket_path.read_text() diff --git a/features/ticket_listing.feature b/features/ticket_listing.feature index 7c78852d..966e8f34 100644 --- a/features/ticket_listing.feature +++ b/features/ticket_listing.feature @@ -161,3 +161,16 @@ Feature: Ticket Listing When I run "ticket closed" Then the command should succeed And the output should not contain "done-0001" + + Scenario: ls -T filter works with quoted tags (after tk update) + Given a ticket exists with ID "tag-001" and title "Quoted tags" + When I run "ticket update tag-001 '--tags=[\"bug\",\"critical\"]'" + And I run "ticket ls -T critical" + Then the command should succeed + And the output should contain "tag-001" + + Scenario: ls -T filter works with legacy unquoted tags (backward compat) + When I run "ticket create 'Legacy tags' --tags bug,urgent" + And I run "ticket ls -T urgent" + Then the command should succeed + And the output should contain "Legacy tags" diff --git a/features/ticket_query.feature b/features/ticket_query.feature index 2ccafe70..6a583b7b 100644 --- a/features/ticket_query.feature +++ b/features/ticket_query.feature @@ -46,3 +46,16 @@ Feature: Ticket Query When I run "ticket query" Then the command should succeed And the JSONL deps field should be a JSON array + + Scenario: Query emits valid JSON for new quoted tags (after tk update) + Given a ticket exists with ID "query-001" and title "Quoted tags" + When I run "ticket update query-001 '--tags=[\"bug\",\"urgent\"]'" + And I run "ticket query" + Then the command should succeed + And the output should contain "\"tags\":[\"bug\", \"urgent\"]" + + Scenario: Query still handles legacy unquoted tags (bare CSV, backward compat) + When I run "ticket create 'Legacy tags' --tags bug,urgent" + And I run "ticket query" + Then the command should succeed + And the output should contain "\"tags\":[\"bug\",\"urgent\"]" diff --git a/features/ticket_update.feature b/features/ticket_update.feature index bc413f7f..9f47b98b 100644 --- a/features/ticket_update.feature +++ b/features/ticket_update.feature @@ -30,7 +30,13 @@ Feature: Ticket Update Command When I run "ticket update test-0001 '--tags=[\"bug\",\"urgent\"]'" Then the command should succeed And the output should be "Updated 1 field(s) on test-0001" - And ticket "test-0001" should have field "tags" with value "[bug, urgent]" + And ticket "test-0001" should have field "tags" with value "[\"bug\", \"urgent\"]" + + Scenario: JSON array with commas in values roundtrip + When I run "ticket update test-0001 '--release_notes=[\"First entry, with comma.\", \"Second entry.\"]'" + Then the command should succeed + And the output should be "Updated 1 field(s) on test-0001" + And ticket "test-0001" should have field "release_notes" with value "[\"First entry, with comma.\", \"Second entry.\"]" Scenario: Invalid JSON array When I run "ticket update test-0001 '--tags=[\"unclosed]'" @@ -160,16 +166,21 @@ Feature: Ticket Update Command Then the command should fail And the output should contain "invalid field name" - Scenario: Reject array items with embedded commas (ecosystem limitation) + Scenario: tags array items must not contain commas (reader limitation) When I run "ticket update test-0001 '--tags=[\"urgent, internal\"]'" Then the command should fail And the output should contain "must not contain commas" - Scenario: Comma-in-item error is specific, not generic (jq error coupling) + Scenario: Comma-in-tag rejection error names the reason (not generic JSON error) When I run "ticket update test-0001 '--tags=[\"a, b\"]'" Then the command should fail And the output should contain "must not contain commas" + Scenario: Comma-in-value is allowed for non-tag freeform fields (release_notes) + When I run "ticket update test-0001 '--release_notes=[\"First, with comma.\", \"Second.\"]'" + Then the command should succeed + And ticket "test-0001" should have field "release_notes" with value "[\"First, with comma.\", \"Second.\"]" + Scenario: Custom field names are allowed (hybrid policy, freeform category) When I run "ticket update test-0001 --sprint=42" Then the command should succeed diff --git a/plugins/ticket-ls b/plugins/ticket-ls index 5bb44b1c..03a58f5b 100755 --- a/plugins/ticket-ls +++ b/plugins/ticket-ls @@ -26,10 +26,10 @@ FNR==1 { in_front && /^id:/ { id = $2 } in_front && /^status:/ { status = $2 } in_front && /^assignee:/ { assignee = $2 } -in_front && /^tags:/ { tags = $2; gsub(/[\[\] ]/, "", tags) } +in_front && /^tags:/ { tags = $2; gsub(/[\[\] "]/, "", tags) } in_front && /^deps:/ { deps = $2 - gsub(/[\[\] ]/, "", deps) + gsub(/[\[\] "]/, "", deps) } !in_front && /^# / && title == "" { title = substr($0, 3) } END { if (prev_file) emit() } diff --git a/plugins/ticket-query b/plugins/ticket-query index 342c1b4f..fd94d3e3 100755 --- a/plugins/ticket-query +++ b/plugins/ticket-query @@ -31,15 +31,21 @@ function emit() { val = field_vals[i] # Handle arrays if (val ~ /^\[.*\]$/) { - gsub(/^\[|\]$/, "", val) - n = split(val, items, ", *") - printf "\"%s\":[", key - for (j = 1; j <= n; j++) { - if (j > 1) printf "," - gsub(/^ +| +$/, "", items[j]) - if (items[j] != "") printf "\"%s\"", items[j] + if (val ~ /^\[[[:space:]]*"/) { + # New quoted form — already valid JSON, emit verbatim. + printf "\"%s\":%s", key, val + } else { + # Legacy unquoted CSV — split on comma. + gsub(/^\[|\]$/, "", val) + n = split(val, items, ", *") + printf "\"%s\":[", key + for (j = 1; j <= n; j++) { + if (j > 1) printf "," + gsub(/^ +| +$/, "", items[j]) + if (items[j] != "") printf "\"%s\"", items[j] + } + printf "]" } - printf "]" } else { printf "\"%s\":\"%s\"", key, val } diff --git a/ticket b/ticket index 7736f4d2..40d871b6 100755 --- a/ticket +++ b/ticket @@ -140,13 +140,16 @@ yaml_field() { # Update YAML field # -# Both branches use awk with -v to pass field/value as variables. This avoids -# sed's s/// metacharacter hazards (&, /, \) that would corrupt values or -# break the separator. First-match-wins: for well-formed frontmatter with a -# single definition per field, behavior is identical to a global replace. -# For accidentally duplicated keys, the canonical first entry stays -# authoritative and the duplicate remains visible as an artifact rather than -# being silently mirrored. +# Field is passed via -v (parser-safe ASCII per validate_field_update). Value +# is passed via ENVIRON to avoid awk's -v C-style escape processing — values +# from cmd_update may contain JSON escapes (\", \\, \n) that -v would +# unescape, corrupting the stored YAML. ENVIRON delivers the byte-exact +# string. This avoids sed's s/// metacharacter hazards (&, /, \) too. +# First-match-wins: for well-formed frontmatter with a single definition per +# field, behavior is identical to a global replace. For accidentally +# duplicated keys, the canonical first entry stays authoritative and the +# duplicate remains visible as an artifact rather than being silently +# mirrored. update_yaml_field() { local file="$1" local field="$2" @@ -157,14 +160,16 @@ update_yaml_field() { # Replace existing field — exit 2 if no match found, to prevent # silently writing an unchanged file (defense against encoding/ # race issues that slip past the _grep gate). - awk -v field="$field" -v value="$value" ' + TK_FIELD_VALUE="$value" awk -v field="$field" ' + BEGIN { value = ENVIRON["TK_FIELD_VALUE"] } !done && $0 ~ "^" field ":" { print field ": " value; done=1; next } { print } END { if (!done) exit 2 } ' "$file" > "$tmp" && mv "$tmp" "$file" else # Insert after first --- (beginning of frontmatter) - awk -v field="$field" -v value="$value" ' + TK_FIELD_VALUE="$value" awk -v field="$field" ' + BEGIN { value = ENVIRON["TK_FIELD_VALUE"] } NR==1 && /^---$/ { print; print field ": " value; done=1; next } { print } END { if (!done) exit 2 } @@ -242,6 +247,22 @@ validate_field_update() { fi fi ;; + tags) + # tags is reader-iterated by `tk ls -T`, `cmd_ready`, `cmd_blocked`, + # `cmd_show`: those readers strip [] " and split on comma, so an + # item containing a comma would be silently fragmented (and + # produce phantom -T matches). Storage roundtrip via @json works, + # but reader roundtrip does not. Reject comma-in-item up front. + # Freeform fields (release_notes, etc.) have no reader iteration + # and are unaffected — they pass through this case (no `tags)` + # match for them) into the freeform pass-through below. + if [[ "$value" =~ ^\[.*\]$ ]] && command -v jq &>/dev/null; then + if printf '%s\n' "$value" | jq -e 'type == "array" and any(.[]; tostring | test(","))' >/dev/null 2>&1; then + echo "Error: tags array items must not contain commas (reader limitation: 'tk ls -T', cmd_ready, cmd_blocked split tag values on comma). Use dashes or underscores instead." >&2 + return 1 + fi + fi + ;; # freeform custom fields — pass through esac return 0 @@ -409,7 +430,7 @@ cmd_dep_tree() { in_front && /^status:/ { status = $2 } in_front && /^deps:/ { deps = $2 - gsub(/[\[\] ]/, "", deps) + gsub(/[\[\] "]/, "", deps) } !in_front && /^# / && title == "" { title = substr($0, 3) } function store() { @@ -593,7 +614,7 @@ cmd_dep_cycle() { in_front && /^status:/ { status = $2 } in_front && /^deps:/ { deps = $2 - gsub(/[\[\] ]/, "", deps) + gsub(/[\[\] "]/, "", deps) } !in_front && /^# / && title == "" { title = substr($0, 3) } function store() { @@ -767,10 +788,10 @@ cmd_ready() { in_front && /^status:/ { status = $2 } in_front && /^priority:/ { priority = $2 } in_front && /^assignee:/ { assignee = $2 } - in_front && /^tags:/ { tags = $2; gsub(/[\[\] ]/, "", tags) } + in_front && /^tags:/ { tags = $2; gsub(/[\[\] "]/, "", tags) } in_front && /^deps:/ { deps = $2 - gsub(/[\[\] ]/, "", deps) + gsub(/[\[\] "]/, "", deps) } !in_front && /^# / && title == "" { title = substr($0, 3) } function has_tag(tags_str, tag, i, n, arr) { @@ -861,7 +882,7 @@ cmd_closed() { in_front && /^id:/ { id = $2 } in_front && /^status:/ { status = $2 } in_front && /^assignee:/ { assignee = $2 } - in_front && /^tags:/ { tags = $2; gsub(/[\[\] ]/, "", tags) } + in_front && /^tags:/ { tags = $2; gsub(/[\[\] "]/, "", tags) } !in_front && /^# / && title == "" { title = substr($0, 3) } function has_tag(tags_str, tag, i, n, arr) { n = split(tags_str, arr, ",") @@ -904,10 +925,10 @@ cmd_blocked() { in_front && /^status:/ { status = $2 } in_front && /^priority:/ { priority = $2 } in_front && /^assignee:/ { assignee = $2 } - in_front && /^tags:/ { tags = $2; gsub(/[\[\] ]/, "", tags) } + in_front && /^tags:/ { tags = $2; gsub(/[\[\] "]/, "", tags) } in_front && /^deps:/ { deps = $2 - gsub(/[\[\] ]/, "", deps) + gsub(/[\[\] "]/, "", deps) } !in_front && /^# / && title == "" { title = substr($0, 3) } function has_tag(tags_str, tag, i, n, arr) { @@ -1051,7 +1072,9 @@ cmd_link() { for (i = 1; i <= n; i++) need[other_arr[i]] = 1 } /^links:/ { - # Parse existing links + # Parse existing links — narrow class (no quote/space) on purpose: + # cmd_link only reads links it itself wrote at line 1081 (bare-CSV). + # External writes are blocked at validate_field_update (links case). gsub(/[\[\]]/, "", $0) sub(/^links: */, "", $0) m = split($0, existing, ", *") @@ -1153,8 +1176,8 @@ cmd_show() { /^---$/ { in_front = !in_front; next } in_front && /^id:/ { id = $2 } in_front && /^status:/ { status = $2 } - in_front && /^deps:/ { deps = $2; gsub(/[\[\] ]/, "", deps) } - in_front && /^links:/ { links = $2; gsub(/[\[\] ]/, "", links) } + in_front && /^deps:/ { deps = $2; gsub(/[\[\] "]/, "", deps) } + in_front && /^links:/ { links = $2; gsub(/[\[\] "]/, "", links) } in_front && /^parent:/ { parent = $2 } !in_front && /^# / && title == "" { title = substr($0, 3) } @@ -1438,48 +1461,31 @@ cmd_update() { # Handle JSON array syntax -> YAML flow array if [[ "$value" =~ ^\[.*\]$ ]]; then - if command -v jq &>/dev/null; then - # jq exits non-zero on invalid JSON. It also exits - # non-zero via error() if any array item contains a - # comma — comma-in-item would corrupt round-trip - # through consumers that split on ", *" (ticket-query, - # ticket-ls, internal awk splits). - local yaml_val - yaml_val=$(echo "$value" | jq -er ' - if type == "array" then - if any(.[] | tostring; test(",")) then - error("array item contains comma") - else - "[" + (map(tostring) | join(", ")) + "]" - end - else - . - end - ' 2>&1) || { - if echo "$yaml_val" | grep -q "array item contains comma"; then - echo "Error: array items for '$field' must not contain commas. The ticket ecosystem stores arrays as comma-separated values and has no quote-aware parsing in consumers (ticket-query, ticket-ls). Use dashes or underscores instead." >&2 - else - echo "Error: invalid JSON for $field" >&2 - fi - return 1 - } - value="$yaml_val" - else - # This branch is exercised only when jq is absent; - # manual verification required on a jq-less system. - # Still guard the contract: reject items that contain - # commas, since downstream consumers split on commas. - local inner="${value#[}" - inner="${inner%]}" - # Require 1+ non-quote chars on each side of the comma: - # that anchors the comma INSIDE a quoted string and - # excludes the separator case '","' between items. - if echo "$inner" | grep -qE '"[^"]+,[^"]+"'; then - echo "Error: array items for '$field' must not contain commas (ecosystem limitation)" >&2 - return 1 - fi - # Value passes through as-is (user's responsibility beyond comma check) + # Fail-fast: jq is required for JSON-array values. Without + # jq, we can't validate the input or emit safe YAML, and a + # silent pass-through risks storing malformed values that + # downstream readers misinterpret. jq is universally + # available on supported platforms. + if ! command -v jq &>/dev/null; then + echo "Error: jq is required for JSON-array values (field '$field')" >&2 + return 1 fi + # jq exits non-zero on invalid JSON. @json emits + # JSON-encoded strings — values with commas, quotes, + # or backslashes roundtrip losslessly as valid YAML 1.2 + # flow sequence (which is also a valid JSON array). + local yaml_val + yaml_val=$(printf '%s\n' "$value" | jq -er ' + if type == "array" then + "[" + (map(@json) | join(", ")) + "]" + else + . + end + ' 2>&1) || { + echo "Error: invalid JSON for $field" >&2 + return 1 + } + value="$yaml_val" fi update_yaml_field "$file" "$field" "$value"