Land wayfinder planning artifacts - #37
Merged
Merged
Conversation
Bring the output of the "Plan secure owner content editing and publishing" wayfinder map (#27) onto main. Every artifact the effort produced was stranded on unmerged branches, including the agent tracker docs, which meant fresh sessions could not find the issue tracker. - Agent setup: AGENTS.md, docs/agents/{domain,issue-tracker,triage-labels}.md, .agents/skills with .claude/skills symlinks, skills-lock.json - CONTEXT.md domain model, including the editable-content and publication-lifecycle vocabulary added while working the map - Research docs backing tickets #29, #30, #31, and #35 Also un-ignore docs/research/ in .gitignore. The existing rule allowed only docs/agents/ and docs/adr/, so the four research docs survived solely because they were already tracked and any new one would have been silently skipped. Planning only; no runtime code changes. The owner-workspace prototype branch is deliberately left unmerged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded a repository-wide agent skill system with routing, implementation, testing, planning, teaching, prototyping, architecture, research, domain, and operational guidance. Added repository instructions, Claude skill links, domain context, research documents, and a skill lockfile. ChangesRepository foundation
Agent workflows
Exploration and knowledge workflows
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 48
🤖 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 @.agents/skills/ask-matt/SKILL.md:
- Around line 66-75: Update the skill routing documentation in ask-matt’s main
flow or Standalone section to account for the resolving-merge-conflicts skill.
Add a concise route entry describing when users should invoke it during an
active merge or rebase conflict, or explicitly document that the skill is
intentionally excluded.
In @.agents/skills/codebase-design/DEEPENING.md:
- Line 34: Revise the testing guidance in DEEPENING.md so shallow-module tests
are not deleted merely because deepened-module interface tests exist. Require
equivalent behavior coverage before retiring tests, while retaining adapter,
integration, and unique contract tests that the deep interface does not cover.
In @.agents/skills/codebase-design/SKILL.md:
- Around line 34-52: Add the text language identifier to both fenced ASCII-art
diagram blocks in .agents/skills/codebase-design/SKILL.md lines 34-52 and both
fenced directory-tree blocks in .agents/skills/domain-modeling/SKILL.md lines
14-38, without changing their contents.
In @.agents/skills/diagnosing-bugs/scripts/hitl-loop.template.sh:
- Line 13: Update the input capture and final KEY=VALUE emission in the HITL
loop so multiline stack traces are preserved; either enforce a single-line error
value or use delimiter-based capture and encode newlines before output. Apply
this consistently to the read logic and the error-value handling referenced near
the prompt and emission sections.
In @.agents/skills/diagnosing-bugs/SKILL.md:
- Around line 43-45: Update the “Non-deterministic bugs” guidance to require
stress reproduction only against an isolated non-production target, with
explicit caps for iterations, concurrency, and request rate before looping,
parallelising, or adding timing stress. Preserve the goal of increasing
reproduction reliability while preventing uncontrolled load.
- Around line 18-29: Update the “Replay a captured trace” guidance and the
corresponding workflow section around lines 51-58 to require redacting
Authorization headers, cookies, API keys, passwords, tokens, and PII before
replaying or sharing artifacts. Instruct agents to prefer synthetic fixtures
over production captures while preserving the existing replay workflow.
In @.agents/skills/grill-me/SKILL.md:
- Line 7: Add a top-level Markdown heading immediately after the front matter in
each affected file: use “# Grill me” in .agents/skills/grill-me/SKILL.md at
lines 7-7 before the grilling instruction, “# Research” in
.agents/skills/research/SKILL.md at lines 6-6 before the background-agent
instruction, and “# Resolving merge conflicts” in
.agents/skills/resolving-merge-conflicts/SKILL.md at lines 6-6 before the
numbered procedure.
In @.agents/skills/implement/SKILL.md:
- Around line 9-15: Update the workflow guidance in SKILL.md to require invoking
/tdd for every testable behavior, completing one red-green slice at a time,
rather than treating it as optional. Require regular typechecks and focused
tests, the full test suite at the end, and successful completion of all checks
before using /code-review or committing.
In @.agents/skills/improve-codebase-architecture/HTML-REPORT.md:
- Line 3: Update the architectural review artifact description so its dependency
behavior is accurate: either embed pinned Tailwind and Mermaid assets in the
generated HTML, or remove the “self-contained” claim and explicitly document the
required network access for CDN resources. Keep the existing rendering guidance
for Mermaid, divs, and inline SVG unchanged.
- Around line 16-20: Update the mermaid.initialize configuration in
HTML-REPORT.md to use securityLevel "strict" for repository-derived diagrams,
and escape repository-provided text before inserting it into Mermaid labels.
Retain "loose" only if the diagram source is demonstrably sanitized and
hostile-label behavior is covered by tests.
In @.agents/skills/improve-codebase-architecture/SKILL.md:
- Around line 43-50: Update the report-generation instructions around the
candidate card fields and Before / After diagrams to require context-aware HTML
escaping for all repository-derived values, including file names, problem,
solution, benefits, and diagram labels. Require safe serialization or encoding
of Mermaid data before embedding it, while preserving the existing card
structure and rendering behavior.
- Around line 20-25: Bound the initial history scan in the scope-selection
guidance by using a limited git log query, such as the most recent 100 commits,
before identifying hot spots. Keep subsequent history inspection scoped to
relevant paths and preserve the existing glossary and ADR-reading steps.
- Around line 37-41: Update the HTML report creation instructions around the
architecture-review filename to require an atomic temporary-file API such as
mktemp or tempfile, using exclusive creation and restrictive 0600 permissions.
Preserve OS-specific temp-directory resolution and opening behavior, but do not
write directly to the predictable timestamp-based path; report the resulting
absolute temporary-file path.
- Around line 39-41: Update the report-generation guidance in the self-contained
HTML workflow to prohibit remote Tailwind and Mermaid CDN scripts, and require
locally vendored or pinned local assets with fixed digests instead. Preserve the
existing before/after visualisation requirement and the OS-specific temp-file
opening behavior.
In @.agents/skills/portfolio-helper/SKILL.md:
- Line 34: Update the guidance around the diff workflow near the existing `git
diff` recommendations to prohibit unrestricted raw diffs that may expose
secrets. Require checking changed paths and diff statistics first, then
inspecting only author-confirmed non-secret files after redaction or a secret
scan; preserve the existing `.env`/secret handling and testing guidance.
In @.agents/skills/prototype/LOGIC.md:
- Around line 52-55: Update the “Read one keystroke” step in the interaction
loop to call the pure reducer or state-machine interface from the portable logic
module, then replace the current state with its returned value rather than
mutating state directly. Keep the existing dispatch and re-render flow
unchanged.
In @.agents/skills/prototype/UI.md:
- Around line 77-89: Update the PrototypeSwitcher controls to use native button
elements with accessible names for both arrow actions, include clearly visible
focus states, and announce the current variant through an appropriate accessible
status or live region. Preserve the existing click, URL-sync, and arrow-key
cycling behavior.
- Line 20: Update the variant-switching instructions around the same-route
rendering flow and lines 85-88 so changing variant modifies only the variant
query parameter while preserving all existing search parameters, route data,
params, authentication context, and URL hash. Do not replace the full query
string or discard IDs, filters, or pagination state.
- Around line 60-68: Normalize the variant selection before rendering: validate
the value returned by searchParams.get("variant") against the declared A, B, and
C variants, and fall back to "A" for missing or unknown values. Use this
normalized variant for both the VariantA/VariantB/VariantC branches and
PrototypeSwitcher.current.
- Around line 87-90: Update the prototype rendering flow around
PrototypeSwitcher so the entire selected variant subtree is excluded in
production, not just the switcher. Place both the variant output and
PrototypeSwitcher inside the host framework’s build-time production guard, and
preserve the same guard for sub-shape B throwaway routes.
In @.agents/skills/resolving-merge-conflicts/SKILL.md:
- Line 10: Update the “Resolve each hunk” guidance in the
resolving-merge-conflicts skill to allow stopping for user clarification or
safely aborting when the primary sources do not establish a confident
resolution; remove the unconditional “Always resolve; never --abort” requirement
while preserving the instruction to avoid inventing behavior and to document
trade-offs when choosing between incompatible intents.
- Line 14: Update the “Finish the merge/rebase” guidance to avoid staging
unrelated user changes: inspect git status and git diff, stage only the paths
involved in conflict resolution, and stop instead of committing when unrelated
pre-existing changes are present.
In @.agents/skills/setup-matt-pocock-skills/domain.md:
- Around line 17-39: Update both fenced directory diagrams in the domain
documentation to declare the text language by changing each opening fence to use
text; leave the directory tree content and closing fences unchanged.
In @.agents/skills/setup-matt-pocock-skills/issue-tracker-github.md:
- Line 45: Make resolution updates recoverable by reordering each workflow to
write and verify the map decision pointer before completing the tracker item: in
.agents/skills/setup-matt-pocock-skills/issue-tracker-github.md:45 update
Decisions-so-far before closing the GitHub issue; in
.agents/skills/setup-matt-pocock-skills/issue-tracker-gitlab.md:46 update the
map pointer before closing the GitLab issue; and in
.agents/skills/setup-matt-pocock-skills/issue-tracker-local.md:30 append and
verify the map pointer before setting the local ticket to resolved.
- Line 8: Update the “Read an issue” command documentation to request structured
JSON fields for labels and comments before applying jq, matching the existing
list-command pattern. Ensure the documented gh issue view invocation uses --json
with the relevant fields and filters the resulting structured data with --jq.
- Line 43: Update the frontier queries to fetch all open child issues before
applying blocker, assignee, and map-order filtering. In
.agents/skills/setup-matt-pocock-skills/issue-tracker-github.md at line 43,
specify GitHub pagination via an appropriate --limit value or gh api --paginate;
in .agents/skills/setup-matt-pocock-skills/issue-tracker-gitlab.md at line 44,
specify glab’s --all option or explicit page iteration. Preserve the existing
filtering and first-in-map-order selection behavior.
- Line 23: Update the “List external PRs for triage” command to use gh api
rather than gh pr list, requesting author_association and comments with the
needed pull-request fields. Filter results to CONTRIBUTOR, FIRST_TIMER,
FIRST_TIME_CONTRIBUTOR, or NONE, excluding repository owners, members, and
collaborators.
In @.agents/skills/setup-matt-pocock-skills/issue-tracker-local.md:
- Around line 7-9: Use one consistent `.scratch/<feature-slug>/` directory key
throughout the local tracker instructions, including the publish, fetch, and
Wayfinding workflows currently referencing `<effort>`. Update the affected
references so all local tracker artifacts target the same feature directory.
- Line 10: Update the issue-file metadata guidance so the canonical triage role
uses a separate Triage: field, while Status: is reserved for Wayfinding workflow
states such as claimed and resolved. Make the triage-labels.md reference
conditional on the triage skill being installed, including the related guidance
at the Status: lines.
In @.agents/skills/setup-matt-pocock-skills/SKILL.md:
- Around line 42-49: Update the exploration and configuration-writing guidance
in the issue-tracker selection sections, including Sections A and B, so existing
tracker settings and agent/label mappings are treated as the default source of
truth on reruns. Require an explicit replacement decision before changing any
existing custom configuration; accepting a recommendation alone must preserve
the current values. Apply the same guard to the related guidance at the
referenced later section.
- Around line 25-30: Broaden the context-layout detection workflow around the
existing repository signals to inspect workspace manifests and context
directories across the repository, including Cargo, Go, Gradle, Nx, apps, and
packages conventions. Classify repositories as multi-context when these signals
indicate separate contexts, avoid generating root-only guidance in that case,
and ask the user when the layout remains ambiguous. Apply the same detection
behavior to the related logic at the referenced later section.
In @.agents/skills/tdd/tests.md:
- Around line 29-35: Update the anti-pattern test example around the checkout
test to create the method spy with jest.spyOn(paymentService, "process") instead
of assigning the result of jest.mock(paymentService) to mockPayment, and keep
the existing mockPayment.process assertion shape valid.
In @.agents/skills/teach/LEARNING-RECORD-FORMAT.md:
- Around line 21-23: Update the learning-record template to include the exact
parseable frontmatter block, including the documented Status field, before the
title. Define the identifier convention used by supersession references,
explicitly stating how an identifier such as LR-0001 maps to the corresponding
0001-slug.md filename, and ensure the status and reference examples consistently
use that convention.
In @.agents/skills/teach/SKILL.md:
- Around line 10-20: Update the “Teaching Workspace” inventory in the skill
contract to include the canonical ./GLOSSARY.md path and its required
create/load behavior, consistent with GLOSSARY-FORMAT.md and the lesson
dependency described near line 136. Preserve the existing inventory structure
and wording style while ensuring agents know where the glossary belongs and when
it must be created or loaded.
In @.agents/skills/to-tickets/SKILL.md:
- Line 17: Update the reference-handling instructions in SKILL.md so only
repository-relative spec files and URLs hosted on trusted ticket trackers are
fetched automatically. Reject file:, absolute-path, localhost, private-network,
and metadata endpoints unless the user explicitly approves the URI, while
preserving the existing full-body-and-comments fetching behavior for allowed
references.
In @.agents/skills/triage/AGENT-BRIEF.md:
- Around line 41-71: Update the agent-brief template in
.agents/skills/triage/AGENT-BRIEF.md (lines 41-71) and the needs-info template
in .agents/skills/triage/SKILL.md (lines 94-106) so every generated triage
comment or issue begins with the exact disclaimer `> *This was generated by AI
during triage.*`; either prepend it directly to both templates or implement one
guaranteed shared wrapper applied to both.
In @.agents/skills/triage/OUT-OF-SCOPE.md:
- Around line 10-15: Specify a language on both Markdown fences triggering
MD040: in .agents/skills/triage/OUT-OF-SCOPE.md lines 10-15, mark the directory
tree fence as text; in .agents/skills/triage/SKILL.md lines 15-17, mark the
disclaimer fence as markdown or replace it with a normal blockquote.
- Around line 56-108: Remove the stray opening and closing code-fence markers
surrounding the guidance after the “Prior requests” section in OUT-OF-SCOPE.md.
Preserve the intervening headings, lists, and prose as normal Markdown so
sections such as “Naming the file,” “Writing the reason,” and “Updating or
removing out-of-scope files” render correctly.
In @.agents/skills/triage/SKILL.md:
- Line 90: Update the ready-for-agent handling in the maintainer instruction so
that the state is not applied without an attached agent brief. Require an
existing brief or generate one as part of the override before changing the
role/state; otherwise keep the item in its current or another appropriate state,
and retain the confirmation step for role changes, comments, and closure.
- Around line 70-72: Update the triage workflow around the “redundancy” check in
Gather context so locating an existing implementation triggers explicit behavior
verification before classifying the request as “already implemented” or wontfix.
Preserve the search-and-report requirement, and only select that classification
after confirming the implementation satisfies the requested behavior; otherwise
continue normal recommendation analysis.
- Line 74: Update the “Verify the claim” guidance in the triage skill to require
read-only inspection during discovery and an explicit isolated, no-credential
execution boundary for external PR setup, tests, or commands. Require an
allowlist before executing PR-related commands, while preserving the existing
requirement to report confirmed, failed, or insufficient verification outcomes.
In @.agents/skills/wayfinder/SKILL.md:
- Line 25: Update the tracker fallback in the Wayfinder skill’s tracker guidance
to match the default established by setup-matt-pocock-skills, rather than
defaulting independently to local-markdown. Preserve the instruction to consult
the provided tracker’s “Wayfinding operations” section, and ensure all skill
documentation uses the same setup-configured default.
- Around line 122-128: Update the Work-through-the-map procedure to make ticket
claims conditional: assign only when the selected ticket is still unassigned,
then re-read and verify ownership before and after resolution, abandoning the
ticket if another session claimed it. Make the Decisions-so-far append
concurrency-safe by using optimistic conflict detection or serialized
read-modify-write updates so concurrent appends are preserved.
In @.agents/skills/writing-great-skills/SKILL.md:
- Around line 7-9: Add the top-level Markdown heading “Writing Great Skills”
immediately after the frontmatter and before the opening paragraph in SKILL.md,
ensuring it is the document’s first body element.
In `@AGENTS.md`:
- Line 3: Remove the mandatory “Always run the caveman skill” instruction from
AGENTS.md, since no discoverable caveman skill exists; only retain it if the
repository adds and exposes that skill.
In `@docs/agents/issue-tracker.md`:
- Line 8: Update the “Read an issue” recipe to use machine-readable gh output by
requesting number, title, body, labels, and comments with --json and filtering
via --jq, or separate the human-readable view from the structured command;
ensure the documented jq filtering and label retrieval operate on valid JSON.
- Line 9: Update the issue-listing command near the line-9 list output to fetch
and expose the supported issue dependency summary, including
issue_dependencies_summary.blocked_by, before the frontier-ticket selection
logic uses it around the issue_dependencies_summary filter. Preserve the
existing number, title, body, labels, and comments output while ensuring callers
can identify and exclude blocked children.
In `@docs/research/github-owner-auth-session-security.md`:
- Line 71: Update the RFC citation in the cookie prefix explanation, replacing
the incorrect rfc10025 link with a stable/current RFC 6265bis RFC-Editor link,
or remove the RFC label while retaining the MDN reference.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 6332bcb4-eced-47f9-b383-97226bcdd6ee
📒 Files selected for processing (100)
.agents/skills/ask-matt/SKILL.md.agents/skills/ask-matt/agents/openai.yaml.agents/skills/code-review/SKILL.md.agents/skills/code-review/agents/openai.yaml.agents/skills/codebase-design/DEEPENING.md.agents/skills/codebase-design/DESIGN-IT-TWICE.md.agents/skills/codebase-design/SKILL.md.agents/skills/codebase-design/agents/openai.yaml.agents/skills/diagnosing-bugs/SKILL.md.agents/skills/diagnosing-bugs/agents/openai.yaml.agents/skills/diagnosing-bugs/scripts/hitl-loop.template.sh.agents/skills/domain-modeling/ADR-FORMAT.md.agents/skills/domain-modeling/CONTEXT-FORMAT.md.agents/skills/domain-modeling/SKILL.md.agents/skills/domain-modeling/agents/openai.yaml.agents/skills/grill-me/SKILL.md.agents/skills/grill-me/agents/openai.yaml.agents/skills/grill-with-docs/SKILL.md.agents/skills/grill-with-docs/agents/openai.yaml.agents/skills/grilling/SKILL.md.agents/skills/grilling/agents/openai.yaml.agents/skills/handoff/SKILL.md.agents/skills/handoff/agents/openai.yaml.agents/skills/implement/SKILL.md.agents/skills/implement/agents/openai.yaml.agents/skills/improve-codebase-architecture/HTML-REPORT.md.agents/skills/improve-codebase-architecture/SKILL.md.agents/skills/improve-codebase-architecture/agents/openai.yaml.agents/skills/portfolio-helper/SKILL.md.agents/skills/prototype/LOGIC.md.agents/skills/prototype/SKILL.md.agents/skills/prototype/UI.md.agents/skills/prototype/agents/openai.yaml.agents/skills/research/SKILL.md.agents/skills/research/agents/openai.yaml.agents/skills/resolving-merge-conflicts/SKILL.md.agents/skills/resolving-merge-conflicts/agents/openai.yaml.agents/skills/setup-matt-pocock-skills/SKILL.md.agents/skills/setup-matt-pocock-skills/agents/openai.yaml.agents/skills/setup-matt-pocock-skills/domain.md.agents/skills/setup-matt-pocock-skills/issue-tracker-github.md.agents/skills/setup-matt-pocock-skills/issue-tracker-gitlab.md.agents/skills/setup-matt-pocock-skills/issue-tracker-local.md.agents/skills/setup-matt-pocock-skills/triage-labels.md.agents/skills/tdd/SKILL.md.agents/skills/tdd/agents/openai.yaml.agents/skills/tdd/mocking.md.agents/skills/tdd/tests.md.agents/skills/teach/GLOSSARY-FORMAT.md.agents/skills/teach/LEARNING-RECORD-FORMAT.md.agents/skills/teach/MISSION-FORMAT.md.agents/skills/teach/RESOURCES-FORMAT.md.agents/skills/teach/SKILL.md.agents/skills/teach/agents/openai.yaml.agents/skills/to-spec/SKILL.md.agents/skills/to-spec/agents/openai.yaml.agents/skills/to-tickets/SKILL.md.agents/skills/to-tickets/agents/openai.yaml.agents/skills/triage/AGENT-BRIEF.md.agents/skills/triage/OUT-OF-SCOPE.md.agents/skills/triage/SKILL.md.agents/skills/triage/agents/openai.yaml.agents/skills/wayfinder/SKILL.md.agents/skills/wayfinder/agents/openai.yaml.agents/skills/writing-great-skills/GLOSSARY.md.agents/skills/writing-great-skills/SKILL.md.agents/skills/writing-great-skills/agents/openai.yaml.claude/skills/ask-matt.claude/skills/code-review.claude/skills/codebase-design.claude/skills/diagnosing-bugs.claude/skills/domain-modeling.claude/skills/grill-me.claude/skills/grill-with-docs.claude/skills/grilling.claude/skills/handoff.claude/skills/implement.claude/skills/improve-codebase-architecture.claude/skills/prototype.claude/skills/research.claude/skills/resolving-merge-conflicts.claude/skills/setup-matt-pocock-skills.claude/skills/tdd.claude/skills/teach.claude/skills/to-spec.claude/skills/to-tickets.claude/skills/triage.claude/skills/wayfinder.claude/skills/writing-great-skills.gitignoreAGENTS.mdCONTEXT.mddocs/agents/domain.mddocs/agents/issue-tracker.mddocs/agents/triage-labels.mddocs/research/backup-and-recovery-operations.mddocs/research/github-owner-auth-session-security.mddocs/research/media-storage-upload-architecture.mddocs/research/runtime-content-persistence.mdskills-lock.json
|
|
||
| ## Testing strategy: replace, don't layer | ||
|
|
||
| - Old unit tests on shallow modules become waste once tests at the deepened module's interface exist — delete them. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not delete tests solely because interface tests exist.
Line 34 permits deleting shallow-module tests without proving equivalent coverage. This can remove adapter coverage and edge cases that the deep module interface does not expose. Retire tests only after equivalent behavior is covered, and retain adapter, integration, and unique contract tests.
Suggested fix
- Old unit tests on shallow modules become waste once tests at the deepened module's interface exist — delete them.
+ Retire shallow-module tests only after equivalent behavior is covered through the deepened module's interface.
+ Keep adapter, integration, and unique contract tests.📝 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
| - Old unit tests on shallow modules become waste once tests at the deepened module's interface exist — delete them. | |
| - Retire shallow-module tests only after equivalent behavior is covered through the deepened module's interface. | |
| - Keep adapter, integration, and unique contract tests. |
🤖 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 @.agents/skills/codebase-design/DEEPENING.md at line 34, Revise the testing
guidance in DEEPENING.md so shallow-module tests are not deleted merely because
deepened-module interface tests exist. Require equivalent behavior coverage
before retiring tests, while retaining adapter, integration, and unique contract
tests that the deep interface does not cover.
Comment on lines
+34
to
+52
| ``` | ||
| ┌─────────────────────┐ | ||
| │ Small Interface │ ← Few methods, simple params | ||
| ├─────────────────────┤ | ||
| │ │ | ||
| │ Deep Implementation│ ← Complex logic hidden | ||
| │ │ | ||
| └─────────────────────┘ | ||
| ``` | ||
|
|
||
| **Shallow module** = large interface + little implementation (avoid): | ||
|
|
||
| ``` | ||
| ┌─────────────────────────────────┐ | ||
| │ Large Interface │ ← Many methods, complex params | ||
| ├─────────────────────────────────┤ | ||
| │ Thin Implementation │ ← Just passes through | ||
| └─────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to all fenced ASCII-art blocks.
Both files contain fenced blocks without language identifiers. markdownlint-cli2 reports MD040. Add text to each opening fence.
.agents/skills/codebase-design/SKILL.md#L34-L52: addtextto the two diagram fences..agents/skills/domain-modeling/SKILL.md#L14-L38: addtextto the two directory-tree fences.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
.agents/skills/codebase-design/SKILL.md#L34-L52(this comment).agents/skills/domain-modeling/SKILL.md#L14-L38
🤖 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 @.agents/skills/codebase-design/SKILL.md around lines 34 - 52, Add the text
language identifier to both fenced ASCII-art diagram blocks in
.agents/skills/codebase-design/SKILL.md lines 34-52 and both fenced
directory-tree blocks in .agents/skills/domain-modeling/SKILL.md lines 14-38,
without changing their contents.
Source: Linters/SAST tools
| # step "<instruction>" → show instruction, wait for Enter | ||
| # capture VAR "<question>" → show question, read response into VAR | ||
| # | ||
| # At the end, captured values are printed as KEY=VALUE for the agent to parse. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Preserve multiline error input.
read -r stops at the first newline. If the user pastes a stack trace at Line 35, the script loses the remaining lines while Lines 39-41 emit an incomplete error message. Require a single-line value, or add delimiter-based capture and encode newlines before emitting KEY=VALUE.
Also applies to: 22-27, 35-35, 39-41
🤖 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 @.agents/skills/diagnosing-bugs/scripts/hitl-loop.template.sh at line 13,
Update the input capture and final KEY=VALUE emission in the HITL loop so
multiline stack traces are preserved; either enforce a single-line error value
or use delimiter-based capture and encode newlines before output. Apply this
consistently to the read logic and the error-value handling referenced near the
prompt and emission sections.
Comment on lines
+18
to
+29
| ### Ways to construct one — try them in roughly this order | ||
|
|
||
| 1. **Failing test** at whatever seam reaches the bug — unit, integration, e2e. | ||
| 2. **Curl / HTTP script** against a running dev server. | ||
| 3. **CLI invocation** with a fixture input, diffing stdout against a known-good snapshot. | ||
| 4. **Headless browser script** (Playwright / Puppeteer) — drives the UI, asserts on DOM/console/network. | ||
| 5. **Replay a captured trace.** Save a real network request / payload / event log to disk; replay it through the code path in isolation. | ||
| 6. **Throwaway harness.** Spin up a minimal subset of the system (one service, mocked deps) that exercises the bug code path with a single function call. | ||
| 7. **Property / fuzz loop.** If the bug is "sometimes wrong output", run 1000 random inputs and look for the failure mode. | ||
| 8. **Bisection harness.** If the bug appeared between two known states (commit, dataset, version), automate "boot at state X, check, repeat" so you can `git bisect run` it. | ||
| 9. **Differential loop.** Run the same input through old-version vs new-version (or two configs) and diff outputs. | ||
| 10. **HITL bash script.** Last resort. If a human must click, drive _them_ with `scripts/hitl-loop.template.sh` so the loop is still structured. Captured output feeds back to you. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact captured artifacts before replay or paste.
The workflow tells the agent to replay captured traces and paste command output. Add mandatory sanitization for Authorization, cookies, API keys, passwords, tokens, and PII. Prefer synthetic fixtures for production captures.
Also applies to: 51-58
🤖 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 @.agents/skills/diagnosing-bugs/SKILL.md around lines 18 - 29, Update the
“Replay a captured trace” guidance and the corresponding workflow section around
lines 51-58 to require redacting Authorization headers, cookies, API keys,
passwords, tokens, and PII before replaying or sharing artifacts. Instruct
agents to prefer synthetic fixtures over production captures while preserving
the existing replay workflow.
Comment on lines
+43
to
+45
| ### Non-deterministic bugs | ||
|
|
||
| The goal is not a clean repro but a **higher reproduction rate**. Loop the trigger 100×, parallelise, add stress, narrow timing windows, inject sleeps. A 50%-flake bug is debuggable; 1% is not — keep raising the rate until it's debuggable. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound stress reproduction to an isolated target.
The skill recommends looping triggers 100 times, parallelising, and adding stress. Require a non-production target and explicit caps for iterations, concurrency, and request rate before using these techniques.
🤖 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 @.agents/skills/diagnosing-bugs/SKILL.md around lines 43 - 45, Update the
“Non-deterministic bugs” guidance to require stress reproduction only against an
isolated non-production target, with explicit caps for iterations, concurrency,
and request rate before looping, parallelising, or adding timing stress.
Preserve the goal of increasing reproduction reliability while preventing
uncontrolled load.
|
|
||
| The map is an **index**, not a store. It lists the decisions made and points at the tickets that hold their detail; a decision lives in exactly one place — its ticket — so the map never restates it, only gists it and links. | ||
|
|
||
| **Where the map, its child tickets, blocking, and frontier queries physically live is tracker-specific.** The issue tracker should have been provided to you — run `/setup-matt-pocock-skills` if not. Consult the tracker doc's "Wayfinding operations" section for how _this_ repo expresses them. If no tracker has been provided, default to the local-markdown tracker. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use one tracker default across the skill system.
Line [25] defaults to the local-markdown tracker, while .agents/skills/setup-matt-pocock-skills/SKILL.md documents GitHub as the default. Different defaults can place the map and its tickets in different trackers. Defer to the setup configuration and use the same default everywhere.
🤖 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 @.agents/skills/wayfinder/SKILL.md at line 25, Update the tracker fallback in
the Wayfinder skill’s tracker guidance to match the default established by
setup-matt-pocock-skills, rather than defaulting independently to
local-markdown. Preserve the instruction to consult the provided tracker’s
“Wayfinding operations” section, and ensure all skill documentation uses the
same setup-configured default.
Comment on lines
+122
to
+128
| 1. Load the **map** — the low-res view, not every ticket body. | ||
| 2. Choose the ticket. If the user named one, use it. Otherwise take the first frontier ticket in order. **Claim it**: assign it to yourself before any work. | ||
| 3. Resolve it — **zoom as needed**: fetch the full body of any related or closed ticket on demand; invoke the skills the `## Notes` block names. If in doubt, use `/grilling` and `/domain-modeling`. | ||
| 4. Record the resolution: post the answer as a **resolution comment**, **close** the issue, and **append a context pointer** to the map's Decisions-so-far. | ||
| 5. Add newly-surfaced tickets (create-then-wire); graduate any fog the answer has made specifiable, clearing each graduated patch from **Not yet specified** so it lives only as its new ticket. If the answer reveals a ticket — this one or another — sits beyond the destination, **rule it out of scope** rather than resolving it on the route. If the decision invalidates other parts of the map, update or delete those tickets. | ||
|
|
||
| The user may run unblocked tickets in parallel, so expect other sessions to be editing the tracker concurrently. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '1,180p' .agents/skills/wayfinder/SKILL.md 2>/dev/null || true
echo
echo "== relevant searches =="
rg -n -i 'claim|assign|assigns|assignment|claiming|Decisions|decisions|concurrent|parallel|optimistic|serialized|map|ticket' .agents/skills/wayfinder -S || trueRepository: alantothe/minimal-portfolio
Length of output: 24019
🏁 Script executed:
#!/bin Bash
set -euo pipefail
echo "== tracked files containing wayfinder/setup/matt =="
git ls-files | rg -i 'wayfinder|matt|pocock|operations|tracker|skill|README|AGENTS|context|prompt' || true
echo
echo "== precise tracker/wayfinder doc searches =="
rg -n -i 'Wayfinding operations|wayfinder operations|atomic|conditional|assign_if|assign.*unassigned|claim|claiming|optimistic|etag|version|sha|lock|serialized|sequence|Decisions so far|append to|append a .*Decisions|append a context pointer|context pointer' $(git ls-files) -S || true
echo
echo "== setup docs around setup-matt-pocock-skills =="
rg -n -i 'setup-matt-pocock-skills|Matt P|pocock' $(git ls-files) -S || trueRepository: alantothe/minimal-portfolio
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs/agents/issue-tracker.md lines 1-80 =="
sed -n '1,80p' docs/agents/issue-tracker.md
echo
echo "== focused concise matches in tracked wayfinder docs =="
rg -n -i 'Assign|assigning|assigned|claim|resolution comment|Decisions-so-far|Decisions so far|append|serialized|optimistic|concurrent|parallel|atomic|read.*after|conditional|If assignee' .agents docs/agents/skills setup-matt-pocock-skills -S || trueRepository: alantothe/minimal-portfolio
Length of output: 15338
Make ticket claims and map appends concurrency-safe.
The Work-through-the-map step permits parallel sessions, but gh issue edit --add-assignee @me`` and appending to Decisions-so-far are read-then-write operations. Two sessions selected from the same frontier can claim the same open unassigned ticket; concurrent map appends can also overwrite each other. Use a conditional assignment that accepts only a currently unassigned claimed ticket, re-read assignment before and after work, and apply optimistic checking or serialized map writes when appending to Decisions so far.
🤖 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 @.agents/skills/wayfinder/SKILL.md around lines 122 - 128, Update the
Work-through-the-map procedure to make ticket claims conditional: assign only
when the selected ticket is still unassigned, then re-read and verify ownership
before and after resolution, abandoning the ticket if another session claimed
it. Make the Decisions-so-far append concurrency-safe by using optimistic
conflict detection or serialized read-modify-write updates so concurrent appends
are preserved.
Comment on lines
+7
to
+9
| A skill exists to wrangle determinism out of a stochastic system. **Predictability** — the agent taking the same _process_ every run, not producing the same output — is the root virtue; every lever below serves it. | ||
|
|
||
| **Bold terms** are defined in [`GLOSSARY.md`](GLOSSARY.md); look them up there for the full meaning. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a top-level Markdown heading after the frontmatter.
Line 7 starts the document body with prose, so markdownlint MD041 reports this file. Add # Writing Great Skills before the opening paragraph.
Proposed heading
---
+# Writing Great Skills
+
A skill exists to wrangle determinism out of a stochastic system.📝 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
| A skill exists to wrangle determinism out of a stochastic system. **Predictability** — the agent taking the same _process_ every run, not producing the same output — is the root virtue; every lever below serves it. | |
| **Bold terms** are defined in [`GLOSSARY.md`](GLOSSARY.md); look them up there for the full meaning. | |
| # Writing Great Skills | |
| A skill exists to wrangle determinism out of a stochastic system. **Predictability** — the agent taking the same _process_ every run, not producing the same output — is the root virtue; every lever below serves it. | |
| **Bold terms** are defined in [`GLOSSARY.md`](GLOSSARY.md); look them up there for the full meaning. |
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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 @.agents/skills/writing-great-skills/SKILL.md around lines 7 - 9, Add the
top-level Markdown heading “Writing Great Skills” immediately after the
frontmatter and before the opening paragraph in SKILL.md, ensuring it is the
document’s first body element.
Source: Linters/SAST tools
| @@ -0,0 +1,17 @@ | |||
| # Agent Instructions | |||
|
|
|||
| Always run the `caveman` skill to save tokens. | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
matches="$(
fd -H -a -t d -t l '^caveman$' .agents/skills .claude/skills 2>/dev/null || true
)"
if [ -z "$matches" ]; then
echo "caveman skill is not discoverable" >&2
exit 1
fi
printf '%s\n' "$matches"Repository: alantothe/minimal-portfolio
Length of output: 199
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Files named AGENTS.md:\n'
fd -H -a '^AGENTS\.md$' .
printf '\nAGENTS.md content:\n'
if [ -f AGENTS.md ]; then
cat -n AGENTS.md
fi
printf '\nSkill-related directories/files:\n'
fd -H -E '^.git$' . | rg -n '(^|/)(\.?agents|\.?claude)/.*|skills|caveman|AGENTS\.md$' | head -200
printf '\nSearch for caveman skill references:\n'
rg -n --hidden --glob '!.git/**' --text 'caveman' . || trueRepository: alantothe/minimal-portfolio
Length of output: 6586
Remove the mandatory caveman prerequisite.
The repository does not define or reference a caveman skill outside this instruction. Keep this line only if the skill is added and discoverable; otherwise remove it or make it conditional to avoid giving agents an impossible prerequisite.
🤖 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 `@AGENTS.md` at line 3, Remove the mandatory “Always run the caveman skill”
instruction from AGENTS.md, since no discoverable caveman skill exists; only
retain it if the repository adds and exposes that skill.
| - `allow_signup=false`; | ||
| - `prompt=select_account`. | ||
|
|
||
| The `__Host-` prefix requires `Secure`, `Path=/`, and no `Domain`, keeping the cookie host-only ([Cookies: HTTP State Management Mechanism](https://www.rfc-editor.org/rfc/rfc10025.html), [MDN: Set-Cookie](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Set-Cookie#cookie_prefixes)). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What RFC defines the __Host- and __Secure- cookie name prefixes?
💡 Result:
The Host- and Secure- cookie name prefixes are defined in the document titled "Cookies: HTTP State Management Mechanism," which is currently maintained as the "rfc6265bis" draft by the IETF HTTP Working Group [1][2][3]. These prefixes were originally introduced in a dedicated proposal titled "Cookie Prefixes" (draft-ietf-httpbis-cookie-prefixes) [4][5], which was subsequently incorporated into the updated HTTP State Management Mechanism specification (often referred to as RFC 6265bis) that is intended to supersede the original RFC 6265 [1][2][6]. As of July 31, 2026, the definitions remain a central part of this ongoing specification update [1][2]: - The Secure- prefix requires that a cookie be set with the Secure attribute from a secure origin [1][2][7]. - The Host- prefix mandates that a cookie be set with the Secure attribute, have a Path attribute of /, and must not contain a Domain attribute (ensuring it is host-only and cannot span subdomains) [1][2][7].
Citations:
- 1: https://www.ietf.org/archive/id/draft-ietf-httpbis-rfc6265bis-22.html
- 2: https://httpwg.org/http-extensions/draft-ietf-httpbis-connect-tcp-11/draft-ietf-httpbis-rfc6265bis.html
- 3: https://httpwg.org/http-extensions/draft-ietf-httpbis-rfc6265bis.html
- 4: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cookie-prefixes
- 5: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cookie-prefixes-00
- 6: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-10
- 7: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Set-Cookie
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Target line context =="
sed -n '65,75p' docs/research/github-owner-auth-session-security.md || true
echo
echo "== RFC link occurrences in target document =='
rg -n "rfc[0-9]+" docs/research/github-owner-auth-session-security.md || true
echo
echo "== RFC 10025 availability via RFC editor? =="
curl -sS -o /dev/null -w "%{http_code}\n" https://www.rfc-editor.org/rfc/rfc10025.html || true
curl -sS -o /dev/null -w "%{http_code}\n" https://www.rfc-editor.org/rfc/rfc6265.html || trueRepository: alantothe/minimal-portfolio
Length of output: 921
🌐 Web query:
RFC 6265bis Set-Cookie prefixes cookie-prefixes section 5
💡 Result:
In RFC 6265bis, cookie name prefixes serve to provide servers with confidence that a cookie was set with specific security attributes in a backward-compatible manner [1][2]. These prefixes, defined in section 4.1.3 and enforced via requirements in section 5.4, introduce two main types [1][2][3]: The Secure- prefix: If a cookie name begins with this string (case-sensitive for servers, case-insensitive for user agents), the cookie must be set with the Secure attribute [1][2][3]. The Host- prefix: If a cookie name begins with this string, it must meet three conditions: it must be set with the Secure attribute, have a Path attribute set to / (the root path), and must not include a Domain attribute [1][2][3]. This configuration binds the cookie to the specific host, preventing it from spanning subdomains and ensuring it is effective host-wide [1][2][3]. Section 5.4 specifies that user agents must match these prefix strings case-insensitively to prevent issues where variations (e.g., Secure- vs secure-) might otherwise allow bypasses [2][4][3]. If a cookie with a reserved prefix does not meet these mandated requirements, user agents are required to reject the cookie [4][3][5].
Citations:
- 1: https://httpwg.org/http-extensions/draft-ietf-httpbis-rfc6265bis.txt
- 2: https://httpwg.org/http-extensions/draft-ietf-httpbis-rfc6265bis.html
- 3: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis/
- 4: https://greenbytes.de/tech/webdav/draft-ietf-httpbis-rfc6265bis-14.txt
- 5: https://datatracker.ietf.org/doc/draft-ietf-httpbis-rfc6265bis/19/
Fix the cookie prefix RFC link.
rfc10025 is not the cookie specification, and the __Host- prefix requirements come from the cookie-prefix update documented as RFC 6265bis (draft-ietf-httpbis-rfc6265bis) rather than the original RFC 6265 text. Use a stable/current RFC 6265bis RFC-Editor link or remove this RFC label.
🤖 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 `@docs/research/github-owner-auth-session-security.md` at line 71, Update the
RFC citation in the cookie prefix explanation, replacing the incorrect rfc10025
link with a stable/current RFC 6265bis RFC-Editor link, or remove the RFC label
while retaining the MDN reference.
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.
Brings the output of the Plan secure owner content editing and publishing wayfinder map onto
main.That map is complete — all nine decision tickets resolved, nothing left unspecified — but every artifact it produced was stranded on unmerged branches with no PR ever opened. That included
docs/agents/issue-tracker.md, so a fresh agent session had no way to discover which issue tracker this repo uses.What lands
AGENTS.md,docs/agents/{domain,issue-tracker,triage-labels}.md,skills-lock.json,.agents/skills/with.claude/skills/symlinksCONTEXT.md— base vocabulary plus the editable-content and publication-lifecycle terms added while working the mapdocs/research/— four docs backing #29, #30, #31, #35Six branches are consolidated here rather than merged one at a time. They all shared the same three setup commits, so under squash-merge each subsequent branch would have needed
mainmerged back into it before it could land — six review and CI cycles for what is one coherent set of planning documents.Incidental fix
.gitignoreignoreddocs/*and un-ignored onlydocs/agents/anddocs/adr/. The four research docs survived solely because they were already tracked; a new research doc would have been silently skipped bygit add. Added!docs/research/to the same block. Verified that new research docs are now stageable and otherdocs/content stays ignored.Scope
Planning artifacts only — no runtime code changes, no behaviour change to the public site.
prototype/owner-workspace-flowis deliberately left unmerged, since it adds throwaway prototype routes tosrc/server/.🤖 Generated with Claude Code
Summary by CodeRabbit