Skip to content

docs(#189): add API pattern replication guidance to AGENTS.md - #205

Closed
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/189-api-pattern-replication-guidance
Closed

docs(#189): add API pattern replication guidance to AGENTS.md#205
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/189-api-pattern-replication-guidance

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown

Add a new section after "Forge abstraction" instructing agents to read existing sibling methods before implementing new ones that call the same API endpoint or use the same multi-step pattern. The guidance covers replicating error handling (truncation checks, 404-to-ErrNotFound mapping), retry logic (retryOnTransient), and response validation, with the Git Trees API pattern in internal/forge/github/github.go as a concrete example.


Closes #189

Post-script verification

  • Branch is not main/master (agent/189-api-pattern-replication-guidance)
  • Secret scan passed (gitleaks — f96750babbed5ada406a9ae04e8068449701d9c7..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add a new section after "Forge abstraction" instructing agents to
read existing sibling methods before implementing new ones that
call the same API endpoint or use the same multi-step pattern.
The guidance covers replicating error handling (truncation checks,
404-to-ErrNotFound mapping), retry logic (retryOnTransient), and
response validation, with the Git Trees API pattern in
internal/forge/github/github.go as a concrete example.

Closes #189
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review — Approve

PR: #205docs(#189): add API pattern replication guidance to AGENTS.md
Author: fullsend-ai-coder[bot] · Base: main · Head: 7a7b13e

Summary

This PR adds a new "API pattern replication" section to AGENTS.md, instructing agents to read existing sibling methods before implementing new ones that call the same API endpoint, and to replicate error handling, retry logic, and response validation. It also adds a corresponding "When reviewing PRs" directive. The change closes issue #189.

Dimensions reviewed

Dimension Result Notes
Correctness ✅ Pass All technical claims verified against source — commitFilesTo (L647), truncation handling (L697-703), retryOnTransient (L561, 5 call sites), 404→ErrNotFound mapping (L82-84)
Security ✅ Pass Docs-only change; no secrets, injection patterns, Unicode steganography, or permission changes
Intent & coherence ✅ Pass Traces to authorized issue #189; scope matches precisely; placement after "Forge abstraction" is architecturally appropriate; docs(#189) prefix correct per COMMITS.md
Style & conventions ✅ Pass Section heading, bold-prefix pattern, tone, and severity characterization ("correctness issue, not a style preference") all follow established AGENTS.md conventions
Cross-repo contracts ⏭ Skipped No exported interfaces or API schemas modified
Docs currency ⏭ Skipped Change is itself documentation; no code changes requiring doc updates

Verdict

Clean across all reviewed dimensions. The new guidance is technically accurate, properly scoped to the linked issue, stylistically consistent with the surrounding content, and well-placed in the document structure. No findings.


Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

  • AGENTS.md
Previous run

Review — PR #205

Verdict: Approve

Summary

This PR adds a 4-line "API pattern replication" section to AGENTS.md, placed after the existing "Forge abstraction" section. It instructs agents to read existing sibling methods before implementing new ones that share the same API endpoint or multi-step pattern, and to replicate their error handling, retry logic, and response validation. The Git Trees API pattern in internal/forge/github/github.go is cited as a concrete example.

Verified claims

All technical references in the new section were verified against the codebase:

Claim Status
retryOnTransient exists in forge/github ✅ 8 occurrences
Truncation handling exists Truncated field + conditional check
404-to-ErrNotFound mapping exists ✅ Multiple call sites
Git Trees API pattern (refs → commit → tree) exists commitFilesTo at line 647
retryOnRepoRace (in issue proposal, omitted from PR) ✅ Correctly omitted — not in codebase

Dimension assessment

  • Correctness: The documentation accurately describes existing patterns. No factual errors.
  • Security: Documentation-only change. No code, secrets, or RBAC impact. No injection patterns detected in PR body or linked issue.
  • Intent & coherence: Directly addresses issue [ARCHIVED] Original issue 189 #189 (human-authored by repo owner). Scope is minimal and appropriate. Placement after "Forge abstraction" is thematically correct.
  • Style/conventions: Section heading style (##), inline code formatting, and imperative tone match the rest of AGENTS.md.
  • Documentation currency: N/A — this is the documentation change; references are current.
  • Cross-repo contracts: No API surface or schema changes.

Notes

  • AGENTS.md is covered by CODEOWNERS (* @fullsend-ai/core), so human approval is required before merge. The requires-manual-review label is already applied.
  • The PR was authored by fullsend-ai-coder[bot] in response to a human-filed issue ([ARCHIVED] Original issue 189 #189 by the repo owner), which provides clear authorization for this change.

Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

  • AGENTS.md
Previous run (2)

Review

Verdict: Approve — no blocking findings. Two low-severity observations noted below.

Scope: 4 lines added to AGENTS.md — new ## API pattern replication section placed after "Forge abstraction" and before "Architecture Decision Records." Closes #189.

What was checked

Dimension Result
Correctness Technical claims verified against internal/forge/github/github.go. retryOnTransient (L561), 404→ErrNotFound mapping (L82-85), and truncation handling (L697-704) all exist. PR correctly omits retryOnRepoRace from the issue's proposed text — that function does not exist in the codebase.
Security Clean. No injection patterns, hidden instructions, bidirectional overrides, or secrets. Only non-ASCII: (U+2192) and (U+2014), both standard typographic characters.
Intent & coherence Faithfully implements issue #189's request. Single-file change, no scope creep. Placement is architecturally sound — extends forge-related guidance before ADR rules.
Style & conventions ## heading level matches siblings. Backtick usage, em-dash style, and parenthetical e.g., patterns are consistent with surrounding content.
Docs currency All referenced identifiers exist in the codebase. No other docs rendered stale by this addition.
Cross-repo contracts N/A — no exported interfaces or API surfaces modified.

Findings

1. "Several methods" overstates current state

Severity: Low · Category: technical-accuracy · File: AGENTS.md

"The internal/forge/github/github.go file has several methods that share the Git Trees API pattern (refs → commit → tree)"

Only one method — commitFilesTo (L647) — implements the full refs → commit → tree pattern with truncation handling. CommitFiles and CommitFilesToBranch are thin wrappers that delegate to it. The phrasing "several methods" implies multiple independent implementations that could drift, which is not the current state.

The guidance is still sound as a forward-looking rule (future methods should replicate the pattern), but the concrete example weakens credibility because a reader checking the code will find one shared implementation, not several. Consider rewording to: "...has a method (commitFilesTo) that implements the Git Trees API pattern (refs → commit → tree) with truncation handling — any new method using this pattern should replicate it." Alternatively, the Contents API pattern (CreateOrUpdateFile, DeleteFile, putFileWithRetry) offers a better example of genuinely independent sibling methods sharing retryOnTransient.

2. No "When reviewing PRs" counterpart

Severity: Low · Category: document-consistency · File: AGENTS.md

The two comparable sections — "Forge abstraction" and "Architecture Decision Records" — both include a bold When reviewing PRs: paragraph that tells reviewers what to flag. The new section provides implementation guidance but no review instruction. Adding something like "When reviewing PRs: Flag new forge methods that omit error handling or retry patterns present in existing methods using the same API endpoint" would close the loop and make the rule enforceable during review, not just during authoring.


Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

  • AGENTS.md
Previous run (3)

Review — comment

PR: docs(#189): add API pattern replication guidance to AGENTS.md
Scope: 1 file changed, 4 additions — documentation only

This PR adds an "API pattern replication" section to AGENTS.md, instructing agents to read existing sibling methods before implementing new ones that call the same API endpoint. The change traces to issue #189, which was created by the repo owner after a retro on PR fullsend-ai#2360 where a missing truncation check required a full fix iteration.

Findings

1. Technical accuracy — "several methods" claim · medium

File: AGENTS.md, line 67

The new text states: "The internal/forge/github/github.go file has several methods that share the Git Trees API pattern (refs → commit → tree)."

I verified the source: only commitFilesTo (line 647) uses the refs → commit → tree pattern. CommitFiles and CommitFilesToBranch are thin wrappers that delegate to it — they don't independently implement the pattern. "Several methods" overstates the current state and could confuse agents trying to locate them.

The wording matches issue #189's proposed text verbatim, so this inaccuracy originates from the issue rather than the code agent. A more accurate phrasing would be:

"The internal/forge/github/github.go file's commitFilesTo method uses the Git Trees API pattern (refs → commit → tree) with truncation handling — any new method using the same endpoint should replicate that handling."

2. Agent modifying agent guardrails · medium

File: AGENTS.md

AGENTS.md line 82 states: "CODEOWNERS files are always human-owned. Agents cannot modify their own guardrails." AGENTS.md is the primary document governing agent behavior in this repo, making it a guardrail file. This PR is authored by fullsend-ai-coder[bot], an autonomous agent.

Mitigating factors:

  • Issue [ARCHIVED] Original issue 189 #189 was created by the repo owner (guyoron1) with the exact proposed text
  • CODEOWNERS (* @fullsend-ai/core) requires human approval to merge
  • The change adds a new rule (does not weaken existing guardrails)
  • The architectural principle may be scoped specifically to CODEOWNERS files (the enforcement mechanism), not all guidance documents

This is worth flagging for human consideration — the principle could reasonably be interpreted either way.

3. Formatting consistency · low

File: AGENTS.md, line 66

The adjacent "Forge abstraction" section uses bold lead-ins (**Prohibited outside...:**, **When writing code:**, **When reviewing PRs:**) to break guidance into scannable chunks. The new section is a single prose paragraph without structural markers. Consider adding bold lead-ins for consistency:

When implementing: ...

When reviewing PRs: ...

Summary

The change is well-scoped and addresses a real gap identified in issue #189. The two medium findings are the factual inaccuracy in "several methods" (easily fixable) and the process question of a bot proposing changes to its own governing document (mitigated by human authorization and CODEOWNERS enforcement). Neither should block the PR, but both merit human attention before merging.


Labels: PR adds documentation guidance to AGENTS.md.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment documentation Improvements or additions to documentation labels Jul 8, 2026
@guyoron1

guyoron1 commented Jul 8, 2026

Copy link
Copy Markdown
Owner

/fs-review

@guyoron1

Copy link
Copy Markdown
Owner

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:19 AM UTC · Completed 10:23 AM UTC
Commit: a2afa8b · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 11, 2026
@guyoron1

Copy link
Copy Markdown
Owner

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 11, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 11:12 AM UTC · Completed 11:19 AM UTC
Commit: a2afa8b · View workflow run →

- Replace 'several methods' with specific reference to commitFilesTo,
  which is the only method implementing the full Git Trees API pattern
- Add 'When reviewing PRs' paragraph for consistency with adjacent
  Forge abstraction and ADR sections

Addresses review feedback on #205
@fullsend-ai-coder

Copy link
Copy Markdown
Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 2 review findings: (1) fixed factual inaccuracy where 'several methods' overstated the number of independent Git Trees API implementations — only commitFilesTo has the full pattern, (2) added 'When reviewing PRs:' paragraph for document consistency with adjacent sections. Both findings were verified against internal/forge/github/github.go.

Fixed (2):

  1. "Several methods" overstates current state (AGENTS.md): Replaced 'has several methods that share the Git Trees API pattern' with 'commitFilesTo method uses the Git Trees API pattern' — only commitFilesTo implements the full refs-to-commit-to-tree pattern; CommitFiles and CommitFilesToBranch are thin wrappers
  2. No "When reviewing PRs" counterpart (AGENTS.md): Added 'When reviewing PRs:' paragraph instructing reviewers to flag new forge methods that omit error handling or retry patterns present in existing sibling methods, matching the structure of adjacent Forge abstraction and ADR sections

Tests: passed

Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:25 AM UTC · Completed 11:31 AM UTC
Commit: a2afa8b · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 11, 2026
@guyoron1 guyoron1 closed this Jul 12, 2026
@guyoron1
guyoron1 deleted the agent/189-api-pattern-replication-guidance branch July 12, 2026 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ARCHIVED] Original issue 189

1 participant