fix: reduce crewmate status wake noise - #6
Merged
Conversation
|
Fork pushed: https://github.com/bryceeppler/firstmate/tree/sandboxed-crewmates (f7db9fe). All acceptance criteria verified - see the implementation comment above. |
andrew-kim-techtorch
referenced
this pull request
in andrew-kim-techtorch/firstmate
Jul 7, 2026
…hment requirement Proven live on PR #11 and PRs #6-#8: github.com/<owner>/<repo>/raw/<sha>/<path> renders inline for authenticated members on private repos; raw.githubusercontent.com does not. Attachment upload (browser-only) is no longer needed. Convention: commit PNGs to docs/pr-screenshots/<task-id>/ and reference via github.com/<owner>/<repo>/raw/<commit-sha>/<path> in the PR body table. fm-pr-body-check --ui now accepts github.com/.../raw/... URLs in addition to existing GitHub attachment URL patterns; blob links still fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
andrew-kim-techtorch
referenced
this pull request
in andrew-kim-techtorch/firstmate
Jul 7, 2026
) * feat(bin): add PR body conventions to ship briefs and body-check script Add a PR body conventions block to ship briefs (no-mistakes and direct-PR modes) so crewmates always receive clear guidance: lead with the requirement satisfied, use rendered screenshots from raw.githubusercontent.com or GitHub attachments (never local /var/folders/private/tmp/Users paths), and present before/after side by side in a table. Add bin/fm-pr-body-check.sh that firstmate runs before relaying any PR as ready; --ui mode also asserts at least one rendered image is present. Wire a one-line reference into AGENTS.md section 7 PR ready. Add tests/fm-pr-body-check.test.sh covering the full pass/fail matrix. * fix(bin): correct PR screenshot convention for private repos raw.githubusercontent.com URLs do not render inline on private repos (GitHub image proxy cannot authenticate). Update the ship-brief PR body conventions and fm-pr-body-check to reflect the correct approach: - Commit screenshots under docs/pr-screenshots/<task-id>/ and reference them as clickable blob links; authenticated reviewers can open these. - Flag in the PR body when inline rendering is wanted so a human can upload images as GitHub attachments (browser drag-drop only). - fm-pr-body-check now always fails on raw.githubusercontent.com refs, in addition to local filesystem paths. - --ui screenshot detection expanded to match blob links to image files as well as inline GitHub attachment images. - 11 tests covering the full pass/fail matrix. * fix(bin): require GitHub attachment URLs for --ui inline screenshots Blob links (github.com/.../blob/...) open in a browser tab but do not render inline in the PR body. The universal inline method for any GitHub repo (public or private) is GitHub attachments, served from user-attachments/assets or <owner>/<repo>/assets URLs. Update ship-brief conventions: attachment URLs required for inline rendering; commit source PNGs under docs/pr-screenshots/ for history; flag the need for a human to perform the browser drag-drop upload. Update fm-pr-body-check --ui: pass only when the body contains at least one attachment-style URL; blob links no longer satisfy the check. 10 tests covering the full pass/fail matrix. * chore: ignore .serena/ tool state directory * no-mistakes(review): fail loudly on PR body fetch errors in fm-pr-body-check * no-mistakes(document): docs: sync pr-body-check script into scripts, tests, and brief-contract docs * fix(bin): use github.com/raw/<sha> for inline screenshots, drop attachment requirement Proven live on PR #11 and PRs #6-#8: github.com/<owner>/<repo>/raw/<sha>/<path> renders inline for authenticated members on private repos; raw.githubusercontent.com does not. Attachment upload (browser-only) is no longer needed. Convention: commit PNGs to docs/pr-screenshots/<task-id>/ and reference via github.com/<owner>/<repo>/raw/<commit-sha>/<path> in the PR body table. fm-pr-body-check --ui now accepts github.com/.../raw/... URLs in addition to existing GitHub attachment URL patterns; blob links still fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Jul 19, 2026
This was referenced Jul 21, 2026
This was referenced Aug 1, 2026
This was referenced Aug 4, 2026
vipentti
pushed a commit
to vipentti/firstmate
that referenced
this pull request
Aug 5, 2026
* fix(brief): report status on phase changes only, not every step * no-mistakes(document): Document sparse crewmate status cadence
4 tasks
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
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.
Intent
Reduce firstmate wake noise from crewmate status reporting. The captain observed that crewmates write FYI-ish status lines (e.g. 'installed dependencies', 'attached chrome-devtools-axi') that each wake the supervising firstmate agent for no actionable reason. Tighten the brief scaffold's reporting rule (rule 4 in bin/fm-brief.sh) so crewmates append status lines only on real phase changes a supervisor would act on (setup done, bug reproduced, fix implemented, validation passed) plus the needs-decision/blocked/done/failed states, and explicitly not step-by-step progress - firstmate peeks the tmux pane for that. Deliberate decisions: keep the same status file mechanism and state vocabulary unchanged; the change is wording-only in the scaffold heredoc, no behavior change to any script logic; the rationale sentence 'Each append wakes firstmate' is intentionally included so crewmate agents understand why sparse reporting matters.
What Changed
fm-brief.shscaffold so crewmates report only supervisor-actionable phase changes plusneeds-decision/blocked/done/failedstates.AGENTS.md.Risk Assessment
✅ Low: The branch only tightens wording in the brief scaffold without changing script behavior, and the new guidance matches the watcher/status-file contract.
Testing
Exercised the actual brief-scaffolding CLI path and captured the generated crewmate-facing markdown; no automated test suite exists for this repo, no lint/static analysis was run, the transient worktree fixture was removed, and the generated brief directly demonstrates the intended sparse status-reporting rule.
Evidence: Generated crewmate brief showing sparse status cadence rule
Lines 17-21 show:States: working, needs-decision, blocked, done, failed.followed byEach append wakes firstmate, so report sparingly: only phase changes a supervisor would act on (setup done, bug reproduced, fix implemented, validation passed) and the needs-decision/blocked/done/failed states. No step-by-step FYI progress lines; firstmate reads your pane for that.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Inspected the target diff withgit diff --stat 683b8cf2ab701edca9209656812b61c9f5ff2656..ad561356880e2df9df9a9ffb4c78b7f78d127e7b.Confirmed there are no repository test files or package test scripts via file discovery.Ran the end-user CLI path:bin/fm-brief.sh status-cadence-e2e-01kty demo-repo.Verified the generateddata/status-cadence-e2e-01kty/brief.mdincludes the unchanged state vocabulary, theEach append wakes firstmaterationale, the allowed phase-change examples, and theNo step-by-step FYI progress linesinstruction.Copied the generated brief to/var/folders/5x/4nqprlbx0518k3ybcb1sz6gr0000gn/T/no-mistakes-evidence/01KTYFNG7DD7NG5V3S3TFVC493/generated-brief-status-cadence.md.Removed the transient generated worktree fixture withrm -rf data/status-cadence-e2e-01ktyand verifiedgit status --shortwas clean.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.