Skip to content

fix(source-control): rule 3 no longer lists bare map(f) as element-wise-safe - #2285

Merged
kyle-sexton merged 4 commits into
mainfrom
cursor/fix-readiness-map-carveout-63e1
Aug 12, 2026
Merged

fix(source-control): rule 3 no longer lists bare map(f) as element-wise-safe#2285
kyle-sexton merged 4 commits into
mainfrom
cursor/fix-readiness-map-carveout-63e1

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Fixes #2245. readiness.md rule 3's carve-out incorrectly listed map over .[] as element-wise-safe. map(f) is defined as [.[] | f] — it builds an array per page under --paginate, so it is a per-page fold unless followed by | .[].

The carve-out now names select and .[] | f as safe, and calls out that bare map(f) is not — use map(f) | .[] or .[] | f instead.

Test plan

  • Read the updated rule 3 prose for accuracy against the gh --paginate + --jq behavior described in the issue
  • Version bump source-control 0.51.12 → 0.51.13 with CHANGELOG entry

Related

…se-safe

map(f) builds a per-page array under --paginate; only map(f) | .[] or
.[] | f are element-wise-safe. Fixes the false carve-out in readiness.md
rule 3 reported in #2245.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor

cursor Bot commented Aug 12, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 29s —— View job


Security review complete

  • Gather PR diff (docs-only change to readiness.md, plugin.json, CHANGELOG.md)
  • Review for security issues (injection, authz, secrets, unsafe workflow patterns)
  • Post findings

No security issues found.

This PR is a documentation-only change:

  • readiness.md — corrects prose in rule 3's carve-out, clarifying that bare map(f) (which is [.[] | f]) folds per page under gh --paginate --jq and is not element-wise-safe, while select and .[] | f are.
  • plugin.json — version bump only.
  • CHANGELOG.md — changelog entry describing the fix.

No code, scripts, GitHub Actions workflows, permissions, dependency pins, or secret/token handling are touched. There is no injection surface, authorization boundary, or trust-boundary logic introduced or altered by this change. The content itself is advisory prose about jq/gh --paginate semantics — it does not affect how any tooling in this repo executes.

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 51s —— View job


Code review: PR #2285

  • Gather context (diff, CLAUDE.md, REVIEW.md)
  • Review changed files for correctness
  • Check alignment with plugin doc conventions
  • Post findings

Summary

Doc-only fix to readiness.md rule 3, plus the matching version bump and CHANGELOG entry. I verified the core technical claim directly with jq:

$ echo '[{"a":1},{"a":2}]' | jq -c 'map(.a)'
[1,2]

Piped through per-page (as gh --paginate --jq does), map(f) really does emit one array per page rather than a flattened element stream, so the PR's correction is accurate: map(f) alone is a per-page fold, and map(f) | .[] / .[] | f are the safe forms.

Checked for other stale copies of the old "map over .[]" claim elsewhere in the plugin — found none; readiness.md line 59 is the only place this text lives, and the CHANGELOG entry describes the change accurately.

One finding, left as an inline comment on the changed line: the same sentence lists bare select as element-wise-safe, but a bare select(f) piped a page (an array) directly errors in jq rather than filtering elements — only .[] | select(f) actually works. Low severity (it fails loudly rather than silently returning a wrong answer, unlike the map(f) case this PR fixes), but since this exact sentence is being edited for accuracy right now, it's worth tightening in the same pass.

CHANGELOG/version bump (0.51.12 → 0.51.13) follows the plugin's existing Keep a Changelog format and conventions.

Test plan items (reading the rule 3 prose, version bump) are the author's own claims — I did not re-verify those as "confirmed by me" beyond what's stated above.

Comment thread plugins/source-control/skills/pull-request/reference/readiness.md Outdated
@github-actions

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

cursoragent and others added 2 commits August 12, 2026 02:25
Bare select(f) on a paginated page errors in jq; only .[] | select(f) is
element-wise-safe. Bump 0.51.13 → 0.51.14 per review feedback.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging.

Re-run the job to retry the review; a new push also retries it only if the caller's pull_request triggers include synchronize (the canonical security caller keeps it). An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator.

Re-running does NOT help for every class:

  • rate-limit that persists across re-runs, or auth — the credential or usage budget needs an operator; retrying will not clear it.
  • a run that exhausted its turn budget ("subtype":"error_max_turns" above) will exhaust it again. As the PR author, split the change into smaller PRs; raising --max-turns is a change to the caller workflow, not something you can set on this PR.

…map-carveout-63e1

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@kyle-sexton
kyle-sexton merged commit bde011a into main Aug 12, 2026
35 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/fix-readiness-map-carveout-63e1 branch August 12, 2026 02:54
@github-actions

Copy link
Copy Markdown

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-run the job to retry the review. A new push re-triggers this lane only if the caller's pull_request triggers include synchronize (the canonical caller omits it).
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).

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.

source-control: readiness.md rule 3's carve-out lists map as element-wise, but map(f) is a per-page fold

2 participants