Skip to content

docs(#243): add API pattern replication guidance to AGENTS.md #264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fullsend-ai-coder wants to merge 2 commits into
base: main
Choose a base branch
from
Open

docs(#243): add API pattern replication guidance to AGENTS.md #264

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd27241
docs(#243): add API pattern replication guidance to AGENTS.md
fullsend-ai-coder[bot] Jul 10, 2026
978e416
fix(#243): address review feedback on PR #264
fullsend-ai-coder[bot] Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ All git forge operations (GitHub API calls, PR comments, issue creation, workflo

**When reviewing PRs:** Flag any direct `exec.Command("gh", ...)`, raw GitHub API calls, or other forge-specific operations outside `internal/forge/github/` as a medium-severity or higher finding. This is an architectural violation, not a style preference.

## API pattern replication

@fullsend-ai-review fullsend-ai-review Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] correctness

retryOnRepoRace does not exist anywhere in the repository. Only retryOnTransient exists (internal/forge/github/github.go:561). Referencing a fabricated identifier in agent-facing guidance will cause confusion.

Suggested fix: Remove retryOnRepoRace from the example list. The parenthetical should read: retry logic (e.g., retryOnTransient).

@fullsend-ai-review fullsend-ai-review Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] correctness

The claim that several methods share the Git Trees API pattern is inaccurate. Only commitFilesTo (line 647) implements the full refs-commit-tree pattern. CommitFiles and CommitFilesToBranch are thin wrappers. The truncation check already covers all callers by design.

Suggested fix: Reword to reflect the current architecture, e.g.: the Git Trees API pattern currently centralized in commitFilesTo — new methods using the same endpoint should replicate its truncation handling.

@fullsend-ai-review fullsend-ai-review Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] style-consistency

The new section lacks bold structural markers (e.g., When writing code:, When reviewing PRs:) that the adjacent Forge abstraction section uses for guidance categories.

Suggested fix: Optional — consider adding a bold lead-in like When implementing a new method: for consistency with adjacent sections.


**When implementing a new method** that calls the same API endpoint or uses the same multi-step API pattern as an existing method in the file, read the existing implementation first. Replicate its error handling (e.g., truncation checks, 404-to-ErrNotFound mapping), retry logic (e.g., `retryOnTransient`), and response validation. The `internal/forge/github/github.go` file has the Git Trees API pattern (refs → commit → tree) currently centralized in `commitFilesTo` — new methods using the same endpoint should replicate its truncation handling.

## Architecture Decision Records (ADRs)

These rules apply whenever you touch `docs/ADRs/` or review a PR that does. Full authoring guidance is in [`skills/writing-adrs/SKILL.md`](skills/writing-adrs/SKILL.md); invoke that skill when writing a new ADR.
Expand Down
Loading