fix(brief): render visual point-of-view paths inside the project worktree - #47
Merged
Merged
Conversation
TastyTom13
force-pushed
the
fm/fm-brief-visual-pov-paths
branch
from
September 30, 2026 08:28
5d86dfe to
04ad0fc
Compare
…ons.test.sh, the "budget exhaustion (hang)" case. It has nothing to do with this PR's brief changes. The test gave the poll a 1 second budget on the real clock. When the wall clock ticked over a second at the wrong moment, the poll gave up before it called the forge, so the test failed at random. I could make it fail locally on the second run. Fix: the test now freezes the fake clock in both modes (hang and exhaust), as the exhaust mode already did (tests/fm-contributions.test.sh, in test_budget_exhaustion_keeps_prior_record). Did it work: yes. I ran the whole file 6 times after the fix and all 6 passed. Before the fix, it failed on the 2nd local run. What is left: the change is uncommitted and not pushed. The outer executor must push it and let CI re-run shard 6
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
1 participant
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
Said 2026-09-30 about design work in general: "All of that should already be understood by any agent doing design work. Not just for this design, but any future design. So please make sure you are aware of that going forward."
Context: the merged visual-work contract makes bin/fm-brief.sh --visual --surface bind a point-of-view document. Its table points decks at $FM_HOME/docs/design/point-of-view.md, scout at $FM_HOME/docs/design-system/point-of-view.md and website at $FM_HOME/sites/tomasmeulenberg/design-concepts/POINT-OF-VIEW.md. None of those exist in the firstmate home: the point-of-view report places them inside the project checkouts: projects/doc-creator/docs/design/point-of-view.md, projects/scout/docs/design-system/point-of-view.md, and the Forge site design-concepts folder. The first scout brief built with the flag rendered a dead path.
What Changed
bin/fm-brief.sh --visual --surfacenow renders thedecks,scoutandwebsitepoint-of-view documents as paths relative to the worker's own worktree (for exampledocs/design-system/point-of-view.md, followed by "in your worktree"). Before, it pointed at$FM_HOME/...paths that do not exist. Theemaildocument still lives under$FM_HOME/data/standards/.$FM_HOME/projects/<repo-name>/<path>only to decide whether to warn. If the document is missing, it prints a warning that names the path it checked and does not refuse. For the project surfaces the brief says the check was advisory. The worker reads the document in its worktree if it exists, creates it from the ratified point-of-view report only if it is absent there, and never overwrites an existing one. Foremail, the brief tells the worker to create the missing document at that path..agents/skills/visual-work/SKILL.mdupdates the surface table and text to match.tests/fm-brief.test.shnow builds fixture projects, and asserts the worktree-relative wording, no primary-checkout path in the brief, and no warning when the document exists. It also asserts the warning and the advisory or create wording when the document is missing, for all four surfaces.Risk Assessment
✅ Low: The change is small and confined to brief text and one advisory warning. It follows the recorded decisions: worktree-relative paths, the repo name checked under $FM_HOME/projects, and advisory wording. The tests run the real scaffold and assert the emitted brief and warning.
Testing
I ran the real brief script against a throwaway firstmate home. Scout, decks and website briefs name the document by a worktree-relative path ("in your worktree"), and never leak a primary-checkout path. A missing document prints a warning naming the exact primary-checkout path checked, and the brief is worded as advisory: read it if present, create only if absent, never overwrite. Email stays an absolute path under the firstmate home. Non-visual briefs are unchanged. Bad or missing surface is refused. The existing
tests/fm-brief.test.shpasses. I did not run the full repo suite. The throwaway home was removed, and the worktree is clean.docs/design-system/point-of-view.mdin your worktree; no warning; no $H/projects path in the briefEvidence: Live fm-brief.sh transcript (6 scenarios)
Source: Live fm-brief.sh transcript (6 scenarios)
Evidence: fm-brief.test.sh output
Source: fm-brief.test.sh output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (2) ✅
bin/fm-brief.sh:554- The fix treats the second argument as a directory path, but that argument is a repo NAME. The usage line says<repo-name>. Every other test passes a bare name (firstmate,alpha,sample,some-proj). The script only uses it as text in the brief (worktree of $REPO). The firstmate-coding-guidelines skill calls it a caller-supplied string. Real projects live at$FM_HOME/projects/<name>(for example~/firstmate/projects/forge). Trace:fm-brief.sh t scout --scout --visual --surface scoutsets POS[1]=scout, so line 554 buildsscout/docs/design-system/point-of-view.md. That path is relative to the caller's working directory, so it is still a dead path and no warning tells the worker why. This is the exact failure in the intent (the first scout brief built with the flag rendered a dead path). It is only fixed when the caller passes an absolute path, which nothing documents. Same defect at: bin/fm-brief.sh:554 (path build), bin/fm-brief.sh:17-18 and 90-97 (help still says repo-name while the new text says target project directory), .agents/skills/visual-work/SKILL.md:20-29 (<project>described as a directory passed as the second argument), tests/fm-brief.test.sh:1517-1526 (passes an absolute$proj, which hides the bug), tests/fm-brief.test.sh:1561-1570 (same in the missing-document loop). The intent points atprojects/doc-creator/...,projects/scout/...andprojects/forge/sites/tomasmeulenberg/design-concepts/.... The smallest fix is to resolve the name to$FM_HOME/projects/<name>(keep an absolute path as it is). Then run the tests with a bare repo name and a real$FM_HOME/projects/<name>tree. The report also puts the website document inprojects/forge, so the surface-to-project mapping should be checked too.bin/fm-brief.sh:557- Once the path resolves into$FM_HOME/projects/<name>, it is the project's PRIMARY checkout. The worker runs in a disposable worktree. For a missing document, the brief sayscreate it at that path. That writes into the primary checkout, outside the worktree that gets committed, pushed and torn down. The file would never reach the PR, and it would dirty the primary checkout. For an existing document, the primary checkout may be behind the worker's base branch. The right rule is a product decision: point the worker at the same relative path inside its own worktree, or keep the absolute primary-checkout path.🔧 Fix applied.
3 issues (1 error, 2 warnings) still open:
bin/fm-brief.sh:554- The fix treats the second argument as a directory path, but that argument is a repo NAME. The usage line says<repo-name>. Every other test passes a bare name (firstmate,alpha,sample,some-proj). The script only uses it as text in the brief (worktree of $REPO). The firstmate-coding-guidelines skill calls it a caller-supplied string. Real projects live at$FM_HOME/projects/<name>(for example~/firstmate/projects/forge). Trace:fm-brief.sh t scout --scout --visual --surface scoutsets POS[1]=scout, so line 554 buildsscout/docs/design-system/point-of-view.md. That path is relative to the caller's working directory, so it is still a dead path and no warning tells the worker why. This is the exact failure in the intent (the first scout brief built with the flag rendered a dead path). It is only fixed when the caller passes an absolute path, which nothing documents. Same defect at: bin/fm-brief.sh:554 (path build), bin/fm-brief.sh:17-18 and 90-97 (help still says repo-name while the new text says target project directory), .agents/skills/visual-work/SKILL.md:20-29 (<project>described as a directory passed as the second argument), tests/fm-brief.test.sh:1517-1526 (passes an absolute$proj, which hides the bug), tests/fm-brief.test.sh:1561-1570 (same in the missing-document loop). The intent points atprojects/doc-creator/...,projects/scout/...andprojects/forge/sites/tomasmeulenberg/design-concepts/.... The smallest fix is to resolve the name to$FM_HOME/projects/<name>(keep an absolute path as it is). Then run the tests with a bare repo name and a real$FM_HOME/projects/<name>tree. The report also puts the website document inprojects/forge, so the surface-to-project mapping should be checked too.bin/fm-brief.sh:557- Once the path resolves into$FM_HOME/projects/<name>, it is the project's PRIMARY checkout. The worker runs in a disposable worktree. For a missing document, the brief sayscreate it at that path. That writes into the primary checkout, outside the worktree that gets committed, pushed and torn down. The file would never reach the PR, and it would dirty the primary checkout. For an existing document, the primary checkout may be behind the worker's base branch. The right rule is a product decision: point the worker at the same relative path inside its own worktree, or keep the absolute primary-checkout path.bin/fm-brief.sh:571- The brief states as fact that the document 'does not exist yet' and orders the worker to 'create it', but the scaffold only checked the project's PRIMARY checkout at $FM_HOME/projects/<repo-name>. The worker reads from its own worktree, which may be on a different --base branch or newer than the primary checkout. Trace: primary checkout is stale or on another branch and lacks docs/design/point-of-view.md, while the worker's base branch already has it. The brief still says 'create it at that path in your worktree', so the worker can overwrite the ratified document. The fix from the prior round left the brief text unconditional even though the check runs in a different tree. Smallest fix: word the note as conditional, for example 'If it is not in your worktree, create it ...'. Also for scout briefs: the scout worktree is scratch with no commit or push, so 'a created file lands in its commit' (skills/visual-work/SKILL.md, bin/fm-brief.sh header) is false for --scout, and a document created there is discarded. Whether a scout should create it at all is a product decision.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Ranbin/fm-brief.shfor real in a throwaway FM_HOME with bare repo names (scout, doc-creator, forge)Scout with the doc present: no warning, brief names docs/design-system/point-of-view.md in your worktreeDecks and website with the doc missing: warning names the primary-checkout path checked, brief uses the advisory wordingEmail: absolute path under FM_HOME, still worksChecked no primary-checkout path leaks into any briefChecked a non-visual brief is unchanged, and --surface without --visual is refusedbash tests/fm-brief.test.sh(the whole file for this script only)✅ No issues found.
docs/design-system/point-of-view.mdin your worktree; no warning; no $H/projects path in the briefFM_HOME=<tmp> bin/fm-brief.sh s1 scout --scout --visual --surface scoutwith the document present in the primary checkoutfm-brief.sh s2 doc-creator --mode no-mistakes --visual --surface deckswith the document missingfm-brief.sh s3 forge --mode direct-PR --visual --surface websitewith the project folder absentfm-brief.sh s4 forge --mode local-only --visual --surface emailfm-brief.sh s5 forge --mode local-only(no visual flag)fm-brief.shwith--surface nopeand with--visualbut no--surfacebash tests/fm-brief.test.sh(whole brief test file, exit 0)✅ **Document** - passed
✅ No issues found.
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
✅ No issues found.
Built by: claude/sonnet at medium