Skip to content

feat(tools/jira): add write subcommands (comment, transition, label, assign, field, attach)#345

Merged
potiuk merged 4 commits into
apache:mainfrom
oscerd:fix/301
May 28, 2026
Merged

feat(tools/jira): add write subcommands (comment, transition, label, assign, field, attach)#345
potiuk merged 4 commits into
apache:mainfrom
oscerd:fix/301

Conversation

@oscerd

@oscerd oscerd commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Extends the read-only JIRA bridge with six write subcommands: comment, transition, label, assign, field, and attach
  • Write operations require JIRA_API_TOKEN and follow the same write-path discipline as the GitHub bridge (mutations gated on explicit user confirmation in the calling skill)
  • Adds a pytest test harness with a mock HTTP server (24 tests, auto-skipped when groovy is not on PATH)

Test plan

  • Tests collected and pass locally (uv run pytest — 24 tests, skipped when groovy not on PATH)
  • Placeholder check passes (tools/dev/check-placeholders.sh — OK)
  • CI pre-commit hooks pass
  • Manual verification with a live JIRA instance (optional, requires groovy + API token)

Closes #301

@oscerd oscerd force-pushed the fix/301 branch 2 times, most recently from a4fae3d to 78b3eec Compare May 28, 2026 08:41

@andreahlert andreahlert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope is tight and the mock harness is a solid first layer — really like where this is going. Requesting changes on three things that imho will bite the first time a skill calls this against issues.apache.org/jira:

  1. cmd_field only sends string values — most non-trivial JIRA fields (priority, version, user, single-select) need objects/arrays and will 400. See inline.
  2. README auth description is Cloud-flavored but cmd_assign uses DC {name}. Needs to either say "DC only" or actually handle Cloud.
  3. ASF JIRA PATs use Authorization: Bearer, not Basic. Today PAT users have to fake Basic via base64(user:pat).

The rest below are nits / follow-ups — flagging them while I'm here, not blocking on them.

Cross-cutting:

  • No retry/backoff anywhere. ASF JIRA does rate-limit; a small 429/5xx → 3 retries w/ backoff loop would save pain in long skill flows.
  • Pre-existing, but since the README is being touched here: @Grab('org.apache.groovy:groovy-json:4.0.21') requires Groovy 4 (3.x uses org.codehaus.groovy), yet README.md:66 still says "Groovy 3.x or newer". Easy fix in passing.

Comment thread tools/jira/bridge.groovy Outdated
Comment thread tools/jira/README.md Outdated
Comment thread tools/jira/bridge.groovy Outdated
Comment thread tools/jira/bridge.groovy Outdated
Comment thread tools/jira/bridge.groovy
Comment thread tools/jira/bridge.groovy
Comment thread tools/jira/bridge.groovy Outdated
Comment thread tools/jira/tests/test_bridge_write.py
@oscerd

oscerd commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

@andreahlert Thanks for the thorough review — addressed all three blocking items and most nits in 2dcfadc.

Retry/backoff: Agreed this will matter in long skill flows. It applies equally to the read path, so imho better as a cross-cutting follow-up than scoped to this PR.

Groovy version: Fixed — README now says "Groovy 4.x" and explains the org.apache.groovy group ID.

@andreahlert andreahlert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this. LGTM.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the iteration — implementation is clean and the prior-feedback
round (Bearer scheme, atomic update API, UTF-8 file reads) shows good
attention to detail. Strict key regex (^[A-Z][A-Z0-9_]*-\d+$) closes the
URL-injection path, and the 28 pytest tests cover the right happy + error +
auth surfaces.

Blocking — wire the test suite into CI

The new tools/jira/ is a proper uv project with tests, but neither the
workflow matrix nor the prek hooks pick it up — so the 28 tests land as
dead weight in CI and a future regression would never block a PR. Two small
additions, both mechanical, mirror the pattern the other tools use:

1. Add jira to the pytest matrix in .github/workflows/tests.yml:

        project:
          - name: oauth-draft
            path: tools/gmail/oauth-draft
          - name: generate-cve-json
            path: tools/vulnogram/generate-cve-json
          - name: skill-and-tool-validator
            path: tools/skill-and-tool-validator
          - name: privacy-llm-checker
            path: tools/privacy-llm/checker
          - name: privacy-llm-redactor
            path: tools/privacy-llm/redactor
          - name: vulnogram-oauth-api
            path: tools/vulnogram/oauth-api
          - name: sandbox-lint
            path: tools/sandbox-lint
          - name: agent-isolation
            path: tools/agent-isolation
          - name: jira
            path: tools/jira

2. Add prek hooks for jira in .pre-commit-config.yaml, mirroring the
sibling pattern (the skill-and-tool-validator block is the closest analogue
— four hooks: ruff-check, ruff-format, mypy, pytest). Each hook's
files: regex should match ^tools/jira/(src|tests|pyproject\.toml|bridge\.groovy).

Once both land, pytest (jira) shows up as a matrix entry under the
tests-ok umbrella and prek runs the lint pass locally and in CI.

Happy to apply this as a maintainer-side fixup if you'd rather not chase it
— just say the word. Otherwise, push the two edits when you have a moment
and I'll re-review.


This review was drafted by an AI-assisted tool and confirmed by an Apache Steward
maintainer. After you've addressed the points above and pushed an update, an
Apache Steward maintainer will take the next look at your PR.

More on how Apache Steward handles maintainer review:
CONTRIBUTING.md.

@oscerd

oscerd commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

@potiuk Done in 5aa58e6:

  1. Added jira to the pytest matrix in .github/workflows/tests.yml
  2. Added four prek hooks (ruff-check, ruff-format, mypy, pytest) in .pre-commit-config.yamlfiles: pattern includes bridge.groovy so tests re-run when the bridge itself changes
  3. Added ruff and mypy to dev dependencies in pyproject.toml with mypy config mirroring sandbox-lint

All four checks pass locally (ruff check, ruff format, mypy, pytest collect — 28 tests).

oscerd added 3 commits May 28, 2026 22:49
…assign, field, attach)

Extends the read-only JIRA bridge with six write subcommands that
mirror the existing read API shape. Write operations require
JIRA_API_TOKEN and follow the same write-path discipline as the
GitHub bridge: every mutation is gated on explicit user confirmation
in the calling skill.

Adds a pytest test harness with a mock HTTP server (24 tests,
auto-skipped when groovy is not on PATH).

Closes apache#301

Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
- Add JIRA_AUTH_SCHEME env var (Basic/Bearer) for ASF JIRA DC PATs
- Add --value-json flag to field subcommand for structured values
- Switch label command to atomic JIRA update API (no read-modify-write race)
- Use explicit UTF-8 encoding for comment body file reads
- Clarify README: DC-only scope, Groovy 4.x requirement, auth docs
- Add DC-only comment on assign payload
- Add auth header value and Bearer scheme assertions in tests (28 total)

Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
Add jira to the pytest matrix in .github/workflows/tests.yml and
add prek hooks (ruff check, ruff format, mypy, pytest) in
.pre-commit-config.yaml, mirroring the pattern used by the other
tool packages. Adds ruff and mypy to dev dependencies.

Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
Re-resolve `tools/jira/uv.lock` so the locked `ruff` version matches
what CI's `prek` job picks (the framework pins to the most recent
release that is at least 7 days old; the committed lock had a newer
version that hadn't yet aged in).

Generated-by: Claude Code (Opus 4.7)

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the CI wire-in addresses the prior REQUEST_CHANGES exactly:

  • .github/workflows/tests.ymljira / tools/jira matrix entry slots in
    alongside the other tool packages; the tests-ok umbrella now gates the
    jira suite alongside the rest.
  • .pre-commit-config.yaml — four hooks under a repo: local block
    (ruff check, ruff format --check, mypy, pytest), each with a
    files: regex matching ^tools/jira/(src|tests|pyproject\.toml|bridge\.groovy)
    exactly as proposed.
  • pyproject.tomlruff and mypy added to the dev dep group; uv.lock
    reflects.

Verified locally on the rebased branch: ruff check clean, ruff format --check clean, mypy reports no issues, pytest skips the Groovy-dependent
cases (groovy not installed locally) but the test suite's skip semantics
are correct and CI ships groovy. The 28 test assertions still match the
prior round.

The substantive review from earlier still stands: clean Groovy
implementation of six write subcommands; strict key regex (^[A-Z][A-Z0-9_]*-\d+$)
closes URL injection; atomic JIRA update API for labels; DC-only scope
called out; Bearer auth supported.

Thanks for the quick turnaround on the CI wiring.


This review was drafted by an AI-assisted tool and confirmed by an Apache Steward
maintainer. The maintainer approving this PR has read the findings and signed off.
If something feels off, please reply on the PR and a maintainer will follow up.

More on how Apache Steward handles maintainer review:
CONTRIBUTING.md.

@potiuk potiuk merged commit 55568bf into apache:main May 28, 2026
17 checks passed
potiuk added a commit that referenced this pull request May 30, 2026
…erns from session manual cleanups (#402)

Per direct observations from the airflow-s 2026-05-29/30 bulk sync —
two recurring title-noise patterns were cleaned manually that the
existing cascade did not catch:

1. Trailing prior-CVE-relationship parentheticals — the cross-CVE
   relationship is structurally captured by the Gate #3 cross-CVE
   clause in the public summary; embedding the relationship in the
   title is noise to downstream advisory consumers. Catches every
   shape observed in this session:
   - `(CVE-YYYY-NNNNN)`
   - `(possible CVE-YYYY-NNNNN variant)` — from #345
   - `(incomplete fix for CVE-YYYY-NNNNN)` — from #351
   - `(fix-bypass of CVE-YYYY-NNNNN)` — from #352
   - and any other `(... CVE-YYYY-NNNNN ...)` shape

2. Trailing reporter-name attribution parentheticals — reporter
   attribution lives in the credits field, never in the public
   title. Pattern matches `(<name> follow-up)` where `<name>`
   matches name-like tokens (word chars, dots, hyphens, single
   inline spaces) to avoid over-stripping substantive technical
   content. Catches:
   - `(Evan Ricafort follow-up)` — from #346

Substantive technical parentheticals stay intact — e.g. the operator-
name list `(GCSToSFTPOperator + GCSTimeSpanFileTransformOperator)` on
the GCS path-traversal tracker is NOT stripped (it lacks a CVE ID
and doesn't end in `follow-up`).

The matching Step 1d signal row in security-issue-sync now enumerates
the two new patterns so the proposal-time detector and the pre-push
Gate #4 stay in lock-step with the cascade.

Validated against 9 cases: 4 session-derived fixes (all pass), 3
synthetic CVE-relationship variants (all pass), 1 substantive
technical parenthetical (preserved correctly), 1 "<word> follow-up"
edge case (stripped as designed — narrow scope acceptable since
"follow-up" titles in airflow-s are exclusively reporter-attribution).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

feat(tools/jira): add write subcommands (comment, transition, label, assign, field, attach)

3 participants