LCORE-1632 Feature design process: improvements#1445
Conversation
…cept JIRA numbers, fix templates 404
…iras skill credentials
…x and spec-doc spike search
…e-parse when files exist
…ent on update, fix naming and UX
…stitution after filing
…d spike doc substitution, better spike key detection
…ike doc substitution step
WalkthroughAdds JIRA tooling and docs: new fetch and shared-credentials scripts; refactors file-jiras.sh to flag-based CLI, metadata-aware Epic-first filing with update-vs-create behavior and linking; updates Claude commands, templates, and contributing docs to use the new workflows and metadata. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User/Agent
participant FileTool as dev-tools/file-jiras.sh
participant FetchTool as dev-tools/fetch-jira.sh
participant Common as dev-tools/jira-common.sh
participant JIRA as JIRA API
participant FS as Filesystem
rect rgba(200,230,255,0.5)
User->>FileTool: run --spike-doc /path --feature-ticket LCORE-1234
FileTool->>Common: ensure_jira_credentials()
Common-->>FileTool: JIRA_EMAIL, JIRA_TOKEN, JIRA_INSTANCE
FileTool->>FS: read spike doc (detect existing jiras/*.md)
FileTool->>FetchTool: optionally fetch feature/spike tickets (if key provided)
FetchTool->>Common: ensure_jira_credentials()
FetchTool->>JIRA: GET /rest/api/3/issue/LCORE-1234
JIRA-->>FetchTool: issue JSON (incl. description, issuelinks, subtasks)
FetchTool-->>FileTool: rendered issue text + children/links
FileTool->>FS: generate/modify `00-epic.md` and `*.md` with metadata
FileTool->>JIRA: POST/PUT to create/update issues, link spike→epic
JIRA-->>FileTool: created/updated issue responses (keys)
FileTool->>FS: write `.meta.json`, insert `<!-- key: ... -->` in files
FileTool-->>User: summary of filed vs existing tickets
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/file-jiras.md:
- Around line 12-15: Update the documentation to explicitly state that
dev-tools/jira-common.sh will create ~/.config/jira/credentials.json with
placeholder FIXMEs and then exit immediately, requiring the user to edit the
file and re-run the script; reference the script name (dev-tools/jira-common.sh)
and the credential file path (~/.config/jira/credentials.json) so users know
where to fill in API token details and that creation alone does not continue the
setup.
- Around line 22-27: The docs refer to an outdated output directory
"/tmp/jiras/" causing verification to check the wrong files; update the guidance
in the Step 1/Step 2 instructions to either (a) reflect the current default
output location "docs/design/<feature>/jiras/" or (b) instruct the user to run
the command with the flag "--output-dir /tmp/jiras" if they want the old
behavior; modify the text that mentions "/tmp/jiras/" and the Step 1 example so
it consistently matches the actual command usage and default output, and ensure
the metadata verification steps still point at the resolved output directory.
In @.claude/commands/spike.md:
- Around line 10-11: Update the parsing instruction text in
.claude/commands/spike.md to explicitly name and require the exact output
section headers "Child issues:" and "Linked issues:" so the agent
deterministically extracts those blocks; change any vague references to
"child/linked issues" to the exact labels and add a short directive to only read
content under those headers (ignore similarly named lines elsewhere) before
deciding which linked tickets to fetch for follow-up.
In `@dev-tools/fetch-jira.sh`:
- Around line 45-49: The curl call that populates the local variable data
currently redirects stderr to /dev/null, hiding connection/SSL/timeouts; remove
the trailing "2>/dev/null" (or capture stderr to a separate variable if you
prefer) in the curl invocation that uses $JIRA_EMAIL, $JIRA_TOKEN and
$JIRA_INSTANCE so connection errors are visible when the script fails JSON
parsing of data; keep the existing JSON presence/error handling logic so the
script still reports failures downstream.
- Around line 162-178: The CHILD_KEYS pipeline currently hides errors because
the curl call redirects stderr to /dev/null and the Python block swallows
exceptions; update the pipeline around CHILD_KEYS so curl does not discard
stderr (remove the trailing 2>/dev/null) and change the python3 block in the
heredoc to surface failures (replace the bare except/ pass with printing a
meaningful error to stderr and non-zero exit, or re-raise the exception) and/or
check curl's exit status before piping into python3 so authentication/JQL/API
errors are visible; refer to the CHILD_KEYS assignment, the curl invocation, and
the python3 block to locate and change these behaviors.
In `@dev-tools/file-jiras.sh`:
- Around line 105-107: The printf call uses the variable JIRA_DIR directly as
the format string, which is vulnerable to format-string specifiers; change the
printf invocation in file-jiras.sh to use a constant format string with a %s
placeholder and pass "$JIRA_DIR" as an argument (i.e., replace the current
printf line that references JIRA_DIR with one like printf "Existing ticket files
found in %s/. Re-parse (existing ticket files will be overwritten)? (y/n): "
"$JIRA_DIR"), ensuring you preserve the trailing text and redirect to stderr as
before and keep the surrounding conditional that checks JIRA_DIR and the
subsequent read -r reparse.
- Around line 46-54: The argument parsing loop currently does blind shift 2 for
options (--spike-doc, --feature-ticket, --output-dir) which crashes if a value
is missing; update the while/case block to validate that $# -ge 2 before
performing shift 2 for each of those branches (e.g., in the handlers for
--spike-doc, --feature-ticket, --output-dir) and if not, print an error
mentioning the missing value, call show_help and exit 1; keep usage of
SPIKE_DOC, FEATURE_TICKET and JIRA_DIR assignment but only assign when the value
exists.
- Around line 563-574: The code unconditionally overwrites EPIC_KEY with the
output of file_single_ticket even if filing fails and ignores an existing key
from get_key; change this to preserve the pre-existing epic key and only update
EPIC_KEY when file_single_ticket succeeds: call file_single_ticket and capture
its stdout into a temporary variable (e.g., new_epic), check the command exit
status and that new_epic is non-empty before assigning EPIC_KEY=new_epic,
leaving EPIC_KEY as epic_existing when filing failed; keep the subsequent
conditional that calls link_spike_to_epic only when both EPIC_KEY and
SPIKE_TICKET_KEY are set.
- Around line 328-334: The current curl call that posts to
"$JIRA_INSTANCE/rest/api/3/issueLink" hides the response by redirecting output
to /dev/null, so when linking fails you only see a generic message; change the
logic around the curl invocation used with variables JIRA_EMAIL, JIRA_TOKEN,
JIRA_INSTANCE and payload link_payload to capture curl's output and HTTP status
(e.g., save response body and HTTP code to variables), check curl's exit status
and HTTP status, and include the response body or HTTP code in the error message
that currently references SPIKE_TICKET_KEY and EPIC_KEY so the warning prints
the underlying error (auth, invalid link type, etc.) instead of silently
discarding it.
- Around line 264-265: The get_file_by_number function risks an off-by-one or
invalid sed expression when $1 is empty or non-numeric; add input validation at
the start of get_file_by_number to ensure the positional parameter is a
non-empty integer (e.g., using a regex like '^[0-9]+$'), convert it to a number,
and clamp it to the valid range (0..(count-1)) based on the number of '*.md'
files in $JIRA_DIR before computing the sed index; if validation fails, return
an error/empty result or a default behavior so the sed -n "$((${1} + 1))p" call
never receives an invalid expression.
- Around line 319-335: The current link_payload construction injects shell
variables directly into the python3 -c string (the python3 -c block that sets
link_payload), which risks shell/Python injection; change it to pass
$SPIKE_TICKET_KEY and $EPIC_KEY safely (for example, build the JSON payload in
shell using printf/printf %q or jq, or invoke python with environment
variables/command-line args and use os.environ or sys.argv to populate
json.dumps) so no unescaped values are interpolated into a code string; update
the code that sets link_payload and the python3 invocation accordingly and
ensure curl still reads the resulting JSON string variable link_payload.
- Around line 93-101: The sed -i invocations in set_key() are non-portable and
fail on macOS; update set_key to handle both platforms by either detecting
Darwin via uname and using sed -i '' for macOS and sed -i for others, or replace
both sed -i usages (the substitution branch matching "<!-- key: [A-Z]*-[0-9]*
-->" and the insertion branch "1a\\<!-- key: $key -->") with a cross-platform
alternative such as perl -pi -e so the function works the same on Linux and
macOS.
In `@dev-tools/jira-common.sh`:
- Around line 22-26: The current check using grep -q "FIXME" on $JIRA_CREDS can
produce false positives; update the logic in dev-tools/jira-common.sh to look
for the specific placeholder pattern(s) used in the credentials file (e.g., keys
set to the literal placeholder like username=FIXME or token=FIXME) instead of
any occurrence of the substring "FIXME"; change the grep invocation around the
JIRA_CREDS check to use a stricter regex (or multiple checks) that matches the
placeholder key/value forms only so legitimate values containing "FIXME" do not
fail the check.
- Around line 28-30: The JIRA credential variables JIRA_EMAIL, JIRA_TOKEN, and
JIRA_INSTANCE in jira-common.sh are flagged as unused by shellcheck because
they’re intended for sourced/subprocess use; make them exported so subprocesses
and shellcheck see them as used. Modify the existing assignments for JIRA_EMAIL,
JIRA_TOKEN, and JIRA_INSTANCE (the python3 JSON-extraction lines) to export the
variables (or add explicit export statements immediately after those
assignments) so they become environment variables available to fetch-jira.sh and
file-jiras.sh and satisfy SC2034.
In `@docs/contributing_guide.md`:
- Around line 146-149: Update Step 4 in the contributing guide to show the
required named arguments for the CLI by replacing the bare
`dev-tools/file-jiras.sh` mention with the full invocation including
`--spike-doc` and `--feature-ticket` (e.g., `dev-tools/file-jiras.sh --spike-doc
PATH --feature-ticket TICKET-ID`) and add a brief note that both flags are
required; reference the `dev-tools/file-jiras.sh` script and the `--spike-doc`
and `--feature-ticket` flags so readers know exactly what to pass.
In `@docs/contributing/templates/jira-ticket-template.md`:
- Line 2: The placeholder key in the template uses `<!-- key: LCORE-xxxx -->`,
which doesn't match the regex in dev-tools/file-jiras.sh:get_key()
(`[A-Z]*-[0-9]*`); update the template placeholder to use a digits-based ticket
key like `<!-- key: LCORE-1234 -->` (or a generic `<!-- key: ABC-1234 -->`) so
the script's get_key() regex will detect it and the placeholder-based update
flow won't silently skip the file.
In `@docs/contributing/templates/spike-template.md`:
- Line 44: The placeholder token "<!-- key: LCORE-xxxx -->" is incompatible with
dev-tools/file-jiras.sh:get_key() which expects digits after the dash; update
the template token to use a digit pattern (e.g., "LCORE-0000" or "LCORE-1234")
so it matches the script's regex and will be correctly extracted/updated by
file-jiras.sh; locate the token in spike-template.md and replace the non-digit
"xxxx" with a numeric placeholder that conforms to the get_key() pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5622195e-83e3-4ddc-9edb-3a1547f04cfd
📒 Files selected for processing (11)
.claude/commands/file-jiras.md.claude/commands/spec-doc.md.claude/commands/spike.mddev-tools/fetch-jira.shdev-tools/file-jiras.shdev-tools/jira-common.shdocs/contributing/howto-run-a-spike.mddocs/contributing/templates/index.mddocs/contributing/templates/jira-ticket-template.mddocs/contributing/templates/spike-template.mddocs/contributing_guide.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: E2E: library mode / ci
- GitHub Check: E2E: server mode / ci
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
🪛 GitHub Actions: Shell check
dev-tools/jira-common.sh
[warning] 28-28: Shellcheck SC2034: JIRA_EMAIL appears unused. Verify use (or export if used externally).
[warning] 29-29: Shellcheck SC2034: JIRA_TOKEN appears unused. Verify use (or export if used externally).
[warning] 30-30: Shellcheck SC2034: JIRA_INSTANCE appears unused. Verify use (or export if used externally).
dev-tools/fetch-jira.sh
[warning] 18-18: Shellcheck SC1091: Not following: jira-common.sh was not specified as input.
dev-tools/file-jiras.sh
[warning] 22-22: Shellcheck SC1091: Not following: jira-common.sh was not specified as input.
[warning] 106-106: Shellcheck SC2059: Don't use variables in the printf format string. Use printf '..%s..' "$foo".
🪛 LanguageTool
.claude/commands/spec-doc.md
[style] ~14-~14: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...the primary source for the spec doc. 4. If no spike doc exists, let the user know ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (10)
docs/contributing/templates/index.md (1)
1-8: Looks good.Template index is clear and the links are aligned with the current workflow docs.
.claude/commands/spec-doc.md (1)
8-19: Flow update is coherent and actionable.The spike-first lookup with Jira-fetch fallback is clear and consistent with the new tooling behavior.
docs/contributing/howto-run-a-spike.md (1)
156-169: JIRA filing section is aligned with new workflow.Flags, Epic behavior, and post-filing key replacement guidance are consistent and practical.
dev-tools/jira-common.sh (1)
1-6: LGTM on file structure and credential path setup.The shebang and credential path definition are appropriate. Good practice storing credentials in
~/.config/jira/.dev-tools/fetch-jira.sh (3)
15-18: LGTM on script setup and sourcing.Good use of
set -euo pipefailfor strict error handling. The shellcheck directive for SC1091 is appropriate since the sourced file is relative and can't be statically resolved.
50-56: Good defensive check for valid JSON response.The check using
python3 -cto verify the response contains a validkeyfield before proceeding is a solid approach to detect API errors or invalid tickets.
188-196: LGTM on additional tickets handling.Good use of visual separator and consistent normalization of bare numbers to LCORE- prefix across all arguments.
dev-tools/file-jiras.sh (3)
21-22: Shellcheck SC1091 is expected for dynamic sourcing.The directive on line 21 correctly documents the source relationship. This warning can be safely ignored.
700-709: Good workaround for subshell variable scoping.The comment explains the issue clearly, and re-reading
EPIC_KEYfrom the file after subshell execution is the correct solution for bash's subshell scoping limitations.
410-431: Markdown parsing handles common cases well.The
parse_inlinefunction correctly handles bold, italic, code, and links. Good use of regex splitting to preserve order.
| The output includes child issues — decide which linked tickets to fetch | ||
| for additional context. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make parsing instructions deterministic by naming exact output section headers.
To reduce ambiguity, reference the exact fetch output labels (Child issues: and Linked issues:) so the agent consistently extracts the right blocks before deciding follow-up fetches.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/spike.md around lines 10 - 11, Update the parsing
instruction text in .claude/commands/spike.md to explicitly name and require the
exact output section headers "Child issues:" and "Linked issues:" so the agent
deterministically extracts those blocks; change any vague references to
"child/linked issues" to the exact labels and add a short directive to only read
content under those headers (ignore similarly named lines elsewhere) before
deciding which linked tickets to fetch for follow-up.
| local data | ||
| data=$(curl -sS --connect-timeout 10 --max-time 30 \ | ||
| -u "$JIRA_EMAIL:$JIRA_TOKEN" \ | ||
| "$JIRA_INSTANCE/rest/api/3/issue/$key?fields=summary,status,issuetype,description,issuelinks,subtasks,parent" 2>/dev/null) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent failure on curl errors may hide connectivity issues.
Redirecting stderr to /dev/null on the curl command suppresses connection errors, timeouts, and SSL issues. While the JSON parsing check on line 50 catches missing data, users won't see why a fetch failed.
Proposed fix to preserve error visibility
local data
- data=$(curl -sS --connect-timeout 10 --max-time 30 \
+ data=$(curl -sS --fail-with-body --connect-timeout 10 --max-time 30 \
-u "$JIRA_EMAIL:$JIRA_TOKEN" \
- "$JIRA_INSTANCE/rest/api/3/issue/$key?fields=summary,status,issuetype,description,issuelinks,subtasks,parent" 2>/dev/null)
+ "$JIRA_INSTANCE/rest/api/3/issue/$key?fields=summary,status,issuetype,description,issuelinks,subtasks,parent" 2>&1) || trueThis preserves error messages while still allowing the script to continue and report the error on lines 153-156.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local data | |
| data=$(curl -sS --connect-timeout 10 --max-time 30 \ | |
| -u "$JIRA_EMAIL:$JIRA_TOKEN" \ | |
| "$JIRA_INSTANCE/rest/api/3/issue/$key?fields=summary,status,issuetype,description,issuelinks,subtasks,parent" 2>/dev/null) | |
| local data | |
| data=$(curl -sS --fail-with-body --connect-timeout 10 --max-time 30 \ | |
| -u "$JIRA_EMAIL:$JIRA_TOKEN" \ | |
| "$JIRA_INSTANCE/rest/api/3/issue/$key?fields=summary,status,issuetype,description,issuelinks,subtasks,parent" 2>&1) || true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev-tools/fetch-jira.sh` around lines 45 - 49, The curl call that populates
the local variable data currently redirects stderr to /dev/null, hiding
connection/SSL/timeouts; remove the trailing "2>/dev/null" (or capture stderr to
a separate variable if you prefer) in the curl invocation that uses $JIRA_EMAIL,
$JIRA_TOKEN and $JIRA_INSTANCE so connection errors are visible when the script
fails JSON parsing of data; keep the existing JSON presence/error handling logic
so the script still reports failures downstream.
| # Search for child issues via parent= JQL (Jira Cloud hierarchy) | ||
| CHILD_KEYS=$(curl -sS --connect-timeout 10 --max-time 30 \ | ||
| -u "$JIRA_EMAIL:$JIRA_TOKEN" \ | ||
| "$JIRA_INSTANCE/rest/api/3/search/jql?jql=parent%3D${TICKET}&fields=key,summary,status,issuetype&maxResults=20" 2>/dev/null | \ | ||
| python3 -c " | ||
| import json, sys | ||
| try: | ||
| data = json.load(sys.stdin) | ||
| for issue in data.get('issues', []): | ||
| key = issue['key'] | ||
| summary = issue['fields']['summary'] | ||
| status = issue['fields']['status']['name'] | ||
| itype = issue['fields']['issuetype']['name'] | ||
| print(f'{key} ({itype}) [{status}]: {summary}') | ||
| except Exception: | ||
| pass | ||
| " 2>/dev/null) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Child issues query silently swallows all errors.
The 2>/dev/null at the end of the curl pipe and except Exception: pass in the Python block mean any API errors (auth failures, rate limits, invalid JQL) are silently ignored. Consider logging or at least not suppressing the curl stderr.
Proposed improvement
CHILD_KEYS=$(curl -sS --connect-timeout 10 --max-time 30 \
-u "$JIRA_EMAIL:$JIRA_TOKEN" \
- "$JIRA_INSTANCE/rest/api/3/search/jql?jql=parent%3D${TICKET}&fields=key,summary,status,issuetype&maxResults=20" 2>/dev/null | \
+ "$JIRA_INSTANCE/rest/api/3/search/jql?jql=parent%3D${TICKET}&fields=key,summary,status,issuetype&maxResults=20" | \
python3 -c "
import json, sys
try:
data = json.load(sys.stdin)
for issue in data.get('issues', []):
key = issue['key']
summary = issue['fields']['summary']
status = issue['fields']['status']['name']
itype = issue['fields']['issuetype']['name']
print(f'{key} ({itype}) [{status}]: {summary}')
-except Exception:
- pass
+except Exception as e:
+ print(f'Warning: Could not fetch child issues: {e}', file=sys.stderr)
" 2>/dev/null)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev-tools/fetch-jira.sh` around lines 162 - 178, The CHILD_KEYS pipeline
currently hides errors because the curl call redirects stderr to /dev/null and
the Python block swallows exceptions; update the pipeline around CHILD_KEYS so
curl does not discard stderr (remove the trailing 2>/dev/null) and change the
python3 block in the heredoc to surface failures (replace the bare except/ pass
with printing a meaningful error to stderr and non-zero exit, or re-raise the
exception) and/or check curl's exit status before piping into python3 so
authentication/JQL/API errors are visible; refer to the CHILD_KEYS assignment,
the curl invocation, and the python3 block to locate and change these behaviors.
| # Check for unfilled FIXMEs | ||
| if grep -q "FIXME" "$JIRA_CREDS"; then | ||
| echo "Error: $JIRA_CREDS still contains FIXME entries. Edit it with your credentials." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider more precise FIXME detection to avoid false positives.
The grep -q "FIXME" check could match if a user's email or token legitimately contains "FIXME" (unlikely but possible). A more robust check would verify the specific placeholder format.
Proposed refinement
# Check for unfilled FIXMEs
- if grep -q "FIXME" "$JIRA_CREDS"; then
+ if grep -qE '"(email|token)"[[:space:]]*:[[:space:]]*"FIXME' "$JIRA_CREDS"; then
echo "Error: $JIRA_CREDS still contains FIXME entries. Edit it with your credentials."
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Check for unfilled FIXMEs | |
| if grep -q "FIXME" "$JIRA_CREDS"; then | |
| echo "Error: $JIRA_CREDS still contains FIXME entries. Edit it with your credentials." | |
| exit 1 | |
| fi | |
| # Check for unfilled FIXMEs | |
| if grep -qE '"(email|token)"[[:space:]]*:[[:space:]]*"FIXME' "$JIRA_CREDS"; then | |
| echo "Error: $JIRA_CREDS still contains FIXME entries. Edit it with your credentials." | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev-tools/jira-common.sh` around lines 22 - 26, The current check using grep
-q "FIXME" on $JIRA_CREDS can produce false positives; update the logic in
dev-tools/jira-common.sh to look for the specific placeholder pattern(s) used in
the credentials file (e.g., keys set to the literal placeholder like
username=FIXME or token=FIXME) instead of any occurrence of the substring
"FIXME"; change the grep invocation around the JIRA_CREDS check to use a
stricter regex (or multiple checks) that matches the placeholder key/value forms
only so legitimate values containing "FIXME" do not fail the check.
| JIRA_EMAIL=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['email'])") | ||
| JIRA_TOKEN=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['token'])") | ||
| JIRA_INSTANCE=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['instance'])") |
There was a problem hiding this comment.
Address shellcheck SC2034 warnings by exporting variables.
The pipeline flags these variables as unused because shellcheck doesn't follow the sourcing relationship. Since these variables are intentionally set for use by calling scripts (fetch-jira.sh, file-jiras.sh), either export them or add shellcheck directives.
Proposed fix
- JIRA_EMAIL=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['email'])")
- JIRA_TOKEN=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['token'])")
- JIRA_INSTANCE=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['instance'])")
+ # shellcheck disable=SC2034 # Variables used by sourcing scripts
+ JIRA_EMAIL=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['email'])")
+ # shellcheck disable=SC2034
+ JIRA_TOKEN=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['token'])")
+ # shellcheck disable=SC2034
+ JIRA_INSTANCE=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['instance'])")Alternatively, export the variables if they need to be available to subprocesses:
- JIRA_EMAIL=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['email'])")
- JIRA_TOKEN=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['token'])")
- JIRA_INSTANCE=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['instance'])")
+ export JIRA_EMAIL JIRA_TOKEN JIRA_INSTANCE
+ JIRA_EMAIL=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['email'])")
+ JIRA_TOKEN=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['token'])")
+ JIRA_INSTANCE=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['instance'])")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| JIRA_EMAIL=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['email'])") | |
| JIRA_TOKEN=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['token'])") | |
| JIRA_INSTANCE=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['instance'])") | |
| # shellcheck disable=SC2034 # Variables used by sourcing scripts | |
| JIRA_EMAIL=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['email'])") | |
| # shellcheck disable=SC2034 | |
| JIRA_TOKEN=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['token'])") | |
| # shellcheck disable=SC2034 | |
| JIRA_INSTANCE=$(python3 -c "import json; print(json.load(open('$JIRA_CREDS'))['instance'])") |
🧰 Tools
🪛 GitHub Actions: Shell check
[warning] 28-28: Shellcheck SC2034: JIRA_EMAIL appears unused. Verify use (or export if used externally).
[warning] 29-29: Shellcheck SC2034: JIRA_TOKEN appears unused. Verify use (or export if used externally).
[warning] 30-30: Shellcheck SC2034: JIRA_INSTANCE appears unused. Verify use (or export if used externally).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev-tools/jira-common.sh` around lines 28 - 30, The JIRA credential variables
JIRA_EMAIL, JIRA_TOKEN, and JIRA_INSTANCE in jira-common.sh are flagged as
unused by shellcheck because they’re intended for sourced/subprocess use; make
them exported so subprocesses and shellcheck see them as used. Modify the
existing assignments for JIRA_EMAIL, JIRA_TOKEN, and JIRA_INSTANCE (the python3
JSON-extraction lines) to export the variables (or add explicit export
statements immediately after those assignments) so they become environment
variables available to fetch-jira.sh and file-jiras.sh and satisfy SC2034.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev-tools/file-jiras.sh`:
- Around line 83-86: The get_type() function's fallback never fires because the
pipeline (grep | head | sed) exits 0 even when producing empty output; change
the implementation to detect empty results and return "Task" when no match is
found: run grep (or grep -o) into a variable or use grep -q to test presence
first, then if the captured value is non-empty echo that value, otherwise echo
"Task"; refer to the get_type function and the pipeline using grep/head/sed to
locate where to add the conditional check and fallback.
- Around line 215-218: The current snippet interpolates $JIRA_DIR directly into
the Python -c string when setting SPIKE_TICKET_KEY, which risks unsafe shell
interpolation; instead pass the metadata file path as an argument to Python (use
$JIRA_DIR/.meta.json as a positional param) and have the Python snippet read
sys.argv[1] to open the file and print the 'spike_ticket' value—update the
SPIKE_TICKET_KEY assignment and mirror the safer pattern used elsewhere (e.g.,
link_spike_to_epic) so no shell-expanded path is embedded inside the Python code
string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5900193c-56de-4bc5-aace-021581b2b3df
📒 Files selected for processing (1)
dev-tools/file-jiras.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: E2E: server mode / ci
- GitHub Check: E2E: library mode / ci
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
🪛 GitHub Actions: Shell check
dev-tools/file-jiras.sh
[warning] 22-22: shellcheck (SC1091): Not following: jira-common.sh was not specified as input (see shellcheck -x).
🔇 Additional comments (11)
dev-tools/file-jiras.sh (11)
46-54: Argument parsing still lacks validation for missing option values.The previous review identified that running
file-jiras.sh --spike-docwithout a following argument causesshift 2to fail. This issue remains unaddressed.
93-101:sed -iwithout backup is non-portable and will fail on macOS.This issue was previously flagged and remains unaddressed.
263-265: Off-by-one risk inget_file_by_numberremains.The previous review identified that empty or non-numeric input could produce unexpected results. This issue remains unaddressed.
319-335: Shell variable interpolation and silent failure issues remain.The previous review identified two issues in this function:
$SPIKE_TICKET_KEYand$EPIC_KEYare interpolated directly into Python code (injection risk).- Curl output is suppressed, hiding the root cause when linking fails.
Both issues remain unaddressed.
563-574: Epic key overwrite logic issue remains.The previous review identified that
EPIC_KEYis set fromepic_existingon line 568, but then unconditionally overwritten on line 570 regardless of whether an existing key was found. This issue remains unaddressed.
19-26: LGTM!Clean initialization of global state variables and proper sourcing of the shared credential manager. The
shellcheck source=directive correctly handles the SC1091 warning since the path is dynamically resolved.
56-79: LGTM!Input validation is comprehensive: required arguments are checked, bare numbers are normalized to LCORE- prefix, and the default output directory is correctly derived from the spike doc's parent directory.
115-211: Well-structured parsing logic.The Python parser handles multiple edge cases well: spike key extraction with fallbacks, Epic stub generation with meaningful title derivation, and proper type metadata extraction from preceding sections. The cleanup of leaked
<!-- type: ... -->lines (line 189) is a nice defensive measure.
267-313: LGTM!Good interactive UX with clear options for Epic handling. Reading from
/dev/ttycorrectly handles the case when stdin is redirected. Error handling is appropriate for each branch.
337-556: LGTM!The update vs create logic is well-structured:
- Updates skip duplicate check (sensible since we know the ticket exists).
- HTTP status codes are correctly checked (204 for PUT, 201 for POST).
- Error responses are captured and displayed for debugging.
- The key is written back to the file after successful creation.
680-710: LGTM!The subshell workaround for refreshing
EPIC_KEYfrom the file (lines 702-709) is a pragmatic solution to the limitation that subshells can't propagate variable changes to the parent shell. The comment on line 702 clearly documents this.
| get_type() { | ||
| local f="$1" | ||
| grep -o '<!-- type: [A-Za-z]* -->' "$f" 2>/dev/null | head -1 | sed 's/<!-- type: //;s/ -->//' || echo "Task" | ||
| } |
There was a problem hiding this comment.
get_type() default "Task" fallback never triggers.
The || echo "Task" is evaluated based on the pipeline's exit status, but sed always succeeds (exit 0) even when processing empty input from grep. When no <!-- type: ... --> comment exists, this returns an empty string instead of "Task".
Proposed fix
get_type() {
local f="$1"
- grep -o '<!-- type: [A-Za-z]* -->' "$f" 2>/dev/null | head -1 | sed 's/<!-- type: //;s/ -->//' || echo "Task"
+ local t
+ t=$(grep -o '<!-- type: [A-Za-z]* -->' "$f" 2>/dev/null | head -1 | sed 's/<!-- type: //;s/ -->//')
+ echo "${t:-Task}"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev-tools/file-jiras.sh` around lines 83 - 86, The get_type() function's
fallback never fires because the pipeline (grep | head | sed) exits 0 even when
producing empty output; change the implementation to detect empty results and
return "Task" when no match is found: run grep (or grep -o) into a variable or
use grep -q to test presence first, then if the captured value is non-empty echo
that value, otherwise echo "Task"; refer to the get_type function and the
pipeline using grep/head/sed to locate where to add the conditional check and
fallback.
| # --- Read metadata --- | ||
| if [ -f "$JIRA_DIR/.meta.json" ]; then | ||
| SPIKE_TICKET_KEY=$(python3 -c "import json; print(json.load(open('$JIRA_DIR/.meta.json')).get('spike_ticket', ''))") | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Shell variable interpolation in Python string.
$JIRA_DIR is interpolated directly into the Python code string. While unlikely to be exploited in practice, this is the same pattern flagged in link_spike_to_epic. Consider passing the path as an argument.
Safer approach
if [ -f "$JIRA_DIR/.meta.json" ]; then
- SPIKE_TICKET_KEY=$(python3 -c "import json; print(json.load(open('$JIRA_DIR/.meta.json')).get('spike_ticket', ''))")
+ SPIKE_TICKET_KEY=$(python3 -c "import json,sys; print(json.load(open(sys.argv[1])).get('spike_ticket', ''))" "$JIRA_DIR/.meta.json")
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # --- Read metadata --- | |
| if [ -f "$JIRA_DIR/.meta.json" ]; then | |
| SPIKE_TICKET_KEY=$(python3 -c "import json; print(json.load(open('$JIRA_DIR/.meta.json')).get('spike_ticket', ''))") | |
| fi | |
| # --- Read metadata --- | |
| if [ -f "$JIRA_DIR/.meta.json" ]; then | |
| SPIKE_TICKET_KEY=$(python3 -c "import json,sys; print(json.load(open(sys.argv[1])).get('spike_ticket', ''))" "$JIRA_DIR/.meta.json") | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev-tools/file-jiras.sh` around lines 215 - 218, The current snippet
interpolates $JIRA_DIR directly into the Python -c string when setting
SPIKE_TICKET_KEY, which risks unsafe shell interpolation; instead pass the
metadata file path as an argument to Python (use $JIRA_DIR/.meta.json as a
positional param) and have the Python snippet read sys.argv[1] to open the file
and print the 'spike_ticket' value—update the SPIKE_TICKET_KEY assignment and
mirror the safer pattern used elsewhere (e.g., link_spike_to_epic) so no
shell-expanded path is embedded inside the Python code string.
max-svistunov
changed the title
…y safety, stale references
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
dev-tools/file-jiras.sh (1)
89-92:⚠️ Potential issue | 🟡 Minor
get_type()default "Task" fallback never triggers.The
|| echo "Task"is evaluated based on pipeline exit status, butsedalways succeeds (exit 0) even when processing empty input fromgrep. When no<!-- type: ... -->comment exists, this returns an empty string instead of "Task".Proposed fix
get_type() { local f="$1" - grep -o '<!-- type: [A-Za-z]* -->' "$f" 2>/dev/null | head -1 | sed 's/<!-- type: //;s/ -->//' || echo "Task" + local t + t=$(grep -o '<!-- type: [A-Za-z]* -->' "$f" 2>/dev/null | head -1 | sed 's/<!-- type: //;s/ -->//') + echo "${t:-Task}" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-tools/file-jiras.sh` around lines 89 - 92, The get_type() function's fallback never triggers because sed returns exit 0 even on empty input; fix by capturing the pipeline output into a variable (e.g., type="$(grep -o '...'" ) then check if the variable is empty and echo "Task" when empty, otherwise echo the captured value; update the function get_type to perform this empty-string test rather than relying on || after the pipeline so the default "Task" is returned when no <!-- type: ... --> comment exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev-tools/fetch-jira.sh`:
- Around line 41-157: The recursive extract_text function inside fetch_ticket
can hit Python's recursion limit for deeply nested ADF nodes; replace it with an
iterative stack-based traversal (or add a try/except around the current call to
catch RecursionError and fall back to a safe non-recursive extraction) so
extremely deep descriptions won't crash the script. Locate extract_text in the
Python block invoked by fetch_ticket, implement a stack/loop that processes
nodes and their depth (preserving handling for types like text, listItem,
bulletList/orderedList, heading, codeBlock, paragraph, hardBreak) or wrap
json.loads/sys.argv processing in try/except RecursionError and produce a simple
plaintext fallback (e.g., strip markup and join visible text) when recursion
fails. Ensure the fallback still prints the key/summary/status and doesn't abort
the shell function.
---
Duplicate comments:
In `@dev-tools/file-jiras.sh`:
- Around line 89-92: The get_type() function's fallback never triggers because
sed returns exit 0 even on empty input; fix by capturing the pipeline output
into a variable (e.g., type="$(grep -o '...'" ) then check if the variable is
empty and echo "Task" when empty, otherwise echo the captured value; update the
function get_type to perform this empty-string test rather than relying on ||
after the pipeline so the default "Task" is returned when no <!-- type: ... -->
comment exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9249608a-505f-4beb-9555-0ef512b8376c
📒 Files selected for processing (5)
.claude/commands/file-jiras.mddev-tools/fetch-jira.shdev-tools/file-jiras.shdev-tools/jira-common.shdocs/contributing_guide.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: library mode / ci
- GitHub Check: E2E: server mode / ci
🔇 Additional comments (15)
dev-tools/fetch-jira.sh (3)
162-178: Child issues query silently swallows all errors.The
2>/dev/nullon curl andexcept Exception: passin Python hide API errors (auth failures, rate limits, invalid JQL). Consider surfacing at least a warning when the query fails.
1-40: LGTM!Clear CLI interface with good help text, proper ticket normalization, and consistent sourcing of shared credentials.
188-197: LGTM!Multiple ticket handling with proper normalization and visual separators.
dev-tools/file-jiras.sh (9)
231-234: Shell variable interpolation in Python string.
$JIRA_DIRis interpolated directly into the Python code string. While unlikely to be exploited from a directory path, this is a code smell. Consider passing the path as an argument.
279-281: Off-by-one risk inget_file_by_numberwith invalid input.If
$1is empty or non-numeric,$((${1} + 1))produces unexpected results (empty string becomes 0, non-numeric may error). Consider adding validation.
331-351: Shell variable interpolation in Python string poses injection risk.Lines 340-341 interpolate
$SPIKE_TICKET_KEYand$EPIC_KEYdirectly into Python code. While Jira keys are constrained, this pattern is fragile.
29-60: LGTM!Argument parsing correctly validates that required options have values before shifting, preventing "can't shift that many" errors.
99-117: LGTM!Portable
_sed_iwrapper correctly handles macOS vs GNU sed differences. Theset_keyfunction properly uses it for both substitution and insertion cases.
119-134: LGTM!Parse skip logic is well-implemented with proper user prompting and printf format string using
%splaceholder.
353-572:file_single_ticketupdate-vs-create logic is well-structured.The function correctly:
- Skips duplicate checks for updates (line 366)
- Handles HTTP 204 for successful PUT, 201 for successful POST
- Writes key back to file after creation (line 563)
- Returns appropriate exit codes
574-618: LGTM!Epic handling logic correctly:
- Preserves existing Epic key before attempting to file
- Uses
filed_keyintermediate variable to avoid overwriting on failure- Properly refreshes Epic key from
00-epic.mdfor child tickets- Handles
__NONE__fallback case
700-731: Subshell variable scope handled correctly.Lines 722-729 properly refresh
EPIC_KEYfrom the file after filing, working around the subshell variable propagation limitation. Good defensive coding..claude/commands/file-jiras.md (1)
6-34: LGTM!The documentation correctly reflects the new flag-based CLI usage, credential behavior (creates with FIXMEs and exits), and updated output directory defaults. The verification checklist aligns with the
file-jiras.shworkflow.docs/contributing_guide.md (1)
122-166: LGTM!The feature design process documentation is well-structured and correctly documents:
- Slash commands with example arguments (
/spike LCORE-1234)- Full flag-based CLI invocations for
file-jiras.sh- The new step for updating spike docs with filed ticket numbers
- CLI tools section for non-Claude Code usage
dev-tools/jira-common.sh (1)
1-35: LGTM!The shared credentials module handles:
- Secure file creation with
chmod 600- Clear error messages guiding users to fill in credentials
- Proper shellcheck suppression for variables used by sourcing scripts
The FIXME detection (
grep -q "FIXME") is simple but sufficient for this use case since the placeholder strings are distinctive.
| fetch_ticket() { | ||
| local key="$1" | ||
| local indent="${2:-}" | ||
|
|
||
| local data | ||
| data=$(curl -sS --connect-timeout 10 --max-time 30 \ | ||
| -u "$JIRA_EMAIL:$JIRA_TOKEN" \ | ||
| "$JIRA_INSTANCE/rest/api/3/issue/$key?fields=summary,status,issuetype,description,issuelinks,subtasks,parent" 2>/dev/null) | ||
|
|
||
| if echo "$data" | python3 -c "import sys,json; json.load(sys.stdin)['key']" >/dev/null 2>&1; then | ||
| python3 -c " | ||
| import json, sys, textwrap | ||
|
|
||
| data = json.loads(sys.argv[1]) | ||
| indent = sys.argv[2] | ||
| key = data['key'] | ||
| fields = data['fields'] | ||
| summary = fields['summary'] | ||
| status = fields['status']['name'] | ||
| issue_type = fields['issuetype']['name'] | ||
| parent = fields.get('parent', {}) | ||
| parent_key = parent.get('key', '') if parent else '' | ||
|
|
||
| print(f'{indent}=== {key}: {summary} ===') | ||
| print(f'{indent}Type: {issue_type} | Status: {status}') | ||
| if parent_key: | ||
| print(f'{indent}Parent: {parent_key}') | ||
| print() | ||
|
|
||
| # Description | ||
| desc = fields.get('description') | ||
| if desc and isinstance(desc, dict): | ||
| # ADF format — extract text | ||
| def extract_text(node, depth=0): | ||
| lines = [] | ||
| if isinstance(node, dict): | ||
| ntype = node.get('type', '') | ||
| if ntype == 'text': | ||
| text = node.get('text', '') | ||
| marks = node.get('marks', []) | ||
| for m in marks: | ||
| if m.get('type') == 'strong': | ||
| text = f'**{text}**' | ||
| elif m.get('type') == 'code': | ||
| text = f'\`{text}\`' | ||
| return [text] | ||
| if ntype == 'hardBreak': | ||
| return ['\n'] | ||
| if ntype == 'listItem': | ||
| child_text = [] | ||
| for c in node.get('content', []): | ||
| child_text.extend(extract_text(c, depth)) | ||
| return [' ' * depth + '- ' + ''.join(child_text).strip()] | ||
| if ntype in ('bulletList', 'orderedList'): | ||
| for c in node.get('content', []): | ||
| lines.extend(extract_text(c, depth + 1)) | ||
| return lines | ||
| if ntype == 'heading': | ||
| level = node.get('attrs', {}).get('level', 1) | ||
| child_text = [] | ||
| for c in node.get('content', []): | ||
| child_text.extend(extract_text(c, depth)) | ||
| return ['#' * level + ' ' + ''.join(child_text).strip()] | ||
| if ntype == 'codeBlock': | ||
| child_text = [] | ||
| for c in node.get('content', []): | ||
| child_text.extend(extract_text(c, depth)) | ||
| return ['\`\`\`\n' + ''.join(child_text) + '\n\`\`\`'] | ||
| for c in node.get('content', []): | ||
| lines.extend(extract_text(c, depth)) | ||
| if ntype == 'paragraph' and lines: | ||
| lines.append('') | ||
| return lines | ||
|
|
||
| text_lines = extract_text(desc) | ||
| desc_text = '\n'.join(text_lines).strip() | ||
| if desc_text: | ||
| for line in desc_text.split('\n'): | ||
| print(f'{indent}{line}') | ||
| print() | ||
|
|
||
| # Links | ||
| links = fields.get('issuelinks', []) | ||
| if links: | ||
| print(f'{indent}Linked issues:') | ||
| for link in links: | ||
| link_type = link.get('type', {}).get('name', '?') | ||
| if 'outwardIssue' in link: | ||
| linked = link['outwardIssue'] | ||
| direction = link.get('type', {}).get('outward', 'relates to') | ||
| elif 'inwardIssue' in link: | ||
| linked = link['inwardIssue'] | ||
| direction = link.get('type', {}).get('inward', 'relates to') | ||
| else: | ||
| continue | ||
| lkey = linked['key'] | ||
| lsummary = linked['fields']['summary'] | ||
| lstatus = linked['fields']['status']['name'] | ||
| print(f'{indent} {direction}: {lkey} — {lsummary} [{lstatus}]') | ||
| print() | ||
|
|
||
| # Subtasks | ||
| subtasks = fields.get('subtasks', []) | ||
| if subtasks: | ||
| print(f'{indent}Child issues:') | ||
| for st in subtasks: | ||
| skey = st['key'] | ||
| ssummary = st['fields']['summary'] | ||
| sstatus = st['fields']['status']['name'] | ||
| print(f'{indent} {skey} — {ssummary} [{sstatus}]') | ||
| print() | ||
| " "$data" "$indent" | ||
| else | ||
| echo "${indent}Error fetching $key" | ||
| echo "$data" | head -3 | ||
| fi | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ADF parsing is comprehensive but extract_text recursion could hit Python's default limit.
Deeply nested Jira descriptions (e.g., 1000+ nested list levels) would cause RecursionError. This is unlikely with real Jira content but worth noting.
The error handling on line 153-156 provides a reasonable fallback when fetch fails.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev-tools/fetch-jira.sh` around lines 41 - 157, The recursive extract_text
function inside fetch_ticket can hit Python's recursion limit for deeply nested
ADF nodes; replace it with an iterative stack-based traversal (or add a
try/except around the current call to catch RecursionError and fall back to a
safe non-recursive extraction) so extremely deep descriptions won't crash the
script. Locate extract_text in the Python block invoked by fetch_ticket,
implement a stack/loop that processes nodes and their depth (preserving handling
for types like text, listItem, bulletList/orderedList, heading, codeBlock,
paragraph, hardBreak) or wrap json.loads/sys.argv processing in try/except
RecursionError and produce a simple plaintext fallback (e.g., strip markup and
join visible text) when recursion fails. Ensure the fallback still prints the
key/summary/status and doesn't abort the shell function.
2 participants
LCORE-1632: Feature design process improvements
Follow-up to PR #1399. Adds JIRA fetching, Epic+children hierarchy, ticket update support, and process doc improvements.
What's in this PR
New CLI tool:
dev-tools/fetch-jira.sh— fetch JIRA ticket content + child issues from the command linedev-tools/jira-common.sh— shared Jira credentials helper, auto-creates~/.config/jira/credentials.jsonwith FIXMEs if missingfile-jiras.sh rewrite:
--spike-doc,--feature-ticket,--output-dir,--help<!-- type: Task/Story/Epic -->metadata in each ticket file, parsed from the spike doc<!-- key: LCORE-xxxx -->placeholder for unfiled tickets, replaced with actual key after filingdocs/design/<feature>/jiras/docs/design/<feature>/jiras/Skills updated:
/spike— accepts JIRA number, callsfetch-jira.sh, mentions type metadata/spec-doc— searches for existing spike docs before starting from scratch/file-jiras— updated for new named args, references--helpDocs updated:
contributing_guide.md— AI-workflow-first: slash commands in step descriptions, explanation of skills before steps, CLI tools listedhowto-run-a-spike.md— new step for LCORE-???? substitution after filing, mentions jiras dir, updated file-jiras syntaxtemplates/index.md— new, fixes 404 on templates directory linktemplates/jira-ticket-template.md—<!-- type: -->and<!-- key: -->metadata addedtemplates/spike-template.md— same metadata addedTested against real Jira
Related: LCORE-1576
Summary by CodeRabbit
New Features
Documentation
Chores