Settings UI to map Crow ticket states ↔ Jira workflow statuses (#523)#525
Conversation
Jira workflow status names are configurable per project, so Crow's
hardcoded Crow→Jira mapping (.ready → "To Do", everything else verbatim)
made transitions silently fail for projects that renamed a status. Add a
per-workspace `jiraStatusMap` (Backlog/Ready/In Progress/In Review/Done →
concrete Jira status names) persisted in config.json, edited in
Settings → Workspaces, consulted by both status surfaces with a fallback
to the built-in defaults when unset.
- Model: `WorkspaceInfo.jiraStatusMap` + `JiraConfig.statusMap`; the
default table moves to `TicketStatus.defaultJiraStatusName` (CrowCore)
as the single source of truth shared by the UI and the backend.
- In-app acli path: `JiraTaskBackend.setTaskStatus` consults the map;
`IssueTracker.markInReview` resolves the matching workspace's map by
the ticket's project key / site.
- Agent MCP path: `ClaudeLauncher` embeds a "Jira Status Mapping" block
in Jira-session prompts, and the `/crow-workspace` skill instructs the
agent to consult `jiraStatusMap` before `transitionJiraIssue`.
- Settings UI: a per-status field (placeholder = current default) plus a
bonus "Fetch from Jira" button that populates dropdowns from the live
workflow via `GET /rest/api/3/project/{key}/statuses` (new
`JiraStatusFetcher`, reusing #524's Atlassian credentials); degrades to
free-text when no credential/site is configured.
- Tests (CrowCore/CrowProvider/CrowClaude) + docs/configuration.md.
🐦⬛ Generated with Claude Code, orchestrated by Crow
Co-Authored-By: Claude <noreply@anthropic.com>
Crow-Session: ACF93A34-A697-4DEF-B282-2734892C8AD2
dgershman
left a comment
There was a problem hiding this comment.
Code & Security Review
Build: not run locally (swift build blocked on missing Frameworks/GhosttyKit.xcframework, env issue unrelated to this PR). Tests run: JiraStatusFetcherTests (8), JiraTaskBackendTests (20), ClaudeLauncherTests (18) all green. CI's Build & Test was still in progress at review time.
Critical Issues
None.
Security Review
Strengths:
JiraStatusFetcherkeeps the Basic-auth header out of logs and uses the same resolver as the MCP path, so the secret never lands at rest. Test coverage confirms the header is set and HTTP errors surface as.http(code)instead of leaking response bodies.- Token resolution is delegated to
AtlassianMCPResolver(which already gates onopavailability) — no new secret-handling code paths. - Status-name input is user-controlled but consumed only as an
acli --statusarg / MCP transition arg; no shell composition or SQL.
Concerns (Yellow):
JiraStatusFetcher.statusesURLacceptshttp://origins (Packages/CrowCore/Sources/CrowCore/JiraStatusFetcher.swift:27). ThehasPrefix(\"http\")test treats anything starting withhttpas a full origin, so a workspacejiraSitetyped ashttp://acme.atlassian.netwill send the Atlassian Basic credential in cleartext. Operator-controlled input, but a one-linehasPrefix(\"https://\")guard (or auto-upgrade) closes the cleartext-credential foot-gun for free.
Code Quality
Yellow — should fix:
-
ClaudeLauncher.jiraStatusMappingBlockis dead in production (Packages/CrowClaude/Sources/CrowClaude/ClaudeLauncher.swift:23,88,108). The newjiraStatusMap:parameter onClaudeLauncher.generatePromptis exercised only by unit tests. The protocol-facing wrapperClaudeCodeAgent.generatePrompt(Packages/CrowClaude/Sources/CrowClaude/ClaudeCodeAgent.swift:89-104) does not passjiraStatusMap(norcustomInstructions) through to the launcher, and theCodingAgentprotocol method doesn't include it either. So no production Jira session will see the## Jira Status Mappingblock — only the SKILL-driven path is live. This contradicts the PR description's "ClaudeLauncher embeds a## Jira Status Mappingblock in Jira-session prompts." Either threadjiraStatusMapthroughCodingAgent.generatePrompt+ClaudeCodeAgent(matches the design intent and gives belt-and-suspenders coverage), or drop the launcher-side block in favor of the already-shipping SKILL path (matches today'scustomInstructionsstory). Either way, the current state ships testable code that nothing actually calls. -
Prompt block doesn't restate the fallback rule (
Packages/CrowClaude/Sources/CrowClaude/ClaudeLauncher.swift:107-122). The rendered block lists only mapped states (In Progress → In Development); an agent reading just the prompt — without consulting the SKILL — could read that as "these are the only valid transitions." SKILL.md covers the default-fallback rule, but the prompt block is the inline cue. One sentence (e.g., "Unlisted states use Crow's defaults:Ready→To Do; others use the state name verbatim.") makes the block self-contained. (Moot if Yellow #1 is resolved by removing the launcher block.) -
IssueTracker.jiraStatusMap(forTicket:)host match is loose substring (Sources/Crow/App/IssueTracker.swift:2399-2403).ticketURL.contains(site)matchesacme.atlassian.netagainst a workspace configured fordev.acme.atlassian.net(or vice versa, depending on enumeration order). The project-key path runs first and is exact, so this only bites when two Jira workspaces share an overlapping site fragment and the ticket isn't keyed to either project — but the workspace order is config-define, not URL-aware. Safer to parse the URL host (URL(string:)?.host) and compare host-equality (or anchor with://\\(site)/substring) rather than free substring.
Green — consider
- Stale
fetchedStatusesafter edits (Packages/CrowUI/Sources/CrowUI/WorkspaceFormView.swift:33-35). The dropdown keeps the last-fetched names even after the operator editsjiraSiteorjiraProjectKey, so a stale list can be silently applied to a different project..onChange(of:)on those two bindings to clearfetchedStatuses+fetchStatusesErrortightens the UX. - Dedup the trim/empty dance.
JiraTaskBackend.jiraStatusName,WorkspaceFormView.statusMapForSave, andClaudeLauncher.jiraStatusMappingBlockeach implement the same "trim, drop blanks" rule. A tinyString?.nonBlank(or one normalizer on the model) would dedupe and make the contract single-sourced. - No request timeout on
JiraStatusFetcher. DefaultURLSession.sharedhas a 60s resource timeout; for a click-driven UI affordance, an explicit 10–15sURLRequest.timeoutIntervalwould surface slow responses as a clear error instead of a long spinner. The work is in a detached task so it can't hang the UI, but the button can stay disabled a while.
Summary Table
| Color | Meaning | Verdict effect |
|---|---|---|
| Red | Must fix | Request changes |
| Yellow | Should fix | Request changes |
| Green | Consider | Approve allowed |
Recommendation: Request Changes — driven by [0 Red, 4 Yellow, 3 Green] findings. Yellow #1 is the biggest one: the PR description and the new tests both imply the launcher embeds the mapping in Jira-session prompts, but it doesn't reach production. Worth either wiring it through ClaudeCodeAgent (and the CodingAgent protocol) so the prompt block actually fires, or removing the launcher-side block and leaning entirely on the SKILL path — current state is half-implemented.
…on (#523) Review feedback from #525 (dgershman): - Yellow #1/#2: the `ClaudeLauncher` `## Jira Status Mapping` prompt block was never wired through `CodingAgent`/`ClaudeCodeAgent`, so it was dead in production (same story as `customInstructions`). Drop the launcher block + its `jiraStatusMap` param and lean entirely on the live `/crow-workspace` SKILL path (which reads `jiraStatusMap` from config.json). Removed the now-stale SKILL/docs cross-reference to the prompt block. - Yellow (security): `JiraStatusFetcher.statusesURL` now strips any user-supplied scheme and forces https, so a `jiraSite` typed as `http://…` can't send the Atlassian Basic credential in cleartext. - Yellow #3: `IssueTracker.jiraStatusMap(forTicket:)` matches the workspace site by parsed-host equality instead of a loose substring, so `acme.atlassian.net` no longer matches `dev.acme.atlassian.net`. - Green: clear stale `fetchedStatuses` on jiraSite/projectKey change; add a 15s request timeout to the fetcher; single-source the "trim, drop blanks" rule via `String.nonBlank` (CrowCore). Tests updated (launcher block tests removed; http→https upgrade test added). make app clean; CrowCore/CrowProvider/CrowClaude + root 165 green. 🐦⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude <noreply@anthropic.com> Crow-Session: ACF93A34-A697-4DEF-B282-2734892C8AD2
dgershman
left a comment
There was a problem hiding this comment.
Code & Security Review
Second pass after the f2cd4b0 hardening commit. The earlier Yellow findings (dead ClaudeLauncher block, http→cleartext credential leak, loose substring host matching) are all addressed, and the new behavior is well-tested. No blockers.
Security Review
Strengths
JiraStatusFetcher.statusesURLnow strips any user-supplied scheme and forceshttps://, with a regression test (statusesURLForcesHTTPSOnCleartextOrigin). Auth header is set onURLRequest(not embedded in URL) and constrained to a Basic value resolved from the already-vettedAtlassianMCPResolver.IssueTracker.jiraStatusMap(forTicket:)now compares parsed hosts viacaseInsensitiveCompare, soacme.atlassian.netno longer accidentally matchesdev.acme.atlassian.net.- Click-driven
JiraStatusFetcher.fetchStatusNamespins a 15stimeoutIntervaland tunnels failures through a typedFetchError, so a slow workflow doesn't leave the button spinning indefinitely. - The off-main-actor
Task.detachedinSettingsView.fetchJiraStatusesis the right call —AtlassianMCPResolver.resolvemay shell out toop read.
Concerns
- None blocking. Consider items below.
Code Quality
- Tests are thorough — URL build/encoding/HTTPS upgrade, parse/dedupe/empty/malformed, basic-auth header, HTTP error surface,
JiraConfig.statusMapoverride + blank-fallback +setTaskStatusargv assertion, and JSON round-trip + missing-key back-compat.swift test --filter JiraStatusFetcherTestsis green locally. - Default-table single-source-of-truth via
TicketStatus.defaultJiraStatusName(CrowCore) is nice — the Settings placeholders, theJiraTaskBackendfallback, and the docs all converge on one definition. String.nonBlankextension consolidates the "treat blank as unset" rule, which previously would have been re-implemented instatusMapForSave,jiraStatusName(for:), etc.WorkspaceFormViewclearsfetchedStatusesonjiraSite/jiraProjectKeychange so a stale list can't be applied to a different project — good.WorkspaceFormView.fetchStatusesis optional withnildefault, so the existing Setup Wizard call site stays source-compatible.
Considerations (Green)
JiraStatusFetcher.statusesURL— userinfo edge case. A site typed asacme.atlassian.net@evil.comwould buildhttps://acme.atlassian.net@evil.com/…, whose parsedhostisevil.comand userinfo isacme.atlassian.net. Hitting "Fetch from Jira" would then POST the Basic credential to the attacker host. Threat model is admin self-config (the user types their own site value), but the source comment already calls out credential-safety intent — abareHost.contains("@") || bareHost.contains("/")reject would close the door cheaply and match that intent. (Packages/CrowCore/Sources/CrowCore/JiraStatusFetcher.swift:27)- Project key encoding.
addingPercentEncoding(withAllowedCharacters: .urlPathAllowed)leaves/unencoded, so a stray project key with path separators could traverse to a different REST endpoint. Same admin-self-config caveat. Tightening to.alphanumericsplus-/_would be safer-by-default. (JiraStatusFetcher.swift:32) - Multi-Jira fallback heuristic.
IssueTracker.jiraStatusMap(forTicket:)final clause returns the lone map when exactly one Jira workspace has ajiraStatusMap— even if other Jira workspaces exist without one and the ticket is from one of them. With multiple Jira sites this could apply the wrong project's renames. Consider returningnilwhenever the ticket doesn't match by project-key or host and multiple Jira workspaces are configured (regardless of which have maps). (Sources/Crow/App/IssueTracker.swift:2410) - Stale PR description. Description still says
ClaudeLauncherembeds a## Jira Status Mappingprompt block, but f2cd4b0 dropped that block in favor of the live/crow-workspaceSKILL path. Worth updating so future readers don't hunt for code that isn't there.
Summary Table
| Color | Meaning | Verdict effect |
|---|---|---|
| Red | Must fix | Request changes |
| Yellow | Should fix | Request changes |
| Green | Consider | Approve allowed |
Recommendation: Approve — driven by 0 Red, 0 Yellow, 4 Green findings.
Closes #523. Follow-up to #522/#524 (the Atlassian MCP migration), rebased onto it.
Problem
Crow's Crow→Jira status mapping was hardcoded in
JiraTaskBackend.jiraStatusName(for:)(.ready→ "To Do"; everything else used theTicketStatusraw value). Jira workflow status names are configurable per project, so a project that renames a status (e.g. "In Development" instead of "In Progress") made Crow's transitions silently fail.What
A per-workspace
jiraStatusMap(Backlog / Ready / In Progress / In Review / Done → concrete Jira status names) persisted inconfig.json, edited in Settings → Workspaces → Jira Status Mapping, consulted by both status surfaces with a fallback to today's hardcoded defaults when unset.How
WorkspaceInfo.jiraStatusMap+JiraConfig.statusMap. The default table moves toTicketStatus.defaultJiraStatusName(CrowCore) as one source of truth shared by the UI (placeholders) and the backend (fallback).aclipath —JiraTaskBackend.setTaskStatusconsults the map;IssueTracker.markInReviewresolves the matching workspace's map from the ticket's project key / site (acli is single-site, so only the map needs threading).ClaudeLauncherembeds a## Jira Status Mappingblock in Jira-session prompts, and the/crow-workspaceskill (+ synced template) tells the agent to consultjiraStatusMapfromconfig.jsonbeforetransitionJiraIssue.GET /rest/api/3/project/{key}/statuses(newJiraStatusFetcher, reusing Replace acli with the official Atlassian Remote MCP Server for Jira (#522) #524's Atlassian email+token Basic auth). Degrades to free-text when no credential / site / project key is set. (aclihas no list-transitions command and the in-app UI can't reach the agent MCP, so REST is the only viable in-app fetch.)docs/configuration.mdJira section documentsjiraStatusMapshape, fallback behavior, and the fetch affordance.Tests
JiraTaskBackendTests— default table, map override, blank-entry fallback,setTaskStatususes the mapped--status.AppConfigTests—jiraStatusMapround-trip + nil-when-absent (back-compat).ClaudeLauncherTests— mapping block present with a map, absent without one, never leaks on non-Jira sessions.JiraStatusFetcherTests— URL build, parse/dedupe, Basic-auth header, HTTP-error surface.make appclean.🤖 Generated with Claude Code