docs: pull Agents docs directly - #5973
Conversation
|
🤖 Finished Review · ✅ Success · Started 10:31 AM UTC · Completed 10:50 AM UTC |
Site previewPreview: https://cc4e81b1-site.fullsend-ai.workers.dev Commit: |
PR Summary by QodoDocs: fetch Agents docs during site build and auto-generate sidebar
AI Description
Diagram
High-Level Assessment
Files changed (17)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1.
|
ReviewFindingsLow
Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewFindingsMedium
Low
Next steps:
Previous run (3)ReviewFindingsHigh
Medium
Low
Labels: PR modifies documentation infrastructure and content sourcing Next steps:
|
05430d6 to
070bcb9
Compare
|
🤖 Finished Review · ✅ Success · Started 11:54 AM UTC · Completed 12:14 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Review sweep findings below (4 new issues; 2 additional candidate findings were already substantively covered by earlier automated review comments on this PR and are omitted here — the issue #5710 submodule/rebuild-trigger divergence, and the absolute-vs-relative agent-doc link inconsistency).
070bcb9 to
796bcaf
Compare
|
🤖 Finished Review · ✅ Success · Started 6:56 AM UTC · Completed 7:12 AM UTC |
Signed-off-by: Hector Martinez <hemartin@redhat.com>
796bcaf to
c015ab0
Compare
|
🤖 Finished Review · ✅ Success · Started 10:34 AM UTC · Completed 10:54 AM UTC Commit: |
Superseded by updated review
| fi | ||
|
|
||
| REF="${FULLSEND_AGENTS_REF:-"main"}" | ||
|
|
There was a problem hiding this comment.
[low] supply-chain
FULLSEND_AGENTS_REF defaults to main, a mutable ref. Unlike the previous approach where agent docs were committed directly in this repository, the fetched documentation content can now change without any auditable change in this repository. Risk is limited by same-org ownership and content being documentation only.
Suggested fix: Pin FULLSEND_AGENTS_REF to a specific commit SHA in the CI workflow, or document the explicit decision to track main.
|
|
||
| rm -rf "$STAGING" | ||
| cp -a "$WORKDIR/docs" "$STAGING" | ||
| rm -rf "$DEST" |
There was a problem hiding this comment.
[low] error-handling
The rm -rf / mv sequence is not atomic: if mv fails after rm, docs/agents/ is deleted with no replacement. The consequence is a failed build that can be re-run, not silent breakage, but the previous content is unrecoverable without re-running.
| git -C "$WORKDIR" fetch --depth 1 "https://github.com/${REPO}.git" "$REF" | ||
| git -C "$WORKDIR" checkout -q FETCH_HEAD | ||
|
|
||
| rm -rf "$STAGING" |
There was a problem hiding this comment.
[low] path-traversal
cp -a preserves symlinks. A compromised source repo could include symlinks pointing outside the destination directory. Risk is limited by same-org ownership.
Suggested fix: Use cp -rL instead of cp -a to dereference symlinks.
| { text: "Prioritize", link: "/agents/prioritize" }, | ||
| { text: "Default vs. Custom", link: "/agents/topics/default-vs-custom" }, | ||
| ], | ||
| items: getMarkdownFiles("agents", "agents"), |
There was a problem hiding this comment.
[low] edge-case
getMarkdownFiles agents agents reads docs/agents/ dynamically at build time from an external repo whose structure is not controlled by this PR. If the external repo introduces subdirectories with README.md files covered by srcExclude, sidebar links would 404. Currently not a problem but represents fragile coupling.
waynesun09
left a comment
There was a problem hiding this comment.
Review sweep: 1 new finding skipped as an already-covered duplicate (the incomplete docs/guides/user/how-to-upgrade-renovate.md stub was already flagged by an earlier automated review comment on this PR). 2 new findings posted inline below.
| git -C "$WORKDIR" checkout -q FETCH_HEAD | ||
|
|
||
| rm -rf "$STAGING" | ||
| cp -a "$WORKDIR/docs" "$STAGING" |
There was a problem hiding this comment.
[HIGH] Fetched agent docs link to harness/policies/scripts dirs that are never fetched — dead links on production site
This script only copies the docs/ subfolder of fullsend-ai/agents (cp -a "$WORKDIR/docs" "$STAGING"). Verified against the live fullsend-ai/agents repo: docs/code.md, docs/fix.md, docs/prioritize.md, docs/retro.md, docs/triage.md, docs/scribe.md, and docs/review.md all contain relative links like [\harness/code.yaml`](../harness/code.yaml)and`scripts/lib/pr-assignee.lib.sh`, and docs/network-policy.mdlinks../policies/, ../policies/code.yaml, ../policies/fix.yaml — all pointing at sibling directories (harness/, policies/, scripts/) that are never copied. Once rendered at /docs/agents/on fullsend.sh, these resolve to nonexistent paths like/docs/harness/code.yaml(404).website/.vitepress/config.tshasignoreDeadLinks: true, so CI will not catch this. This is distinct from the already-resolved absolute-vs-relative link thread for fullsend's own cross-references into docs/agents/` — this is about dead links baked into the vendored content itself once it lands on the live site.
Suggestion: Rewrite these relative links during fetch (e.g. a sed pass turning ../harness/*, ../policies/*, ../scripts/* into absolute https://github.com/fullsend-ai/agents/blob/main/... URLs), or coordinate with fullsend-ai/agents to keep those links absolute at the source so they survive being vendored elsewhere.
| STAGING="${DEST}.tmp" | ||
| trap 'rm -rf "$WORKDIR" "$STAGING"' EXIT | ||
|
|
||
| git init -q "$WORKDIR" |
There was a problem hiding this comment.
[MEDIUM] Docs build now has a hard external network dependency with no fallback
predev/prebuild now shell out to git init + git fetch --depth 1 https://github.com/fullsend-ai/agents.git "$REF" under set -euo pipefail. Previously agent docs were static files checked into this repo; now every doc-site build (local dev and CI) requires live network access to github.com and a successful fetch of another repo. If that repo/ref is temporarily unreachable or the requested ref disappears, npm run build/npm run dev fails immediately with no cached fallback to a previously-fetched snapshot, rather than degrading gracefully.
Suggestion: Consider falling back to a previously-fetched docs/agents/ (warn instead of hard-fail) when the network fetch fails, or add retry-on-transient-failure around the git fetch call, especially for CI resilience.
|
Closing due to the high nubmer of conflicts. Will reopen. |
|
🤖 Finished Retro · ✅ Success · Started 7:20 AM UTC · Completed 7:29 AM UTC Commit: |
Retro: PR #5973 — docs: pull Agents docs directlyOutcome: Closed without merge (conflicts). Author plans to reopen. Timeline
Review quality assessmentWhat the agent caught well: editLink.pattern routing agent pages to wrong repo (unique find), double-slash URL bug from trailing slash, TMPDIR shadowing POSIX env var, scope-creep detection of accidentally included incomplete file, implementation divergence from issue #5710. What the human caught that the agent missed:
Evidence for existing issues (skipped as proposals)
New proposalOne proposal filed below — review agent should inspect externally-sourced content when reviewing content-vendoring scripts. Proposals filed
|
Summary
Pull
fullsend-ai/agentsatmainwhen building the site. This way we don't duplicate docs and we get the currentmainas we also build frommainour docs.Related Issue
Closes #5710
Changes
docs/agents.gitignorefullsend-ai/agents@mainand put it intodocs/agents(just thedocs/folder there)docs/agentsdocs/agentsfolder into other folders or into thefullsend-ai/agentsrepository.Testing
make lintpasses (stage changes first, then run)Checklist
!for breaking changes)