feat(riffrec-feedback-analysis): add Riffrec feedback skill with three-path routing#747
feat(riffrec-feedback-analysis): add Riffrec feedback skill with three-path routing#747kieranklaassen wants to merge 3 commits intomainfrom
Conversation
…e-path routing Convert Riffrec recordings, videos, audio, or notes into structured feedback. Routes to one of three references at trigger time so the loaded payload stays small: - Setup: install Riffrec in a project (links to kieranklaassen/riffrec). - Quick bug report: short recordings or single issues — analyzer to temp dir, one concise inline bug report, no docs/brainstorms writes, no ce-brainstorm handoff. Auto-escalates if the recording turns out to contain multiple issues. - Extensive analysis: the full pipeline — analysis.md, problem-analysis, source-materials, requirements-kickoff, source mapping, and automatic handoff into ce-brainstorm. Skill content adapted from the personal ~/.codex copy: absolute paths replaced with relative ones, ce-brainstorm referenced as a bare skill per plugin convention, and the Riffrec capture tool surfaced in SKILL.md, the install reference, and the README row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4795dc4ea9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| resolved = member_path.resolve() | ||
| if not str(resolved).startswith(str(dest.resolve())): | ||
| raise RuntimeError(f"Unsafe zip member path: {member.filename}") |
There was a problem hiding this comment.
The zip extraction guard uses str(resolved).startswith(str(dest.resolve())), which is bypassable by sibling-prefixed paths (for example, a member resolving to /tmp/raw-evil/... still starts with /tmp/raw). In this skill, a crafted riffrec-*.zip can therefore write outside raw/, potentially overwriting arbitrary files in writable directories. Use a boundary-safe check such as os.path.commonpath([dest_resolved, resolved]) == str(dest_resolved) (or Path.is_relative_to on Python 3.9+) before writing each member.
Useful? React with 👍 / 👎.
| return {"status": "failed", "text": "", "reason": "ffmpeg is not installed; cannot chunk media"} | ||
|
|
||
| chunks_dir.mkdir(parents=True, exist_ok=True) | ||
| chunk_count = max(1, int((duration or chunk_seconds) // chunk_seconds) + (1 if duration % chunk_seconds else 0)) |
There was a problem hiding this comment.
When a full transcription fails and chunk retry is triggered, chunk_count becomes 1 whenever duration is 0/unknown, so only the first chunk is transcribed and the rest of long media is silently dropped. This happens in environments where duration probing fails (e.g., missing/failed ffprobe) and yields incomplete analysis output that appears successful. The chunking loop should continue extracting chunks until ffmpeg indicates EOF (or probe duration another way) instead of assuming one chunk when duration is unavailable.
Useful? React with 👍 / 👎.
…ew skills/agents Rename the skill from `riffrec-feedback-analysis` to `ce-riffrec-feedback-analysis` to match the plugin's naming convention. The directory, SKILL.md `name:` frontmatter, and README row all carry the prefix. Make the rule mechanically enforced so this mistake doesn't recur: - tests/frontmatter.test.ts: new tests assert every compound-engineering skill directory and agent file starts with `ce-`. A small allowlist pins the three legacy unprefixed skills (every-style-editor, file-todos, lfg) -- new skills cannot be added there. - plugins/compound-engineering/AGENTS.md: strengthen the Naming Convention section to call the prefix mandatory, point at the test, and forbid extending the allowlist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Capture the convention-with-visible-exceptions failure mode that let PR #747 ship a non-prefixed skill, and the fix: pair the prose rule with a frontmatter test plus an explicit legacy allowlist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb211bdebc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| All non-setup paths share the same analyzer: | ||
|
|
||
| ```bash | ||
| python scripts/analyze_riffrec_zip.py /path/to/input |
There was a problem hiding this comment.
The runbook uses python scripts/analyze_riffrec_zip.py, but this script is Python 3 code and many developer environments only provide python3 (or map python inconsistently). In those environments, the analyzer step fails before any artifacts are produced, breaking both quick and extensive paths. Use python3 (or an executable script invocation) consistently in the documented command so the workflow is runnable by default.
Useful? React with 👍 / 👎.
1 participant
Summary
riffrec-feedback-analysisthat turns Riffrec recordings (or any video/audio/notes) into structured product feedback.docs/brainstorms/writes, noce-brainstormhandoff. Auto-escalates if multiple issues surface.analysis.md,problem-analysis.md,source-materials.md,requirements-kickoff.md, source mapping) with automatic handoff toce-brainstorm.~/.codex/skills/copy: absolute paths swapped for relative,ce-brainstormreferenced as a bare skill per plugin convention, Riffrec repo surfaced in SKILL.md and README.Test plan
bun run release:validateclean — 49 agents, 39 skills, 0 MCP serversbun test tests/frontmatter.test.ts— 260/260 pass (incl. new skill)bun test tests/skill-shell-safety.test.ts— 82/82 pass🤖 Generated with Claude Code