feat(feedback): add /gsd-feedback with shared issue-filing pipeline#2371
feat(feedback): add /gsd-feedback with shared issue-filing pipeline#2371ElliotDrel wants to merge 8 commits intogsd-build:mainfrom
Conversation
…peline (gsd-build#2331) Add `/gsd-feedback` for filing bugs, feature requests, and questions with auto-attached diagnostics. Extract issue submission into shared `file-issue.md` workflow used by both `/gsd-feedback` and `/gsd-forensics`: - New `commands/gsd/feedback.md` — slash command entry point - New `workflows/feedback.md` — intake, diagnostics, optional forensics enrichment - New `workflows/file-issue.md` — shared gh submit → URL fallback → raw markdown - Update `workflows/forensics.md` — delegate issue filing to file-issue.md, add auto/custom title prompt with feedback intake cross-call - Add error-hint pointers in review/ship/update workflows - Tests: 44 passing across feedback, file-issue, and forensics suites Closes gsd-build#2331 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Claude
participant FeedbackWF as /gsd-feedback Workflow
participant SDK as gsd-sdk
participant Forensics as forensics (optional)
participant FileIssueWF as file-issue Workflow
participant GH as gh CLI
participant Browser
User->>Claude: /gsd-feedback "description"
Claude->>FeedbackWF: Invoke with $ARGUMENTS
FeedbackWF->>User: Ask type/title/description if needed
User->>FeedbackWF: Provide inputs
FeedbackWF->>SDK: query state.json, state-snapshot, config
SDK-->>FeedbackWF: diagnostics
FeedbackWF->>Forensics: (if bug & requested) run enrichment
Forensics-->>FeedbackWF: INVESTIGATION_FINDINGS
FeedbackWF->>FileIssueWF: Delegate with ISSUE_* and DIAGNOSTICS_MARKDOWN
FileIssueWF->>GH: Attempt gh issue create
alt gh succeeds
GH-->>FileIssueWF: Issue URL
FileIssueWF-->>User: Filed issue link + rendered markdown
else gh fails
FileIssueWF->>FileIssueWF: Build prefilled GitHub URL (encode)
FileIssueWF->>Browser: Open URL (cross-platform)
FileIssueWF-->>User: Prefilled URL + raw markdown fallback
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new /gsd-feedback command and consolidates GitHub issue filing into a shared workflow so both /gsd-feedback and /gsd-forensics use a single, consistent submission pipeline (gh → prefilled URL → raw markdown).
Changes:
- Introduces
/gsd-feedbackcommand plusworkflows/feedback.mdintake (diagnostics + optional forensics enrichment). - Adds shared
workflows/file-issue.mdand updates/gsd-forensicsto delegate issue creation to it (with auto/custom title support). - Improves discoverability by adding
/gsd-feedbackpointers in help/docs and in high-value failure surfaces (review/ship/update).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
commands/gsd/feedback.md |
New slash-command entrypoint delegating to the feedback workflow. |
get-shit-done/workflows/feedback.md |
New guided intake + diagnostics collection + delegation to shared filing workflow. |
get-shit-done/workflows/file-issue.md |
New shared “render + submit + fallback” issue-filing workflow for feedback/forensics. |
get-shit-done/workflows/forensics.md |
Replaces inline issue creation with delegation; adds auto/custom title flow. |
get-shit-done/workflows/help.md |
Adds /gsd-feedback to help listing. |
get-shit-done/workflows/review.md |
Adds error-hint pointer to /gsd-feedback on a key failure path. |
get-shit-done/workflows/ship.md |
Adds error-hint pointer to /gsd-feedback on key preflight failures. |
get-shit-done/workflows/update.md |
Adds error-hint pointer to /gsd-feedback for npm/install failures. |
docs/COMMANDS.md |
Documents /gsd-feedback and notes shared filing path + cross-calling. |
docs/ARCHITECTURE.md |
Updates workflow count to reflect the new workflows. |
README.md |
Adds /gsd-feedback to the command table. |
tests/feedback.test.cjs |
Adds tests asserting command/workflow presence and doc references. |
tests/file-issue.test.cjs |
Adds tests for the shared issue-filing workflow contract and behavior. |
tests/forensics.test.cjs |
Updates forensics tests to assert delegation to file-issue.md and auto/custom title prompt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ARCHITECTURE.md (1)
116-117:⚠️ Potential issue | 🟠 MajorUpdate stale inventory counts in this doc to unblock CI.
Total commandsand the installation tree counts are out of sync with the repository, and this is currently failing the test pipeline.🛠️ Proposed doc fix
-**Total commands:** 75 +**Total commands:** 76 @@ -├── commands/gsd/*.md # 75 slash commands +├── commands/gsd/*.md # 76 slash commands @@ -│ ├── workflows/*.md # 72 workflow definitions +│ ├── workflows/*.md # 74 workflow definitionsAlso applies to: 412-417
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ARCHITECTURE.md` around lines 116 - 117, The docs show stale counts—update the "Total commands: 75" entry and any installation tree counts in ARCHITECTURE.md (including the other occurrences around the installation tree section) to reflect the current repository state so CI parity is restored; locate the literal "Total commands:" line and the installation tree count lines and replace the numeric values with the accurate counts derived from the current codebase (recount commands and installation entries) and ensure formatting matches the existing doc style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@get-shit-done/workflows/feedback.md`:
- Around line 127-137: The template currently prints full STATE_JSON and
STATE_SNAPSHOT which can leak sensitive/local data; change the block to generate
and embed redacted-and-truncated versions instead: replace references to
STATE_JSON and STATE_SNAPSHOT with STATE_JSON_REDACTED_TRUNCATED and
STATE_SNAPSHOT_REDACTED_TRUNCATED, update headings to "Redacted State Snapshot
(truncated)" and similar, and ensure the redaction/truncation logic removes
known sensitive keys (e.g., paths, session tokens, secrets) and enforces a max
payload size before formatting into the markdown.
- Around line 35-36: Update the ambiguous instruction in step 2 to explicitly
select the most recently generated forensics report instead of a placeholder
path; change the text referencing `.planning/forensics/report-{timestamp}.md` to
something like "Read the latest generated report from `.planning/forensics/`
(e.g., the most recent `report-*.md` by timestamp)" and clarify in step 3 that
`INVESTIGATION_FINDINGS` should be set to the contents of that latest report;
ensure the wording around "latest generated report" replaces the placeholder so
automation or manual steps consistently pick the correct file.
- Around line 57-63: The version-discovery for loop that iterates over
"$PREFERRED_CONFIG_DIR/get-shit-done/VERSION",
"$HOME/.claude/get-shit-done/VERSION", "$HOME/.codex/get-shit-done/VERSION",
".claude/get-shit-done/VERSION", ".codex/get-shit-done/VERSION" misses other
supported config locations; update the loop to include the same candidates used
by hooks/gsd-check-update.js — add checks for
"$CLAUDE_CONFIG_DIR/get-shit-done/VERSION" (if CLAUDE_CONFIG_DIR is set),
"$HOME/.gemini/get-shit-done/VERSION",
"$HOME/.config/kilo/get-shit-done/VERSION", "$HOME/.kilo/get-shit-done/VERSION",
"$HOME/.config/opencode/get-shit-done/VERSION", and
"$HOME/.opencode/get-shit-done/VERSION" (and the relative-dot variants
".gemini/...", ".config/kilo/...", ".kilo/...", ".config/opencode/...",
".opencode/...") so the for loop and its candidates match gsd-check-update.js
and properly discover the VERSION file.
In `@get-shit-done/workflows/file-issue.md`:
- Around line 29-36: Validate that ISSUE_TYPE is one of the allowed enum values
before proceeding: in the input validation step that currently checks presence
of ISSUE_TYPE/ISSUE_TITLE/ISSUE_DESCRIPTION, add an explicit check that
ISSUE_TYPE is one of ["bug","feature","question"] and abort with a clear error
message if not; then in the render_issue_body mapping logic (the step named
render_issue_body) handle only those cases (bug -> bug, feature -> enhancement,
question -> question) and either produce a safe default or raise an error for
unknown types so invalid values cannot slip through.
---
Outside diff comments:
In `@docs/ARCHITECTURE.md`:
- Around line 116-117: The docs show stale counts—update the "Total commands:
75" entry and any installation tree counts in ARCHITECTURE.md (including the
other occurrences around the installation tree section) to reflect the current
repository state so CI parity is restored; locate the literal "Total commands:"
line and the installation tree count lines and replace the numeric values with
the accurate counts derived from the current codebase (recount commands and
installation entries) and ensure formatting matches the existing doc style.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69d0b73f-b3d2-438d-bbb8-7ac114947fdc
📒 Files selected for processing (14)
README.mdcommands/gsd/feedback.mddocs/ARCHITECTURE.mddocs/COMMANDS.mdget-shit-done/workflows/feedback.mdget-shit-done/workflows/file-issue.mdget-shit-done/workflows/forensics.mdget-shit-done/workflows/help.mdget-shit-done/workflows/review.mdget-shit-done/workflows/ship.mdget-shit-done/workflows/update.mdtests/feedback.test.cjstests/file-issue.test.cjstests/forensics.test.cjs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
get-shit-done/workflows/file-issue.md (1)
29-36:⚠️ Potential issue | 🟡 MinorAdd explicit allowlist validation for
ISSUE_TYPE.The current validation text checks required presence, but not that
ISSUE_TYPEis strictly one ofbug|feature|question. Please fail fast on invalid values before label/body mapping.Suggested patch
-Validate required inputs. If `ISSUE_TYPE`, `ISSUE_TITLE`, or `ISSUE_DESCRIPTION` is missing, abort with a clear error — callers must satisfy the contract. +Validate required inputs. If `ISSUE_TYPE`, `ISSUE_TITLE`, or `ISSUE_DESCRIPTION` is missing, abort with a clear error — callers must satisfy the contract. +Also validate `ISSUE_TYPE` is exactly one of: `bug`, `feature`, `question`; otherwise abort with a clear error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/file-issue.md` around lines 29 - 36, Add an explicit allowlist check for ISSUE_TYPE before the label/body mapping in the validation step: after verifying presence of ISSUE_TYPE, ISSUE_TITLE, and ISSUE_DESCRIPTION, ensure ISSUE_TYPE strictly matches one of "bug", "feature", or "question" and abort with a clear error message if not; update the validation logic referenced in the initial validation step (the required inputs check) so it fails fast on invalid ISSUE_TYPE values and prevents reaching the render_issue_body mapping logic.get-shit-done/workflows/feedback.md (3)
127-137:⚠️ Potential issue | 🟠 MajorAvoid embedding raw state payloads directly in issue diagnostics.
Unredacted
STATE_JSONandSTATE_SNAPSHOTcan leak local/sensitive metadata and unnecessarily bloat issue bodies.Suggested patch
-### Raw State JSON +### Redacted State JSON (truncated) ```json -{STATE_JSON} +{STATE_JSON_REDACTED_TRUNCATED}-### Raw State Snapshot
+### Redacted State Snapshot (truncated)-{STATE_SNAPSHOT} +{STATE_SNAPSHOT_REDACTED_TRUNCATED}</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@get-shit-done/workflows/feedback.mdaround lines 127 - 137, The template
currently embeds raw STATE_JSON and STATE_SNAPSHOT into issue bodies, which
risks leaking sensitive/local metadata and bloating output; update the markers
used in the feedback.md snippet to emit redacted/truncated placeholders
instead—replace {STATE_JSON} with {STATE_JSON_REDACTED_TRUNCATED} and rename the
heading and {STATE_SNAPSHOT} insertion to a redacted version (e.g., "Redacted
State Snapshot (truncated)" and {STATE_SNAPSHOT_REDACTED_TRUNCATED}) so only
truncated/redacted payloads are output while keeping the original placeholders
(STATE_JSON, STATE_SNAPSHOT) easy to find for future changes.</details> --- `57-63`: _⚠️ Potential issue_ | _🟠 Major_ **Expand `GSD_VERSION` candidate paths to match all supported config locations.** Current candidates are limited and can produce empty/incorrect version detection in supported runtimes. <details> <summary>Suggested patch</summary> ```diff for candidate in \ + "${CLAUDE_CONFIG_DIR:-}/get-shit-done/VERSION" \ "$PREFERRED_CONFIG_DIR/get-shit-done/VERSION" \ "$HOME/.claude/get-shit-done/VERSION" \ + "$HOME/.gemini/get-shit-done/VERSION" \ + "$HOME/.config/kilo/get-shit-done/VERSION" \ + "$HOME/.kilo/get-shit-done/VERSION" \ + "$HOME/.config/opencode/get-shit-done/VERSION" \ + "$HOME/.opencode/get-shit-done/VERSION" \ "$HOME/.codex/get-shit-done/VERSION" \ ".claude/get-shit-done/VERSION" \ + ".gemini/get-shit-done/VERSION" \ + ".config/kilo/get-shit-done/VERSION" \ + ".kilo/get-shit-done/VERSION" \ + ".config/opencode/get-shit-done/VERSION" \ + ".opencode/get-shit-done/VERSION" \ ".codex/get-shit-done/VERSION" do
35-36:⚠️ Potential issue | 🟠 MajorUse deterministic “latest report” selection instead of a timestamp placeholder path.
.planning/forensics/report-{timestamp}.mdis ambiguous and can attach the wrong report if multiple files exist.Suggested patch
-2. Read the generated report from `.planning/forensics/report-{timestamp}.md`. +2. Read the most recently generated report from `.planning/forensics/` (latest `report-*.md` by timestamp).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/feedback.md` around lines 35 - 36, Replace the ambiguous hardcoded path `.planning/forensics/report-{timestamp}.md` with deterministic “latest report” selection: glob `.planning/forensics/report-*.md`, pick the most recent file (e.g., by mtime or lexicographic sort if timestamps are encoded), read and redact its contents, then set INVESTIGATION_FINDINGS to that redacted content; update the step that reads the report to use this selection logic so it cannot attach the wrong report when multiple files exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@get-shit-done/workflows/forensics.md`:
- Around line 252-254: The blockquote in the forensics.md workflow contains a
blank line breaking continuity; locate the quoted prompt sequence containing the
lines starting with "Suggest a title for this issue (or press Enter to use the
auto-derived one): {auto title}" and "Add a description (or press Enter to use
the investigation summary):" and remove the empty line between them (or add a
leading ">" to that blank line) so the two quoted lines are contiguous and
satisfy markdownlint MD028.
---
Duplicate comments:
In `@get-shit-done/workflows/feedback.md`:
- Around line 127-137: The template currently embeds raw STATE_JSON and
STATE_SNAPSHOT into issue bodies, which risks leaking sensitive/local metadata
and bloating output; update the markers used in the feedback.md snippet to emit
redacted/truncated placeholders instead—replace {STATE_JSON} with
{STATE_JSON_REDACTED_TRUNCATED} and rename the heading and {STATE_SNAPSHOT}
insertion to a redacted version (e.g., "Redacted State Snapshot (truncated)" and
{STATE_SNAPSHOT_REDACTED_TRUNCATED}) so only truncated/redacted payloads are
output while keeping the original placeholders (STATE_JSON, STATE_SNAPSHOT) easy
to find for future changes.
- Around line 35-36: Replace the ambiguous hardcoded path
`.planning/forensics/report-{timestamp}.md` with deterministic “latest report”
selection: glob `.planning/forensics/report-*.md`, pick the most recent file
(e.g., by mtime or lexicographic sort if timestamps are encoded), read and
redact its contents, then set INVESTIGATION_FINDINGS to that redacted content;
update the step that reads the report to use this selection logic so it cannot
attach the wrong report when multiple files exist.
In `@get-shit-done/workflows/file-issue.md`:
- Around line 29-36: Add an explicit allowlist check for ISSUE_TYPE before the
label/body mapping in the validation step: after verifying presence of
ISSUE_TYPE, ISSUE_TITLE, and ISSUE_DESCRIPTION, ensure ISSUE_TYPE strictly
matches one of "bug", "feature", or "question" and abort with a clear error
message if not; update the validation logic referenced in the initial validation
step (the required inputs check) so it fails fast on invalid ISSUE_TYPE values
and prevents reaching the render_issue_body mapping logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ce2a26d-8d0e-42e3-8122-c5f4579cb423
📒 Files selected for processing (3)
get-shit-done/workflows/feedback.mdget-shit-done/workflows/file-issue.mdget-shit-done/workflows/forensics.md
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
get-shit-done/workflows/feedback.md (2)
35-36: Specify the report selection mechanism.Line 35 says "most recently generated report" but doesn't specify how to identify it. Given the timestamp format
report-YYYYMMDD-HHMMSS.mdfrom context, lexicographic sorting works, but explicit implementation guidance would help:📋 Suggested implementation guidance
# Find most recent forensics report LATEST_REPORT=$(ls -t .planning/forensics/report-*.md 2>/dev/null | head -n1) if [ -z "$LATEST_REPORT" ]; then echo "Warning: Forensics step completed but no report found" INVESTIGATION_FINDINGS="" else INVESTIGATION_FINDINGS=$(cat "$LATEST_REPORT") fiAdd this as an implementation note in step 2 or step 3 of the forensics enrichment section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/feedback.md` around lines 35 - 36, Clarify and implement how to choose the "most recently generated report": search for files matching report-*.md (timestamp format report-YYYYMMDD-HHMMSS.md), pick the newest (e.g., by lexicographic/time sort such as ls -t or equivalent to get the first match into a variable like LATEST_REPORT), if none exists emit a warning and set INVESTIGATION_FINDINGS="" otherwise read the file contents and assign them to INVESTIGATION_FINDINGS; ensure this behavior is documented in the step text referencing the report-*.md pattern and the INVESTIGATION_FINDINGS variable.
1-166: Character encoding issues throughout the file.Multiple lines contain corrupted UTF-8 characters (em-dashes rendered as "â€"", arrows as "â†'", etc.) on lines 34, 148-153, and 163. These likely occurred during copy-paste or editor encoding conversion. While not breaking functionality, they reduce readability and should be standardized to ASCII equivalents.
🔤 Global search-and-replace suggestions
# Replace corrupted em-dash with hyphen sed -i 's/â€"/—/g; s/—/-/g' workflows/feedback.md # Replace corrupted arrow with ASCII arrow sed -i 's/â†'/→/g; s/→/->/g' workflows/feedback.mdOr manually replace:
â€"→-â†'→->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/feedback.md` around lines 1 - 166, The file contains corrupted UTF-8 sequences (e.g. â€", â†') in step names and content (notably inside the "collect_feedback" optional forensics prompt text and the "gather_diagnostics"/"render_diagnostics" sections) which hurts readability; fix by replacing those sequences with ASCII equivalents (use hyphen "-" for em-dash and "->" for arrows) throughout the workflow text, updating occurrences inside the "collect_feedback" step prompt, the forensics step list, and the diagnostics rendering block (references: step names collect_feedback, gather_diagnostics, render_diagnostics, file_issue) so the doc uses plain ASCII dashes and arrows consistently.get-shit-done/workflows/file-issue.md (4)
64-64: SpecifyISSUE_BODY_FILEcreation and error handling.Line 64 writes to
ISSUE_BODY_FILEbut doesn't define where/how this temp file is created. The workflow should either create it explicitly (e.g.,ISSUE_BODY_FILE=$(mktemp)) or document that callers must set it. Additionally, verify the write succeeded before proceeding to submission steps.📝 Suggested addition before line 64
ISSUE_BODY_FILE=$(mktemp) || { echo "Failed to create temp file"; exit 1; }And after the write:
[ -s "$ISSUE_BODY_FILE" ] || { echo "Failed to write issue body"; exit 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/file-issue.md` at line 64, The workflow writes to ISSUE_BODY_FILE but never creates or validates it; update the script around ISSUE_BODY_FILE to create a temp file (e.g., via mktemp) and fail if creation fails, then after writing the rendered body check that ISSUE_BODY_FILE exists and is non-empty (or writable) and exit with an error if the write failed so subsequent commands that use ISSUE_BODY_FILE (gh issue create and the URL fallback) won't proceed with an empty or missing file.
15-15: Clarify whetherDIAGNOSTICS_MARKDOWNis truly required.Line 15 states it's "required", yet line 26 provides a default of
"(none)". This creates ambiguity: should callers always explicitly set this, or is the placeholder acceptable?If callers must provide diagnostics, the contract should say "required; must not be
(none)" and validation should enforce it. If the placeholder is intentional for cases where diagnostics are unavailable, update line 15 to say "required; use(none)when unavailable."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/file-issue.md` at line 15, Clarify the contract for DIAGNOSTICS_MARKDOWN: decide whether callers must always supply real diagnostics or may pass the placeholder "(none)"; then update the docs and validation accordingly — either change the description of DIAGNOSTICS_MARKDOWN to "required; use \"(none)\" when unavailable" if the placeholder is acceptable, or change it to "required; must not be \"(none)\"" and add validation that rejects "(none)". Ensure the updated wording and any validation reference the DIAGNOSTICS_MARKDOWN symbol so callers and runtime checks are consistent.
99-102: Validate URL length and handle file read errors.GitHub has URL length limits (typically ~8KB). Large issue bodies will produce URLs that fail silently when opened. Additionally, line 100 doesn't handle file read errors if
ISSUE_BODY_FILEis missing or unreadable.🛡️ Suggested safety checks
# Verify file exists before reading [ -f "$ISSUE_BODY_FILE" ] || { echo "Error: Issue body file not found"; exit 1; } TITLE_ENC=$(node -e "console.log(encodeURIComponent(process.argv[1]))" "$ISSUE_TITLE") BODY_ENC=$(node -e "console.log(encodeURIComponent(require('fs').readFileSync(process.argv[1], 'utf8')))" "$ISSUE_BODY_FILE" 2>/dev/null) [ -n "$BODY_ENC" ] || { echo "Error: Failed to encode issue body"; exit 1; } LABEL_ENC=$(node -e "console.log(encodeURIComponent(process.argv[1]))" "{mapped label from ISSUE_TYPE}") PREFILLED_URL="https://github.com/${REPO}/issues/new?title=${TITLE_ENC}&body=${BODY_ENC}&labels=${LABEL_ENC}" # Warn if URL is too long (GitHub limit ~8KB, leave margin) if [ ${`#PREFILLED_URL`} -gt 7000 ]; then echo "Warning: Prefilled URL may exceed GitHub's length limit. Manual fallback recommended." fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/file-issue.md` around lines 99 - 102, The prefilled GitHub URL generation (variables TITLE_ENC, BODY_ENC, LABEL_ENC, PREFILLED_URL) does not validate that ISSUE_BODY_FILE exists/is readable and does not guard against URL length limits; add a check that ISSUE_BODY_FILE exists and is readable before computing BODY_ENC, handle/read errors from the node encoding step and fail early with a clear error message if encoding fails, and compute the final PREFILLED_URL length and emit a warning or fallback if it exceeds a safe threshold (e.g., ~7000 characters) so you avoid silently opening an unusable URL.
85-92: Add error handling and quote$LABEL_FLAGexpansion.Line 88 expands
$LABEL_FLAGunquoted, which can break if the label name contains spaces or special characters (though GitHub typically restricts these). More critically, the step states "If this succeeds, capture the returned issue URL and skip tofinal_output" but doesn't show the actual capture logic or conditional branching.🔧 Suggested improvements
# Capture output and check exit status if ISSUE_URL=$(gh issue create \ --repo "$REPO" \ --title "$ISSUE_TITLE" \ ${LABEL_FLAG:+--label "$EXISTING_LABEL"} \ --body-file "$ISSUE_BODY_FILE" 2>&1); then # Success - skip to final_output with ISSUE_URL set SUBMISSION_MODE="gh_success" else # Fall through to prefilled URL fallback SUBMISSION_MODE="url_fallback" fiNote: Using
${LABEL_FLAG:+--label "$EXISTING_LABEL"}is more robust than concatenating strings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@get-shit-done/workflows/file-issue.md` around lines 85 - 92, The gh issue create invocation mishandles LABEL_FLAG (unquoted expansion) and lacks capture/branching for success vs fallback; update the gh issue create call to capture its output into ISSUE_URL and test its exit status (e.g., if ISSUE_URL=$(gh issue create ...); then ...) so success sets SUBMISSION_MODE="gh_success" and failure sets SUBMISSION_MODE="url_fallback", and replace the raw LABEL_FLAG expansion with a safe conditional expansion (e.g., ${LABEL_FLAG:+--label "$EXISTING_LABEL"} or similar) so label values are quoted when provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@get-shit-done/workflows/feedback.md`:
- Around line 137-141: STATE_SNAPSHOT is collected but STATE_SNAPSHOT_REDACTED
is never produced; add a redaction step (in the gather_diagnostics or
render_diagnostics function/block, before STATE_SNAPSHOT_REDACTED is referenced)
that takes STATE_SNAPSHOT, uses jq to: replace absolute home paths with "~"
(gsub(env.HOME;"~")), delete sensitive keys like .session.auth_token,
.session.api_keys, .secrets (del(...)), and then truncate the output to a safe
size (e.g., head -c 2048) and append an ellipsis marker if truncated, assigning
the result to STATE_SNAPSHOT_REDACTED so the workflow emits the sanitized
snapshot.
- Line 34: Replace the corrupted UTF-8 characters in the string "Steps 2–4
from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence →
Detect Anomalies → Generate Report)" so it reads with proper punctuation, e.g.
"Steps 2–4 from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather
Evidence → Detect Anomalies → Generate Report)" or use ASCII "Steps 2-4" and
"->" if preferred; update the exact text occurrence in feedback.md that contains
"Steps 2–4" and "→" (search for those sequences) to the corrected
characters.
- Line 148: The markdown variable list uses an em-dash character instead of a
standard hyphen; locate the line containing `ISSUE_TYPE` and replace the "—"
(em-dash) with a "-" (hyphen), and apply the same replacement for the other
variable lines referenced in the comment (lines following `ISSUE_TYPE`, i.e.,
the variable list entries) so all list items use a standard hyphen for
consistent encoding.
---
Nitpick comments:
In `@get-shit-done/workflows/feedback.md`:
- Around line 35-36: Clarify and implement how to choose the "most recently
generated report": search for files matching report-*.md (timestamp format
report-YYYYMMDD-HHMMSS.md), pick the newest (e.g., by lexicographic/time sort
such as ls -t or equivalent to get the first match into a variable like
LATEST_REPORT), if none exists emit a warning and set INVESTIGATION_FINDINGS=""
otherwise read the file contents and assign them to INVESTIGATION_FINDINGS;
ensure this behavior is documented in the step text referencing the report-*.md
pattern and the INVESTIGATION_FINDINGS variable.
- Around line 1-166: The file contains corrupted UTF-8 sequences (e.g. â€", â†')
in step names and content (notably inside the "collect_feedback" optional
forensics prompt text and the "gather_diagnostics"/"render_diagnostics"
sections) which hurts readability; fix by replacing those sequences with ASCII
equivalents (use hyphen "-" for em-dash and "->" for arrows) throughout the
workflow text, updating occurrences inside the "collect_feedback" step prompt,
the forensics step list, and the diagnostics rendering block (references: step
names collect_feedback, gather_diagnostics, render_diagnostics, file_issue) so
the doc uses plain ASCII dashes and arrows consistently.
In `@get-shit-done/workflows/file-issue.md`:
- Line 64: The workflow writes to ISSUE_BODY_FILE but never creates or validates
it; update the script around ISSUE_BODY_FILE to create a temp file (e.g., via
mktemp) and fail if creation fails, then after writing the rendered body check
that ISSUE_BODY_FILE exists and is non-empty (or writable) and exit with an
error if the write failed so subsequent commands that use ISSUE_BODY_FILE (gh
issue create and the URL fallback) won't proceed with an empty or missing file.
- Line 15: Clarify the contract for DIAGNOSTICS_MARKDOWN: decide whether callers
must always supply real diagnostics or may pass the placeholder "(none)"; then
update the docs and validation accordingly — either change the description of
DIAGNOSTICS_MARKDOWN to "required; use \"(none)\" when unavailable" if the
placeholder is acceptable, or change it to "required; must not be \"(none)\""
and add validation that rejects "(none)". Ensure the updated wording and any
validation reference the DIAGNOSTICS_MARKDOWN symbol so callers and runtime
checks are consistent.
- Around line 99-102: The prefilled GitHub URL generation (variables TITLE_ENC,
BODY_ENC, LABEL_ENC, PREFILLED_URL) does not validate that ISSUE_BODY_FILE
exists/is readable and does not guard against URL length limits; add a check
that ISSUE_BODY_FILE exists and is readable before computing BODY_ENC,
handle/read errors from the node encoding step and fail early with a clear error
message if encoding fails, and compute the final PREFILLED_URL length and emit a
warning or fallback if it exceeds a safe threshold (e.g., ~7000 characters) so
you avoid silently opening an unusable URL.
- Around line 85-92: The gh issue create invocation mishandles LABEL_FLAG
(unquoted expansion) and lacks capture/branching for success vs fallback; update
the gh issue create call to capture its output into ISSUE_URL and test its exit
status (e.g., if ISSUE_URL=$(gh issue create ...); then ...) so success sets
SUBMISSION_MODE="gh_success" and failure sets SUBMISSION_MODE="url_fallback",
and replace the raw LABEL_FLAG expansion with a safe conditional expansion
(e.g., ${LABEL_FLAG:+--label "$EXISTING_LABEL"} or similar) so label values are
quoted when provided.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 22d83d0d-fdad-4e4a-a702-0cd5daad48f0
📒 Files selected for processing (2)
get-shit-done/workflows/feedback.mdget-shit-done/workflows/file-issue.md
| > "Want me to run a quick forensics investigation first? This checks git history, planning state, and artifacts for anomalies that might explain the bug. [Y/n]" | ||
|
|
||
| If yes: | ||
| 1. Execute Steps 2–4 from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence → Detect Anomalies → Generate Report). Skip Steps 5–8 (presentation, interactive investigation, issue creation, STATE update — those are handled here). |
There was a problem hiding this comment.
Fix character encoding issues in step reference.
Line 34 contains corrupted characters: "Steps 2â€"4" (should be "Steps 2–4" or "Steps 2-4") and "â†'" (should be "→"). These appear to be UTF-8 encoding issues with en-dash and arrow characters.
🔧 Proposed fix
-1. Execute Steps 2â€"4 from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence â†' Detect Anomalies â†' Generate Report). Skip Steps 5â€"8 (presentation, interactive investigation, issue creation, STATE update â€" those are handled here).
+1. Execute Steps 2-4 from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence → Detect Anomalies → Generate Report). Skip Steps 5-8 (presentation, interactive investigation, issue creation, STATE update — those are handled here).📝 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.
| 1. Execute Steps 2–4 from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence → Detect Anomalies → Generate Report). Skip Steps 5–8 (presentation, interactive investigation, issue creation, STATE update — those are handled here). | |
| 1. Execute Steps 2-4 from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence → Detect Anomalies → Generate Report). Skip Steps 5-8 (presentation, interactive investigation, issue creation, STATE update — those are handled here). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@get-shit-done/workflows/feedback.md` at line 34, Replace the corrupted UTF-8
characters in the string "Steps 2–4 from
`@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence → Detect
Anomalies → Generate Report)" so it reads with proper punctuation, e.g. "Steps
2–4 from `@~/.claude/get-shit-done/workflows/forensics.md` (Gather Evidence →
Detect Anomalies → Generate Report)" or use ASCII "Steps 2-4" and "->" if
preferred; update the exact text occurrence in feedback.md that contains "Steps
2–4" and "→" (search for those sequences) to the corrected characters.
| ### State Snapshot (redacted) | ||
|
|
||
| ```json | ||
| {STATE_SNAPSHOT_REDACTED} | ||
| ``` |
There was a problem hiding this comment.
Implement the redaction logic for STATE_SNAPSHOT_REDACTED.
Line 140 uses STATE_SNAPSHOT_REDACTED but the workflow never defines how to produce it from STATE_SNAPSHOT (collected at line 94). The past review comment requested redaction of sensitive keys (paths, tokens, secrets) and truncation to cap payload size, but no implementation guidance is provided.
🔧 Suggested implementation before line 117
# Redact sensitive fields from state snapshot
STATE_SNAPSHOT_REDACTED=$(printf '%s' "$STATE_SNAPSHOT" | jq '
# Redact absolute paths - replace home directory
walk(if type == "string" then gsub(env.HOME; "~") else . end)
# Remove potentially sensitive keys
| del(.session.auth_token, .session.api_keys, .secrets)
# Truncate to reasonable size (keep first 50 keys/2KB)
' 2>/dev/null | head -c 2048)
# Add ellipsis if truncated
if [ ${`#STATE_SNAPSHOT_REDACTED`} -eq 2048 ]; then
STATE_SNAPSHOT_REDACTED="${STATE_SNAPSHOT_REDACTED}... [truncated]"
fiAdd this to the gather_diagnostics or render_diagnostics step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@get-shit-done/workflows/feedback.md` around lines 137 - 141, STATE_SNAPSHOT
is collected but STATE_SNAPSHOT_REDACTED is never produced; add a redaction step
(in the gather_diagnostics or render_diagnostics function/block, before
STATE_SNAPSHOT_REDACTED is referenced) that takes STATE_SNAPSHOT, uses jq to:
replace absolute home paths with "~" (gsub(env.HOME;"~")), delete sensitive keys
like .session.auth_token, .session.api_keys, .secrets (del(...)), and then
truncate the output to a safe size (e.g., head -c 2048) and append an ellipsis
marker if truncated, assigning the result to STATE_SNAPSHOT_REDACTED so the
workflow emits the sanitized snapshot.
| <step name="file_issue"> | ||
| Set the variables required by the shared issue-filing workflow, then delegate: | ||
|
|
||
| - `ISSUE_TYPE` — from step 1 |
There was a problem hiding this comment.
Fix character encoding in variable list comment.
Line 148 contains "—" (em-dash) which should be "-" (hyphen) for consistency. Same encoding issue as line 34.
🔧 Proposed fix
-- `ISSUE_TYPE` â€" from step 1
+- `ISSUE_TYPE` - from step 1Apply similar fixes to lines 149-153.
📝 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.
| - `ISSUE_TYPE` — from step 1 | |
| - `ISSUE_TYPE` - from step 1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@get-shit-done/workflows/feedback.md` at line 148, The markdown variable list
uses an em-dash character instead of a standard hyphen; locate the line
containing `ISSUE_TYPE` and replace the "—" (em-dash) with a "-" (hyphen), and
apply the same replacement for the other variable lines referenced in the
comment (lines following `ISSUE_TYPE`, i.e., the variable list entries) so all
list items use a standard hyphen for consistent encoding.
trek-e
left a comment
trek-e
left a comment
There was a problem hiding this comment.
Adversarial review — REQUEST CHANGES.
Critical: Issue #2331 is not approved.
Issue #2331 carries feature-request and status: waiting-for-user labels. It does not have approved-feature or approved-enhancement. Per CONTRIBUTING.md, contributors must wait for approved-feature before writing code. This PR was submitted against an unapproved issue.
What needs to happen:
- A maintainer needs to approve issue #2331 (add
approved-featurelabel and explicitly approve) - Once approved, this PR can be reconsidered
Technical findings (for reference when re-review occurs after approval):
-
gsd-sdk query state.jsonandgsd-sdk query state-snapshotinfeedback.md—state-snapshotdoes not appear in the query handler registry in PR #2341. If this PR merges before the handler exists, the diagnostic step silently falls back to{}which is an acceptable degradation, but the documentation implies the field is populated. Acceptable if documented as graceful fallback. -
Forensics enrichment step references
Steps 2–4of forensics.md — tight coupling to forensics step numbering. If forensics.md steps are renumbered, this silently breaks. A named step reference (gather_evidence,detect_anomalies,generate_report) would be more robust, but this is a style concern, not a correctness blocker. -
44 tests pass — the test coverage for the shared
file-issue.mdworkflow is thorough (gh path, URL fallback, label existence check). No gaps found. -
No security issues — no prompt injection vectors, no path traversal, no credential exposure in diagnostics (state snapshot explicitly redacted).
This PR represents solid work. The primary blocker is the missing approval on #2331, not the implementation quality.
|
CodeRabbit finding audit: all four findings are false positives against the current branch. Finding 1 (raw state embedding): False positive. The current feedback.md uses STATE_SNAPSHOT_REDACTED, not raw STATE_JSON. Already addressed. Finding 2 (VERSION candidate paths): False positive. gather_diagnostics already includes CLAUDE_CONFIG_DIR, Gemini, Kilo, and OpenCode paths. Already addressed. Finding 3 (forensics report path): False positive. The text already says 'Read the most recently generated report from .planning/forensics/report-*.md'. Already addressed. Finding 4 (ISSUE_TYPE allowlist): False positive. file-issue.md already includes the validation that ISSUE_TYPE must be one of bug, feature, or question. Already addressed. One valid observation not flagged by CodeRabbit: the branch has corrupted UTF-8 byte sequences in step names (em-dashes rendered as a-with-circumflex). These came from copy-paste and affect readability. Worth a cleanup commit. |
trek-e
added
needs merge fixes
Summary
/gsd-feedbackcommand for filing bugs, feature requests, and questions with auto-attached diagnosticsworkflows/file-issue.mdused by both/gsd-feedbackand/gsd-forensics/gsd-forensicsStep 7 now delegates tofile-issue.mdinstead of inlinegh issue create— gains URL fallback and raw markdown fallback it previously lackedCloses #2331
What changed
commands/gsd/feedback.mdworkflows/feedback.mdworkflows/file-issue.mdworkflows/forensics.mdworkflows/review.md/gsd-feedbackworkflows/ship.md/gsd-feedbackworkflows/update.md/gsd-feedbackdocs/COMMANDS.md/gsd-feedbackentry + cross-notes on both commandsdocs/ARCHITECTURE.mdREADME.mdworkflows/help.mdAddresses feedback from trek-e on #2331
Trek-e noted
/gsd-forensicsalready covers post-failure bug filing. This PR makes the two commands complementary rather than duplicative:file-issue.md) — no more divergent issue-filing logicTest plan
node --test tests/feedback.test.cjs tests/file-issue.test.cjs tests/forensics.test.cjs— 44/44 passing/gsd-feedbackproactive bug flow with forensics enrichment/gsd-feedbackfeature request (no forensics offered)/gsd-forensics→ file issue with auto-derived title/gsd-forensics→ file issue with custom title via feedback intake🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
New Workflows
Documentation
Tests