Fix/[HZ-815] - #3
Open
gauravchavan2412 wants to merge 3 commits into
Open
Conversation
|
Static Code Review 📊 ✅ All quality checks passed! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis change adds a tag-pair release pipeline, richer repository diff processing, configurable weekly/monthly Linear ticket creation, expanded service coverage, and updated workflow documentation. ChangesRelease ticket workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Makefile
participant generate_custom_input_file
participant process_all_repos
participant create_monthly_release_ticket
participant Linear
Makefile->>generate_custom_input_file: Generate input.json from FROM_REF and TO_REF
generate_custom_input_file-->>Makefile: Write service tag matrix
Makefile->>process_all_repos: Process tag differences and ticket data
process_all_repos-->>Makefile: Write final_tag_differences.json
Makefile->>create_monthly_release_ticket: Build or preview release ticket
create_monthly_release_ticket->>Linear: Resolve metadata and create issue
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![typo-app[bot]](https://avatars.githubusercontent.com/in/360763?s=60&v=4){kind=small}
Comment on lines
+361
to
+366
| EXTRA="--release-kind $(RELEASE_KIND) --stackgen-tag $$tag --assignee-query $(ASSIGNEE_QUERY) --state-name $(STATE_NAME)"; \ | ||
| if [ -n "$(MONTH_LABEL)" ]; then EXTRA="$$EXTRA --month-label \"$(MONTH_LABEL)\""; fi; \ | ||
| if [ "$(DRY_RUN)" = "1" ]; then EXTRA="$$EXTRA --dry-run"; fi; \ | ||
| if [ -f "$$services_input" ]; then EXTRA="$$EXTRA --services-input \"$$services_input\""; fi; \ | ||
| echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \ | ||
| eval $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" $$EXTRA |
There was a problem hiding this comment.
logic: EXTRA is executed through eval, so values with spaces such as ASSIGNEE_QUERY="Jane Doe" break argparse and can misexecute. Pass arguments positionally without eval.
Suggested change
| EXTRA="--release-kind $(RELEASE_KIND) --stackgen-tag $$tag --assignee-query $(ASSIGNEE_QUERY) --state-name $(STATE_NAME)"; \ | |
| if [ -n "$(MONTH_LABEL)" ]; then EXTRA="$$EXTRA --month-label \"$(MONTH_LABEL)\""; fi; \ | |
| if [ "$(DRY_RUN)" = "1" ]; then EXTRA="$$EXTRA --dry-run"; fi; \ | |
| if [ -f "$$services_input" ]; then EXTRA="$$EXTRA --services-input \"$$services_input\""; fi; \ | |
| echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \ | |
| eval $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" $$EXTRA | |
| set -- --release-kind "$(RELEASE_KIND)" --stackgen-tag "$$tag" --assignee-query "$(ASSIGNEE_QUERY)" --state-name "$(STATE_NAME)"; \ | |
| if [ -n "$(MONTH_LABEL)" ]; then set -- "$$@" --month-label "$(MONTH_LABEL)"; fi; \ | |
| if [ "$(DRY_RUN)" = "1" ]; then set -- "$$@" --dry-run; fi; \ | |
| if [ -f "$$services_input" ]; then set -- "$$@" --services-input "$$services_input"; fi; \ | |
| echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \ | |
| $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" "$$@" |
Comment on lines
+666
to
+667
| if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'): | ||
| all_tickets.append(f"{ticket_id}: {details['issueUrl']}") |
There was a problem hiding this comment.
logic: Unresolved tickets are written as ID: https://..., which the release parser splits into status https and a broken title. Preserve the three-field ticket format.
Suggested change
| if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'): | |
| all_tickets.append(f"{ticket_id}: {details['issueUrl']}") | |
| if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'): | |
| all_tickets.append(f"{ticket_id:<{max_ticket_id_len}}: {'Unknown':<{max_status_len}}: {details['issueUrl']}") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
create_monthly_release_ticket.py (1)
654-675: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNon-idempotent retry can create duplicate Linear issues.
The loop retries
issueCreateon any exception, including a timeout or connection reset raised after Linear already created the issue. Restrict the retry to template-argument rejections rather than treating every failure as retryable.🛡️ Suggested guard
last_error: Optional[Exception] = None - for issue_input in candidate_inputs: + for attempt, issue_input in enumerate(candidate_inputs): try: payload = linear_request(api_key, mutation, {"input": issue_input}).get( "issueCreate", {} ) if not payload.get("success"): raise RuntimeError("issueCreate returned success=false") created_issue = payload.get("issue") if not created_issue: raise RuntimeError("issueCreate returned no issue") return created_issue except Exception as exc: last_error = exc + # Only fall through to the next input shape when the template field + # itself was rejected; anything else may have created the issue. + if "template" not in str(exc).lower(): + break🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@create_monthly_release_ticket.py` around lines 654 - 675, The issueCreate loop retries every exception, including uncertain transport failures that may follow a successful Linear issue creation. Update the retry logic around linear_request so only errors indicating rejected template arguments try the next candidate input; immediately propagate timeouts, connection resets, and other non-template failures, while preserving the existing candidate order and final error reporting.
🧹 Nitpick comments (3)
create_monthly_release_ticket.py (1)
140-147: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUnreachable fallback:
data.get(...)raises before theisinstance(data, list)check.If a bare list were ever passed, Line 141 raises
AttributeError, so the list branch can never execute. Guard the type first.♻️ Suggested change
rows: List[Dict[str, Any]] = [] - services = data.get("services") - if not isinstance(services, list): - # input.json is a bare list - if isinstance(data, list): - services = data - else: - return rows + if isinstance(data, list): + services = data + else: + services = data.get("services") + if not isinstance(services, list): + return rows🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@create_monthly_release_ticket.py` around lines 140 - 147, Update the data handling around services extraction so the input type is checked before calling data.get: preserve bare lists as services, read the "services" key only for mapping-like input, and return the initialized rows for unsupported types.release_pipeline/pipeline.py (1)
97-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMutating the caller's
MonthlyTicketConfigin place.Line 102 writes into the object owned by the caller, unlike the
replace()used forinput_pathat Lines 140-143. Prefer a copy for consistency and to avoid surprising callers that reuse the config.♻️ Suggested change
- resolved_ticket_config.stackgen_tag = stackgen_tag.strip() + resolved_ticket_config = replace( + resolved_ticket_config, stackgen_tag=stackgen_tag.strip() + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@release_pipeline/pipeline.py` around lines 97 - 102, Update the stackgen_tag assignment in the pipeline configuration resolution flow to copy resolved_ticket_config before setting the fallback tag, rather than mutating the caller-owned MonthlyTicketConfig in place. Preserve the existing conditions and fallback value, and use the same copy/replace approach already used for input_path.Makefile (1)
19-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePersonal assignee email hardcoded as a default in three places. The same individual's address is the fallback assignee across the Make config and both Python CLIs, so a role change requires edits in every file.
Makefile#L19-L22: allow an env override for theASSIGNEE_QUERYdefault instead of the literal address.run_monthly_release.py#L87-L91: take the--assignee-querydefault from an environment variable (falling back to the shared constant).create_monthly_release_ticket.py#L811-L815: define one module-levelDEFAULT_ASSIGNEEconstant used by both the dataclass default and this argument.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 19 - 22, Remove the hardcoded personal assignee fallback across all three sites: in Makefile lines 19-22, make ASSIGNEE_QUERY overridable through the environment; in run_monthly_release.py lines 87-91, derive --assignee-query’s default from an environment variable with the shared constant as fallback; and in create_monthly_release_ticket.py lines 811-815, define one module-level DEFAULT_ASSIGNEE and reuse it for both the dataclass default and argument default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 292-332: Update the create-release-ticket pipeline recipe to fail
immediately when any cleanup, generation, fetch, or ticket-creation command
fails. Add shell fail-fast behavior at the start of the recipe or explicitly
chain each command with failure propagation, ensuring the final success message
and zero exit status occur only after all steps complete successfully; preserve
the existing Step 1–4 flow and logging.
- Around line 361-366: Update the release-ticket command around EXTRA and the
create_monthly_release_ticket.py invocation to remove eval and pass every option
as a separately quoted argument, including RELEASE_KIND, tag, ASSIGNEE_QUERY,
STATE_NAME, MONTH_LABEL, and services_input. Preserve conditional inclusion of
the optional arguments and ensure values containing spaces or shell
metacharacters remain single literal arguments.
In `@process_all_repos.py`:
- Around line 634-643: Prune each service result’s tickets and ticket_count to
included_tickets_set before constructing the aggregated output, ensuring
canceled IDs are absent from services[].tickets and all counts match. Update the
console summary and metadata.total_unique_tickets to use the same filtered set,
while preserving the existing project aggregation flow.
- Around line 250-251: Update the HTTP 400 handling in the method containing the
response-status branch to inspect the response body using the same
error-discrimination logic as the 200-with-errors path; only return
_unresolved_issue_placeholder(ticket_id) for a confirmed unresolved-ticket
condition, and propagate or handle malformed-query and authentication errors
through the existing error path.
- Around line 664-668: Update the unresolved-ticket branch in the loop over
sorted included_tickets_set so each all_tickets entry preserves the parser’s
three-field ticket_id: state: title format; do not place the issue URL directly
after the first delimiter because its https colon is parsed as a field
separator. Reuse the existing ticket details to emit an appropriate state and
title while keeping the issue URL available in the expected field position.
In `@README.md`:
- Around line 110-118: Update the “Linear issue shape” table to describe the
Assignee and Status values as defaults rather than fixed requirements: identify
`gaurav@stackgen.com` and `Todo` as the default values, and state that operators
can override them through the exposed `ASSIGNEE_QUERY` and `STATE_NAME` Make
parameters.
- Line 93: The README currently documents literal ref-based artifact paths, but
the Makefile sanitizes refs by replacing slashes, spaces, and colons. Update
README.md lines 93-93 to describe and exemplify the sanitized
<FROM_REF>-<TO_REF> directory, and update README.md lines 299-299 to reference
that sanitized directory or document the transformation.
In `@release_pipeline/pipeline.py`:
- Around line 92-96: Set the default release kind to monthly in the
MonthlyTicketConfig created by run_monthly_release_pipeline in
release_pipeline/pipeline.py (lines 92-96), and change the --release-kind CLI
default in run_monthly_release.py (lines 96-101) to monthly so both monthly
entry points generate monthly release titles by default.
---
Outside diff comments:
In `@create_monthly_release_ticket.py`:
- Around line 654-675: The issueCreate loop retries every exception, including
uncertain transport failures that may follow a successful Linear issue creation.
Update the retry logic around linear_request so only errors indicating rejected
template arguments try the next candidate input; immediately propagate timeouts,
connection resets, and other non-template failures, while preserving the
existing candidate order and final error reporting.
---
Nitpick comments:
In `@create_monthly_release_ticket.py`:
- Around line 140-147: Update the data handling around services extraction so
the input type is checked before calling data.get: preserve bare lists as
services, read the "services" key only for mapping-like input, and return the
initialized rows for unsupported types.
In `@Makefile`:
- Around line 19-22: Remove the hardcoded personal assignee fallback across all
three sites: in Makefile lines 19-22, make ASSIGNEE_QUERY overridable through
the environment; in run_monthly_release.py lines 87-91, derive
--assignee-query’s default from an environment variable with the shared constant
as fallback; and in create_monthly_release_ticket.py lines 811-815, define one
module-level DEFAULT_ASSIGNEE and reuse it for both the dataclass default and
argument default.
In `@release_pipeline/pipeline.py`:
- Around line 97-102: Update the stackgen_tag assignment in the pipeline
configuration resolution flow to copy resolved_ticket_config before setting the
fallback tag, rather than mutating the caller-owned MonthlyTicketConfig in
place. Preserve the existing conditions and fallback value, and use the same
copy/replace approach already used for input_path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 84ab6422-e160-46ae-a648-ade75b5783a5
📒 Files selected for processing (8)
MakefileREADME.mdcreate_monthly_release_ticket.pydocs/create-release-ticket.mdgenerate_input_json.pyprocess_all_repos.pyrelease_pipeline/pipeline.pyrun_monthly_release.py
Comment on lines
+292
to
+332
| @from_safe=$$(printf '%s' "$(FROM_REF)" | sed 's|[/ :]|-|g'); \ | ||
| to_safe=$$(printf '%s' "$(TO_REF)" | sed 's|[/ :]|-|g'); \ | ||
| out_dir="$(OUT_DIR)"; \ | ||
| if [ -z "$$out_dir" ]; then out_dir="$${from_safe}-$${to_safe}"; fi; \ | ||
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | ||
| echo "create-release-ticket pipeline"; \ | ||
| echo " FROM_REF=$(FROM_REF) → TO_REF=$(TO_REF)"; \ | ||
| echo " OUT_DIR=$$out_dir"; \ | ||
| echo " STACKGEN_TAG=$(if $(STACKGEN_TAG),$(STACKGEN_TAG),$(TO_REF))"; \ | ||
| echo " RELEASE_KIND=$(RELEASE_KIND) DRY_RUN=$(if $(DRY_RUN),$(DRY_RUN),0)"; \ | ||
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | ||
| echo ""; \ | ||
| echo "▶ Step 1/4 — clean $$out_dir"; \ | ||
| rm -rf "$$out_dir"; \ | ||
| echo "✅ Removed $$out_dir (if it existed)"; \ | ||
| echo ""; \ | ||
| echo "▶ Step 2/4 — generate-custom-input-file → $$out_dir/"; \ | ||
| $(MAKE) generate-custom-input-file \ | ||
| FROM_REF="$(FROM_REF)" \ | ||
| TO_REF="$(TO_REF)" \ | ||
| GENERATED_DIR="$$out_dir"; \ | ||
| echo ""; \ | ||
| echo "▶ Step 3/4 — fetch_changes_between_tags_from_input → $$out_dir/"; \ | ||
| $(MAKE) fetch_changes_between_tags_from_input GENERATED_DIR="$$out_dir"; \ | ||
| echo ""; \ | ||
| echo "▶ Step 4/4 — create Linear release ticket"; \ | ||
| $(MAKE) create-release-ticket-only \ | ||
| FROM_REF="$(FROM_REF)" \ | ||
| TO_REF="$(TO_REF)" \ | ||
| GENERATED_DIR="$$out_dir" \ | ||
| STACKGEN_TAG="$(if $(STACKGEN_TAG),$(STACKGEN_TAG),$(TO_REF))" \ | ||
| RELEASE_KIND="$(RELEASE_KIND)" \ | ||
| MONTH_LABEL="$(MONTH_LABEL)" \ | ||
| ASSIGNEE_QUERY="$(ASSIGNEE_QUERY)" \ | ||
| STATE_NAME="$(STATE_NAME)" \ | ||
| DRY_RUN="$(DRY_RUN)"; \ | ||
| echo ""; \ | ||
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | ||
| echo "✅ create-release-ticket pipeline finished"; \ | ||
| echo " Artifacts: $$out_dir/"; \ | ||
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Pipeline steps are chained with ;, so failures are swallowed.
The whole recipe is one shell invocation without set -e; if generate-custom-input-file, fetch_changes_between_tags_from_input, or create-release-ticket-only fails, execution continues and the recipe exits with the status of the final echo — make prints "✅ pipeline finished" and returns 0.
🐛 Proposed fix
- `@from_safe`=$$(printf '%s' "$(FROM_REF)" | sed 's|[/ :]|-|g'); \
+ `@set` -e; \
+ from_safe=$$(printf '%s' "$(FROM_REF)" | sed 's|[/ :]|-|g'); \
to_safe=$$(printf '%s' "$(TO_REF)" | sed 's|[/ :]|-|g'); \📝 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.
Suggested change
| @from_safe=$$(printf '%s' "$(FROM_REF)" | sed 's|[/ :]|-|g'); \ | |
| to_safe=$$(printf '%s' "$(TO_REF)" | sed 's|[/ :]|-|g'); \ | |
| out_dir="$(OUT_DIR)"; \ | |
| if [ -z "$$out_dir" ]; then out_dir="$${from_safe}-$${to_safe}"; fi; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | |
| echo "create-release-ticket pipeline"; \ | |
| echo " FROM_REF=$(FROM_REF) → TO_REF=$(TO_REF)"; \ | |
| echo " OUT_DIR=$$out_dir"; \ | |
| echo " STACKGEN_TAG=$(if $(STACKGEN_TAG),$(STACKGEN_TAG),$(TO_REF))"; \ | |
| echo " RELEASE_KIND=$(RELEASE_KIND) DRY_RUN=$(if $(DRY_RUN),$(DRY_RUN),0)"; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | |
| echo ""; \ | |
| echo "▶ Step 1/4 — clean $$out_dir"; \ | |
| rm -rf "$$out_dir"; \ | |
| echo "✅ Removed $$out_dir (if it existed)"; \ | |
| echo ""; \ | |
| echo "▶ Step 2/4 — generate-custom-input-file → $$out_dir/"; \ | |
| $(MAKE) generate-custom-input-file \ | |
| FROM_REF="$(FROM_REF)" \ | |
| TO_REF="$(TO_REF)" \ | |
| GENERATED_DIR="$$out_dir"; \ | |
| echo ""; \ | |
| echo "▶ Step 3/4 — fetch_changes_between_tags_from_input → $$out_dir/"; \ | |
| $(MAKE) fetch_changes_between_tags_from_input GENERATED_DIR="$$out_dir"; \ | |
| echo ""; \ | |
| echo "▶ Step 4/4 — create Linear release ticket"; \ | |
| $(MAKE) create-release-ticket-only \ | |
| FROM_REF="$(FROM_REF)" \ | |
| TO_REF="$(TO_REF)" \ | |
| GENERATED_DIR="$$out_dir" \ | |
| STACKGEN_TAG="$(if $(STACKGEN_TAG),$(STACKGEN_TAG),$(TO_REF))" \ | |
| RELEASE_KIND="$(RELEASE_KIND)" \ | |
| MONTH_LABEL="$(MONTH_LABEL)" \ | |
| ASSIGNEE_QUERY="$(ASSIGNEE_QUERY)" \ | |
| STATE_NAME="$(STATE_NAME)" \ | |
| DRY_RUN="$(DRY_RUN)"; \ | |
| echo ""; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | |
| echo "✅ create-release-ticket pipeline finished"; \ | |
| echo " Artifacts: $$out_dir/"; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" | |
| `@set` -e; \ | |
| from_safe=$$(printf '%s' "$(FROM_REF)" | sed 's|[/ :]|-|g'); \ | |
| to_safe=$$(printf '%s' "$(TO_REF)" | sed 's|[/ :]|-|g'); \ | |
| out_dir="$(OUT_DIR)"; \ | |
| if [ -z "$$out_dir" ]; then out_dir="$${from_safe}-$${to_safe}"; fi; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | |
| echo "create-release-ticket pipeline"; \ | |
| echo " FROM_REF=$(FROM_REF) → TO_REF=$(TO_REF)"; \ | |
| echo " OUT_DIR=$$out_dir"; \ | |
| echo " STACKGEN_TAG=$(if $(STACKGEN_TAG),$(STACKGEN_TAG),$(TO_REF))"; \ | |
| echo " RELEASE_KIND=$(RELEASE_KIND) DRY_RUN=$(if $(DRY_RUN),$(DRY_RUN),0)"; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | |
| echo ""; \ | |
| echo "▶ Step 1/4 — clean $$out_dir"; \ | |
| rm -rf "$$out_dir"; \ | |
| echo "✅ Removed $$out_dir (if it existed)"; \ | |
| echo ""; \ | |
| echo "▶ Step 2/4 — generate-custom-input-file → $$out_dir/"; \ | |
| $(MAKE) generate-custom-input-file \ | |
| FROM_REF="$(FROM_REF)" \ | |
| TO_REF="$(TO_REF)" \ | |
| GENERATED_DIR="$$out_dir"; \ | |
| echo ""; \ | |
| echo "▶ Step 3/4 — fetch_changes_between_tags_from_input → $$out_dir/"; \ | |
| $(MAKE) fetch_changes_between_tags_from_input GENERATED_DIR="$$out_dir"; \ | |
| echo ""; \ | |
| echo "▶ Step 4/4 — create Linear release ticket"; \ | |
| $(MAKE) create-release-ticket-only \ | |
| FROM_REF="$(FROM_REF)" \ | |
| TO_REF="$(TO_REF)" \ | |
| GENERATED_DIR="$$out_dir" \ | |
| STACKGEN_TAG="$(if $(STACKGEN_TAG),$(STACKGEN_TAG),$(TO_REF))" \ | |
| RELEASE_KIND="$(RELEASE_KIND)" \ | |
| MONTH_LABEL="$(MONTH_LABEL)" \ | |
| ASSIGNEE_QUERY="$(ASSIGNEE_QUERY)" \ | |
| STATE_NAME="$(STATE_NAME)" \ | |
| DRY_RUN="$(DRY_RUN)"; \ | |
| echo ""; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \ | |
| echo "✅ create-release-ticket pipeline finished"; \ | |
| echo " Artifacts: $$out_dir/"; \ | |
| echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 292 - 332, Update the create-release-ticket pipeline
recipe to fail immediately when any cleanup, generation, fetch, or
ticket-creation command fails. Add shell fail-fast behavior at the start of the
recipe or explicitly chain each command with failure propagation, ensuring the
final success message and zero exit status occur only after all steps complete
successfully; preserve the existing Step 1–4 flow and logging.
Comment on lines
+361
to
+366
| EXTRA="--release-kind $(RELEASE_KIND) --stackgen-tag $$tag --assignee-query $(ASSIGNEE_QUERY) --state-name $(STATE_NAME)"; \ | ||
| if [ -n "$(MONTH_LABEL)" ]; then EXTRA="$$EXTRA --month-label \"$(MONTH_LABEL)\""; fi; \ | ||
| if [ "$(DRY_RUN)" = "1" ]; then EXTRA="$$EXTRA --dry-run"; fi; \ | ||
| if [ -f "$$services_input" ]; then EXTRA="$$EXTRA --services-input \"$$services_input\""; fi; \ | ||
| echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \ | ||
| eval $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" $$EXTRA |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Unquoted variables passed through eval — injection and breakage on values with spaces.
RELEASE_KIND, $$tag, ASSIGNEE_QUERY, and STATE_NAME are interpolated unquoted into EXTRA and then re-parsed by eval. A STATE_NAME="In Progress" silently splits into two args, and any shell metacharacter in these values is executed (e.g. TO_REF='v1; …').
🛡️ Proposed fix — quote each value and drop `eval`
- EXTRA="--release-kind $(RELEASE_KIND) --stackgen-tag $$tag --assignee-query $(ASSIGNEE_QUERY) --state-name $(STATE_NAME)"; \
- if [ -n "$(MONTH_LABEL)" ]; then EXTRA="$$EXTRA --month-label \"$(MONTH_LABEL)\""; fi; \
- if [ "$(DRY_RUN)" = "1" ]; then EXTRA="$$EXTRA --dry-run"; fi; \
- if [ -f "$$services_input" ]; then EXTRA="$$EXTRA --services-input \"$$services_input\""; fi; \
- echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \
- eval $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" $$EXTRA
+ set -- --release-kind "$(RELEASE_KIND)" --stackgen-tag "$$tag" \
+ --assignee-query "$(ASSIGNEE_QUERY)" --state-name "$(STATE_NAME)"; \
+ if [ -n "$(MONTH_LABEL)" ]; then set -- "$$@" --month-label "$(MONTH_LABEL)"; fi; \
+ if [ "$(DRY_RUN)" = "1" ]; then set -- "$$@" --dry-run; fi; \
+ if [ -f "$$services_input" ]; then set -- "$$@" --services-input "$$services_input"; fi; \
+ echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \
+ $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" "$$@"📝 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.
Suggested change
| EXTRA="--release-kind $(RELEASE_KIND) --stackgen-tag $$tag --assignee-query $(ASSIGNEE_QUERY) --state-name $(STATE_NAME)"; \ | |
| if [ -n "$(MONTH_LABEL)" ]; then EXTRA="$$EXTRA --month-label \"$(MONTH_LABEL)\""; fi; \ | |
| if [ "$(DRY_RUN)" = "1" ]; then EXTRA="$$EXTRA --dry-run"; fi; \ | |
| if [ -f "$$services_input" ]; then EXTRA="$$EXTRA --services-input \"$$services_input\""; fi; \ | |
| echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \ | |
| eval $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" $$EXTRA | |
| set -- --release-kind "$(RELEASE_KIND)" --stackgen-tag "$$tag" \ | |
| --assignee-query "$(ASSIGNEE_QUERY)" --state-name "$(STATE_NAME)"; \ | |
| if [ -n "$(MONTH_LABEL)" ]; then set -- "$$@" --month-label "$(MONTH_LABEL)"; fi; \ | |
| if [ "$(DRY_RUN)" = "1" ]; then set -- "$$@" --dry-run; fi; \ | |
| if [ -f "$$services_input" ]; then set -- "$$@" --services-input "$$services_input"; fi; \ | |
| echo "Creating Linear ticket from $$output_file (tag=$$tag)…"; \ | |
| $(PYTHON) create_monthly_release_ticket.py --input "$$output_file" "$$@" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 361 - 366, Update the release-ticket command around
EXTRA and the create_monthly_release_ticket.py invocation to remove eval and
pass every option as a separately quoted argument, including RELEASE_KIND, tag,
ASSIGNEE_QUERY, STATE_NAME, MONTH_LABEL, and services_input. Preserve
conditional inclusion of the optional arguments and ensure values containing
spaces or shell metacharacters remain single literal arguments.
Comment on lines
+250
to
+251
| if response.status_code == 400: | ||
| return self._unresolved_issue_placeholder(ticket_id) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Blanket HTTP 400 → placeholder can mask real query/auth errors.
Unlike the 200-with-errors path, this branch does no discrimination: a malformed query or bad credentials would turn every ticket into a fabricated "unresolved" entry with a guessed browser URL. Reuse the same error inspection on the 400 body.
🛡️ Proposed fix
if response.status_code == 400:
- return self._unresolved_issue_placeholder(ticket_id)
+ try:
+ errs = response.json().get('errors')
+ except ValueError:
+ errs = None
+ if errs is None or self._graphql_errors_indicate_issue_not_found(errs):
+ return self._unresolved_issue_placeholder(ticket_id)
+ return None📝 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.
Suggested change
| if response.status_code == 400: | |
| return self._unresolved_issue_placeholder(ticket_id) | |
| if response.status_code == 400: | |
| try: | |
| errs = response.json().get('errors') | |
| except ValueError: | |
| errs = None | |
| if errs is None or self._graphql_errors_indicate_issue_not_found(errs): | |
| return self._unresolved_issue_placeholder(ticket_id) | |
| return None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@process_all_repos.py` around lines 250 - 251, Update the HTTP 400 handling in
the method containing the response-status branch to inspect the response body
using the same error-discrimination logic as the 200-with-errors path; only
return _unresolved_issue_placeholder(ticket_id) for a confirmed
unresolved-ticket condition, and propagate or handle malformed-query and
authentication errors through the existing error path.
Comment on lines
+634
to
+643
|
|
||
| # Omit tickets whose Linear state is Canceled from aggregated outputs | ||
| included_tickets_set = { | ||
| tid for tid in all_tickets_set | ||
| if not self._is_canceled_ticket(ticket_details_map.get(tid)) | ||
| } | ||
|
|
||
| # Collect unique project IDs from ticket details | ||
| project_ids = set() | ||
| for ticket_id in all_tickets_set: | ||
| for ticket_id in included_tickets_set: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Canceled tickets are filtered from aggregates but remain in services[].tickets.
results still carries canceled IDs, and the console summary at Lines 610/620-623 still counts them, so metadata.total_unique_tickets disagrees with the per-service lists. create_monthly_release_ticket.load_release_data falls back to data["services"][].tickets when tickets_by_project is empty, which would reintroduce canceled tickets into the Linear body.
Consider pruning result['tickets']/ticket_count against included_tickets_set before building the output structure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@process_all_repos.py` around lines 634 - 643, Prune each service result’s
tickets and ticket_count to included_tickets_set before constructing the
aggregated output, ensuring canceled IDs are absent from services[].tickets and
all counts match. Update the console summary and metadata.total_unique_tickets
to use the same filtered set, while preserving the existing project aggregation
flow.
Comment on lines
+664
to
+668
| for ticket_id in sorted(included_tickets_set): | ||
| details = ticket_details_map.get(ticket_id) | ||
| if details and details.get('title'): | ||
| if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'): | ||
| all_tickets.append(f"{ticket_id}: {details['issueUrl']}") | ||
| elif details and details.get('title'): |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Unresolved-ticket line breaks the downstream all_tickets parser.
create_monthly_release_ticket.parse_ticket_line matches ^(\S+)\s*:\s*(.+?)\s*:\s*(.+)$ against these strings. For AE-1952: https://linear.app/stackgen/issue/AE-1952 the colon in https: becomes the second delimiter, yielding state="https" and title="//linear.app/stackgen/issue/AE-1952" — which is what lands in the Linear issue table.
Emit the same three-field shape so the contract holds.
🐛 Proposed fix
- if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'):
- all_tickets.append(f"{ticket_id}: {details['issueUrl']}")
+ if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'):
+ all_tickets.append(
+ f"{ticket_id:<{max_ticket_id_len}}: {'Unknown':<{max_status_len}}: "
+ f"(details unavailable)"
+ )📝 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.
Suggested change
| for ticket_id in sorted(included_tickets_set): | |
| details = ticket_details_map.get(ticket_id) | |
| if details and details.get('title'): | |
| if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'): | |
| all_tickets.append(f"{ticket_id}: {details['issueUrl']}") | |
| elif details and details.get('title'): | |
| for ticket_id in sorted(included_tickets_set): | |
| details = ticket_details_map.get(ticket_id) | |
| if details and str(details.get('fetchHttpStatus')) == '400' and details.get('issueUrl'): | |
| all_tickets.append( | |
| f"{ticket_id:<{max_ticket_id_len}}: {'Unknown':<{max_status_len}}: " | |
| f"(details unavailable)" | |
| ) | |
| elif details and details.get('title'): |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@process_all_repos.py` around lines 664 - 668, Update the unresolved-ticket
branch in the loop over sorted included_tickets_set so each all_tickets entry
preserves the parser’s three-field ticket_id: state: title format; do not place
the issue URL directly after the first delimiter because its https colon is
parsed as a field separator. Reuse the existing ticket details to emit an
appropriate state and title while keeping the issue URL available in the
expected field position.
| ``` | ||
|
|
||
| Optional `make setup` copies `env.template` → `.env` for local `LINEAR_API_KEY`. | ||
| Artifacts are written under **`<FROM_REF>-<TO_REF>/`** (for example `v2026.7.3-v2026.7.7/`), not under a shared `generated_files/` folder. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document sanitized tag-pair artifact paths.
The Makefile replaces /, spaces, and : in refs before constructing the directory, so the documentation should use <sanitized FROM_REF>-<sanitized TO_REF> rather than the literal refs.
README.md#L93-L93: update the primary artifact-path description and example guidance.README.md#L299-L299: update troubleshooting to reference the sanitized directory or explain the transformation.
📍 Affects 1 file
README.md#L93-L93(this comment)README.md#L299-L299
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 93, The README currently documents literal ref-based
artifact paths, but the Makefile sanitizes refs by replacing slashes, spaces,
and colons. Update README.md lines 93-93 to describe and exemplify the sanitized
<FROM_REF>-<TO_REF> directory, and update README.md lines 299-299 to reference
that sanitized directory or document the transformation.
Comment on lines
+110
to
+118
| ### Linear issue shape | ||
|
|
||
| | Target | Purpose | | ||
| |--------|---------| | ||
| | `make help` | List targets | | ||
| | `make setup` | `.env` from template + Linear smoke test | | ||
| | `make generate-input STACKGEN_TAG=v2026.3.12` | Production `version.json` + **raw** appcd-dist `.env` at that tag | | ||
| | `make generate-input-custom STACKGEN_TAG=vX.Y.Z` | Interactive **version.json** only; `.env` is always raw at that tag | | ||
| | `make fetch_changes_between_tags_from_input` | Run `process_all_repos.py` on `input.json` | | ||
| | `make full-workflow STACKGEN_TAG=<tag>` | `generate-input` then `fetch_changes...` | | ||
| | `make monthly-release STACKGEN_TAG=vX.Y.Z` | Full pipeline: clean → prod input → tickets → **Linear issue** | | ||
| | `make monthly-release-no-ticket STACKGEN_TAG=vX.Y.Z` | Steps 1–3 only (no Linear create) | | ||
| | `make clean` | Remove `generated_files/` + local `__pycache__` | | ||
| | `make test-linear` | Linear API test | | ||
| | Field | Value | | ||
| |-------|--------| | ||
| | Team | HZ | | ||
| | Title | `[Weekly release] <TO_REF>` or `[Monthly release] <TO_REF>` | | ||
| | Assignee | `gaurav@stackgen.com` | | ||
| | Status | `Todo` | | ||
| | Body | Release Candidate Tags (all services) · AIOS / DPP / CORE tables (linked ID, status, summary) · other teams · Projects (linked) | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe assignee and status as defaults, not fixed values.
ASSIGNEE_QUERY and STATE_NAME are exposed as Make parameters and are passed through to ticket creation. Label gaurav@stackgen.com and Todo as defaults and mention that operators can override them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 110 - 118, Update the “Linear issue shape” table to
describe the Assignee and Status values as defaults rather than fixed
requirements: identify `gaurav@stackgen.com` and `Todo` as the default values,
and state that operators can override them through the exposed `ASSIGNEE_QUERY`
and `STATE_NAME` Make parameters.
Comment on lines
92
to
96
| resolved_ticket_config = MonthlyTicketConfig( | ||
| input_path=config.final_tag_differences_path(root_path), | ||
| release_kind="weekly", | ||
| stackgen_tag=stackgen_tag.strip(), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Monthly entry points default release_kind to weekly. Both the monthly pipeline and its CLI now produce [Weekly release] <tag> titles unless explicitly overridden.
release_pipeline/pipeline.py#L92-L96: setrelease_kind="monthly"in the defaultMonthlyTicketConfigbuilt byrun_monthly_release_pipeline, or confirm weekly is intended.run_monthly_release.py#L96-L101: change the--release-kinddefault tomonthlyto match the script's purpose.
📍 Affects 2 files
release_pipeline/pipeline.py#L92-L96(this comment)run_monthly_release.py#L96-L101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@release_pipeline/pipeline.py` around lines 92 - 96, Set the default release
kind to monthly in the MonthlyTicketConfig created by
run_monthly_release_pipeline in release_pipeline/pipeline.py (lines 92-96), and
change the --release-kind CLI default in run_monthly_release.py (lines 96-101)
to monthly so both monthly entry points generate monthly release titles by
default.
Comment on lines
+836
to
+840
| comment = create_issue_comment(api_key, issue_uuid, comment_body) | ||
| print() | ||
| print("✅ Candidate-build comment added") | ||
| if comment.get("url"): | ||
| print(f"Comment URL: {comment.get('url')}") |
There was a problem hiding this comment.
Logic: create_issue_comment exceptions happen after create_issue succeeds, so the command can fail and trigger duplicate ticket creation on retry. Handle comment failure separately.
Suggested change
| comment = create_issue_comment(api_key, issue_uuid, comment_body) | |
| print() | |
| print("✅ Candidate-build comment added") | |
| if comment.get("url"): | |
| print(f"Comment URL: {comment.get('url')}") | |
| try: | |
| comment = create_issue_comment(api_key, issue_uuid, comment_body) | |
| except Exception as exc: | |
| print( | |
| f"⚠️ Ticket created but failed to add candidate-build comment: {exc}", | |
| file=sys.stderr, | |
| ) | |
| return 0 | |
| print() | |
| print("✅ Candidate-build comment added") | |
| if comment.get("url"): | |
| print(f"Comment URL: {comment.get('url')}") |
abhishes007
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes