Skip to content

fix(cookiy.sh): die on flags missing required values; tighten guide update - #44

Open
yupeng-dev wants to merge 1 commit into
mainfrom
fix/cookiy-sh-strict-flag-validation
Open

fix(cookiy.sh): die on flags missing required values; tighten guide update#44
yupeng-dev wants to merge 1 commit into
mainfrom
fix/cookiy-sh-strict-flag-validation

Conversation

@yupeng-dev

Copy link
Copy Markdown
Contributor

Problem

build_json silently substituted the literal string "true" for any --flag whose next token began with -- (or was end-of-args). For required string fields like --base-revision and --idempotency-key, this produced JSON like:

{"base_revision":"true","idempotency_key":"true", ...}

— a 4-character placeholder that shipped to the server and was rejected by zod .min(8) as invalid_discussion_guide_patch_body. Users saw a confusing 400 round-trip instead of a clear local error.

Reproduction:

# $UNSET expands to nothing, so build_json sees: --base-revision --idempotency-key K...
bash cookiy.sh study guide update \
  --study-id S \
  --base-revision $UNSET \
  --idempotency-key K12345678 \
  --json '{"research_overview":{"sample_size":20}}' \
  --change-message "bump"

Before:

{"ok":false,"status_code":400,"error":{"code":"BAD_REQUEST","message":"invalid_discussion_guide_patch_body","details":{"issues":[{"path":"base_revision","message":"String must contain at least 8 character(s)"}, ...]}}}

After:

--base-revision requires a value (got next flag or end of args)

Changes

  1. build_json — keep the implicit "true" default only for an explicit boolean-flag whitelist (include_raw, skip_synthetic_interview, include_incomplete). Any other flag without a value triggers a clear local die.
  2. New require_min_string_length helper — mirrors server-side zod .min(N) on string fields, producing a precise local error before any HTTP.
  3. study guide update — uses require_min_string_length 8 for base_revision and idempotency_key, matching the server contract.

Test plan

  • bash -n cookiy.sh — syntax OK
  • Original bug: --base-revision $UNSET --idempotency-key K... --json ... → dies with clear message
  • Empty value: --base-revision "" → dies with <string of length >= 8>
  • Short value: --base-revision short → dies client-side instead of round-tripping
  • Valid 8+ chars: passes client validation and reaches the server normally
  • Boolean flag: --include-incomplete (in commands that accept it) still defaults to true

No backend changes required; behavior change is strictly safer (silent corruption → loud, local error).

Made with Cursor

…pdate

build_json silently substituted the literal string "true" for any --flag
whose next token began with "--" (or was end-of-args). For required
string fields like --base-revision and --idempotency-key, this produced
JSON like {"base_revision":"true","idempotency_key":"true"} — a 4-char
placeholder that shipped to the server and was rejected by zod
.min(8) as invalid_discussion_guide_patch_body.

Changes:
- build_json now keeps the implicit "true" default only for an explicit
  whitelist of boolean flags (include_raw, skip_synthetic_interview,
  include_incomplete). Any other flag without a value triggers a clear
  local die: "--foo requires a value (got next flag or end of args)".
- New require_min_string_length helper mirrors server-side zod min(N)
  on string fields, producing a precise local error before any HTTP.
- "study guide update" uses require_min_string_length 8 for
  base_revision and idempotency_key, matching the server contract.

Repro of original bug:
  bash cookiy.sh study guide update --study-id S \
    --base-revision $UNSET --idempotency-key K12345678 --json '{"x":1}'
  Before: 400 invalid_discussion_guide_patch_body from server.
  After:  --base-revision requires a value (got next flag or end of args)

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants