Skip to content

[ARCHIVED] Benchmark issue 481#481

Closed
guyoron1 wants to merge 2 commits into
mainfrom
lsp-test/480-document-agents-repo-migration
Closed

[ARCHIVED] Benchmark issue 481#481
guyoron1 wants to merge 2 commits into
mainfrom
lsp-test/480-document-agents-repo-migration

Conversation

@guyoron1

Copy link
Copy Markdown
Owner

Closes #480

LSP experiment v2: clean first-review with gopls-lsp enabled.
Code identical to PR #420 (issue #371).
Branch created from commit 8bcd31d.

Add "Agent definitions and skills" section to AGENTS.md noting that
agent definitions, skills, harness configs, and related content
have moved to fullsend-ai/agents. The copies under
internal/scaffold/fullsend-repo/ are stale and will be removed.

Add deprecation README.md files to the three affected scaffold
directories (agents/, skills/, harness/) so contributors see
the migration notice before editing.

Closes #371
The harness scaffold files are still embedded by baseurl.go, unlike
agents/ and skills/ which are genuinely stale. Update the deprecation
notice to say "being migrated" and note the Go code dependency.

Addresses review feedback on #420
@guyoron1

Copy link
Copy Markdown
Owner Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:20 AM UTC · Completed 10:28 AM UTC
Commit: 758c511 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] AGENTS.md — This file is a protected path requiring human approval. The PR links to issue [ARCHIVED] Benchmark issue 480 #480 and explains the rationale (documenting agent/skill content migration to fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [stale-doc] docs/guides/user/customizing-agents.md:158 — Pre-existing reference to internal/scaffold/fullsend-repo/harness/code.yaml via a raw GitHub URL. The path still works (harness files are still embedded by Go code per internal/scaffold/baseurl.go) but will become stale once the harness migration completes. Consider updating in a follow-up PR.

  • [stale-doc] docs/agents/README.md:5 — Pre-existing reference to internal/scaffold/fullsend-repo/harness/ with a relative link. The path still contains the harness files but the canonical source is moving to fullsend-ai/agents. Consider updating in a follow-up PR.

Previous run

Review

Verdict: Comment — medium-severity findings worth noting; none blocking.

This PR adds documentation in AGENTS.md and deprecation READMEs to signal that agent/skill/harness content has migrated to fullsend-ai/agents. The scope matches the authorized work in #480 and the change is well-structured. Two items deserve attention.

Findings

1. Contradictory guidance for harness/ between AGENTS.md and harness/README.md — medium

AGENTS.md adds a blanket prohibition: "Do not modify files under internal/scaffold/fullsend-repo/agents/, internal/scaffold/fullsend-repo/skills/, or internal/scaffold/fullsend-repo/harness/".

But harness/README.md says: "These files are still embedded by Go code (internal/scaffold/baseurl.go), so changes here remain necessary until the Go references are updated."

These two instructions directly contradict each other for the harness/ directory. An agent following the AGENTS.md prohibition (which has higher authority as a top-level instruction file) will refuse to make legitimate harness updates that the README acknowledges are still needed.

Verified: internal/scaffold/scaffold.go:11 has //go:embed all:fullsend-repo, which embeds the entire fullsend-repo directory — including agents/ and skills/, not just harness/. The agents/ and skills/ files are also referenced by Go test code (scaffold_test.go accesses agents/triage.md, agents/code.md, skills/code-implementation/SKILL.md). The asymmetric treatment (only harness/ gets the Go-embed caveat) does not reflect the technical reality.

Suggestion: Either carve harness/ out of the AGENTS.md prohibition (e.g., "Do not modify files under .../agents/ or .../skills/ — make changes in fullsend-ai/agents instead. Files under .../harness/ are still embedded by Go code and may need updates until references are migrated."), or add the same Go-embedding caveat to the agents/ and skills/ READMEs.

2. 30+ stale references to deprecated paths in other documentation — medium

Multiple documentation files still reference the deprecated internal/scaffold/fullsend-repo/{agents,skills,harness}/ paths without noting the migration:

  • docs/agents/README.md, docs/agents/code.md, docs/agents/fix.md, docs/agents/review.md, docs/agents/triage.md, docs/agents/prioritize.md, docs/agents/retro.md — link to harness YAML files at the old paths
  • docs/guides/user/customizing-agents.md — references raw GitHub URL with old path
  • docs/ADRs/0045-forge-portable-harness-schema.md — multiple references
  • docs/plans/adr-0045-forge-portable-harness-phase{1,2}.md — multiple references
  • docs/superpowers/plans/2026-05-04-retro-agent.md — references all three deprecated directories

This is outside the scope authorized by #480, but worth tracking as follow-up — agents reading these docs will still be directed to the old paths, partially undermining the PR's intent.

3. Root README.md does not mention the migration — low

The root README.md documents the repo structure but does not reference the migration to fullsend-ai/agents. Users and agents discovering the project via README will not know about the move.

4. Asymmetric deprecation language across READMEs — low

The agents/ and skills/ READMEs say content "has moved" and "will be removed," while harness/README.md says content "is being migrated" and changes "remain necessary." The //go:embed all:fullsend-repo directive embeds all three directories equally. If the intent is that agents/skills are truly frozen while harness is not, the AGENTS.md prohibition needs the same carve-out (see Finding 1).

Security

No secrets, injection patterns, Unicode steganography, permission changes, or prompt injection detected.

Scope

The PR matches the authorized scope of #480. No scope creep detected — the four files changed are exactly what the issue proposed.


Labels: PR adds documentation about content migration and deprecation notices

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

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:28 AM UTC · Completed 10:40 AM UTC
Commit: fa6bdf7 · 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 13, 2026
@guyoron1 guyoron1 closed this Jul 14, 2026
@guyoron1 guyoron1 changed the title [LSP] [RTK+Ponytail] AGENTS.md: document that agent/skill content has moved to fullsend-ai/agents [ARCHIVED] Benchmark issue 481 Jul 14, 2026
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] Benchmark issue 480

1 participant