feat(#6966): add a completed reference agent and an authoring skill - #1167
feat(#6966): add a completed reference agent and an authoring skill#1167waynesun09 wants to merge 9 commits into
Conversation
PR Summary by QodoAdd reference link-check agent and custom-agent authoring skill
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
|
🤖 Review · ❌ Terminated · Started 4:42 PM UTC · Ended 5:24 PM UTC Commit: |
Code Review by Qodo
1.
|
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 4:42 PM UTC · Completed 5:24 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
|
/fs-review |
|
🤖 Review · Commit: |
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 5:32 PM UTC · Completed 6:14 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.35 |
|
At least publicise the skill in our docs, briefly mention it, we don't need a 200 words paragraph to say "we have a skill to do this we are about to do in this document, use it". |
|
Also added the authoring-skill pointer you asked for, in 49f5ee7 — one sentence at the end of
Head is now 49f5ee7. |
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
The generated prompt says the summary is one line and the generated post-script refuses one that is not, but the generated schema only constrained type and length. So a multi-line summary passed the validation loop, no retry was triggered, and the run died in the post-script with nothing posted — the three files that are supposed to describe one contract described two. Found reviewing the reference agent in fullsend-ai/agents#1167, which is generated from this template. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
`fullsend agent new` (fullsend-ai/fullsend#6966) generates a valid agent skeleton; these are the two things it hands off to. examples/link-check/ is a complete agent generated by that command and then finished — the tree the generator produces, with the prompt filled in. It lives under examples/ rather than in the fleet directories: those are consumed by URL via `agent add`, and anything placed there reads as a first-class agent. It is deliberately not registered in config.yaml. Being an example is not a reason for it to be unchecked, since it exists to be copied: - skillsaw now runs content rules over examples/**/agents/*.md. It did not before — verified by planting a FILL IN marker and a TODO, which passed `make lint --strict` unnoticed; with the content-paths entry the same plant fails the build. - The post-script gets a test wired into `make test`. It consumes untrusted model output, so it is held to the same standard as a fleet one: the 16 cases cover the field allowlist, non-string coercion, length caps, and ISSUE_URL validation including a foreign host and a path-traversal attempt. skills/authoring-custom-agents/ is the procedure for completing a generated skeleton: replace every marker, keep the output contract, the schema and the post-script naming the same fields, keep tools: matching the body, and leave all mutation to the post-script. It is named apart from the existing agent-scaffolding skill, which is a diagnostic lens for evaluating agent infrastructure rather than an authoring procedure. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
…he cap Three fixes to the reference agent, from review. The prompt checked every link in each changed file, so a pre-existing broken link in a touched file surfaced as a finding against whoever touched it. It now extracts links only from lines the pull request adds (`git diff -U0 --diff-filter=AMR base...HEAD -- '*.md'`), and says why: blaming an author for a link they did not write is the fastest way to get an agent's comments ignored. `--diff-filter=AMR` also drops files the pull request deletes, which previously survived the filter and then failed to open at head. `tools:` gains git to match the body. The post-script cut the comment at the 16384-character cap and then appended the truncation marker, overshooting the limit the result schema declares. It now reserves the marker's length: a 17,000-character comment prints at exactly 16384. The fix is in the shared generator template, so it reaches every future generated agent, and the shell test now measures the emitted body rather than only looking for the marker. The example is regenerated with --validation-loop. The CLI default is opt-in, but every schema-backed fleet harness here uses one and the runner has python3 with jsonschema, so the example matches fleet convention — and it exercises the generated preflight_check, which reports a missing dependency before sandbox creation instead of after a full inference run. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
…ng skill The examples README described itself in fullsend jargon. It now says plainly what happened: the generator wrote every file except the prompt, and what was added by hand is the body of agents/<name>.md plus the choice of trigger and scope. "Fleet agents" becomes "the agents in this repository's harness/ directory", and the sentence explaining what an examples directory is has gone — a reader who has opened it does not need telling. docs/README.md gains one sentence where someone about to build an agent would look: run `fullsend agent new`, then follow the authoring-custom-agents skill to complete the prompt, with the link-check example as a finished one. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The three review comments on the README were one defect, and fixing only the lines pointed at left the rest. "Fleet" survived twice more — "fleet harness", "a fleet one" — so both are now "the agents in this repository's harness/ directory" and "the ones in scripts/". "The generator pins the current sandbox image" is now what that means: it records the exact container image those agents run on. "Vendor", "scaffold" and "inference run" are gone the same way. The authoring skill introduced agent definition, result schema and post-script by name and then relied on them; it now says what each one is and, for the post-script, that it is the only thing that touches the issue or pull request — which is the point of the separation the skill goes on to enforce. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The generator template had the post-script looking for its input in the wrong directory (fullsend-ai/fullsend#6972), so the example carried the same defect and this repository's test confirmed it: run_post built <tmp>/output/agent-result.json and ran from there, which is not the layout fullsend provides. Seventeen assertions passed against a script that could not have worked. The test now builds iteration-<N>/output/ and runs from the run directory, as internal/cli/run.go does, and adds the two cases the old shape could not express: that the highest-numbered iteration wins, and that FULLSEND_VALIDATED_ITERATION_DIR overrides it. The example is regenerated from the fixed template, which also brings the locale pin, the JSON-object shape check in place of `jq -e .`, the single-line summary check, and the work_item guard that keeps the trigger off GitHub Discussions. Three more reject cases cover shapes the old `jq -e .` gate got wrong: a bare null, a bare false, and a top-level array. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
A second review round, three reviewers, found that the example agent was
documented rather than possible.
Step 1 called `gh pr view --json`, which `gh` implements over GraphQL,
and step 2 called `git diff` — but the sandbox profile the generator
copies permits only the `gh` and `node` binaries, and its endpoint list
has no GraphQL entry. Both steps would have been refused at the egress
layer before the agent did any work.
The steps now use `gh api .../pulls/{n}/files`, which is REST, needs no
`git`, and returns the per-file patch hunks the agent actually wants.
`git` comes back out of `tools:`, and `jq` earns its place there for the
first time. The added-line walk is spelled out — a hunk header restarts
the counter — because a model asked for head line numbers without being
told how to derive them will invent them.
Reference-style links were being resolved against the diff rather than
the document, so a link whose definition sat on an unchanged line was
never checked; unused definitions are now skipped instead of reported,
since one renders nothing. Query strings and percent-encoding are
stripped before the existence check.
The result schema now carries a `pattern` forbidding newlines in
`summary`, which is a generator change: the prompt and the post-script
both required a single line, and only the schema did not — so a
multi-line summary passed validation and died in the post-script with
nothing posted.
The README claimed the trigger was "pr-opened" when the harness fires on
opened, synchronized and marked_ready and skips forks, and stated a
`fullsend lock` criterion without saying that command needs a
config.yaml the example deliberately does not have. The skill gave two
different invocations for the same command.
Four tests added for behaviour that had none: iteration-10 beating
iteration-9, a validated iteration directory with no result, a posting
`status: error`, and a missing GH_TOKEN.
Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
A fourth reviewer went over this and found the example asserting things
that are not true, including one I had claimed as verified.
The FILL-IN gate does not exist. I wrote that `skillsaw --strict` fails
on an unfilled `<!-- FILL IN -->` marker and showed before/after output
to prove it — but that test appended a `TODO` line as well, and the TODO
is what failed the build. Isolating them shows the marker alone passes
with zero warnings. `FILL IN` is precisely what `fullsend agent new`
emits, so the one gate this PR added `content-paths` for did not fire on
the one marker that matters. `example-link-check-test.sh` now greps for
it, scoped to agent definitions so the README can still name it in
prose, and both directions are verified.
Step 1 could not run. `gh api "repos/{owner}/{repo}/pulls/{number}/files"`
looks like it interpolates, but `{owner}` and `{repo}` are gh's own
placeholders for the current checkout's remote and there is no
`{number}` placeholder at all — a literal one goes through unsubstituted
and returns 404. The step now parses ISSUE_URL and interpolates, and
says why, since the failure looks like a permissions problem.
The rationale was wrong even where the conclusion was right. A profile's
`binaries:` list restricts which programs may reach a network endpoint;
it does not stop `git` from running. `git diff` against the base fails
here because the checkout is shallow and not at the pull request's head
— that is the real reason and the one a reader should carry away.
Also: the byte-identity claim now names the files it is true of, since
the post-script and schema carry fixes queued for the generator; the
`fullsend lock` caveat is gone because lock resolves a harness by path
and never needed a config.yaml; optional link titles and angle-bracket
destinations are handled; candidates inside code spans are skipped; a
null `patch` and the 3,000-file cap report `error` rather than a false
`ok`; and `Grep` leaves `tools:`, which nothing used.
The skill gains the enforcement half of its own rule 6: `readonly_repo:
true` is a harness field, and a prompt asking an agent not to mutate is
a request where that field is a guarantee.
Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
Rebased onto origin/main, which brings fullsend-ai/fullsend#6975 into the generator's embedded scaffold: the Vertex profile now allows `**/claude.exe` and `**/pi` alongside `**/claude`. On the v0.40.0 sandbox image the Claude binary is `claude.exe`, so before that fix it matched no profile's binaries list, no egress policy applied to it, and every request it made was refused — which is what a local `--runtime claude` run of this example hit. The example's copied profile carries the fix now, and examples/README.md gains the step that was missing for anyone trying to run one: these are deliberately absent from this repository's config.yaml, so `fullsend run` cannot resolve them until you register a copy. The exact error and the `fullsend agent add` command that avoids it are both in the README — found by hitting it. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
1592030 to
ca26a6c
Compare
The generated prompt says the summary is one line and the generated post-script refuses one that is not, but the generated schema only constrained type and length. So a multi-line summary passed the validation loop, no retry was triggered, and the run died in the post-script with nothing posted — the three files that are supposed to describe one contract described two. Found reviewing the reference agent in fullsend-ai/agents#1167, which is generated from this template. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Review · Commit: |
Running the example against a real pull request showed the agent reasoning about whether a path "exists at the pull request head" rather than checking the tree in front of it — because the prompt told it the repository was checked out at that head. It is not: the review stage checks out the default branch, shallow, so a file the pull request adds is absent and a file it deletes is still present. The Inputs section now says that, and step 5 says what to do about it: a path the pull request itself adds resolves once merged even though it is not on disk, and anything else is checked against the checkout. The previous wording invited exactly the inference the run produced. Verified by running it: exit 0, schema validation passed, post-script printed rather than posted. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 4:40 PM UTC · Completed 5:22 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
What
The two things
fullsend agent newhands off to:examples/link-check/— a complete agent generated by that command and then finished. Areview-role agent that reports Markdown links in changed docs which do not resolve.skills/authoring-custom-agents/— the procedure for completing a generated skeleton.Why
examples/and not the fleet directoriesharness/,agents/,schemas/andscripts/are consumed by URL viaagent add, so anything placed there reads as a first-class agent. Per the #631 discussion, custom and community agents are not mixed with tier-one agents. The example lives in its own subtree and is not registered inconfig.yaml— nothing dispatches it.Being an example is not a reason to be unchecked
It exists to be copied, so an unfilled placeholder or an unsafe post-script in it is exactly as harmful as in a real agent.
skillsaw did not lint
examples/at all. I verified this rather than assuming: planting a<!-- FILL IN -->marker and aTODO:into the example's agent definition and runningmake lintgaveWarnings: 0 / ✓ All checks passed. Addingexamples/**/agents/*.mdtocontent-pathsfixes it — the identical plant now givesWarnings: 1andmake lintexits 1 under--strict.The post-script is tested.
scripts/example-link-check-test.sh, wired intomake test, covers 16 cases against untrusted model output:{"summary": {"nested": true}})statusenum, required fields, the 200-char summary capISSUE_URLvalidation: a foreign host (https://evil.example.com/...) and a path-traversal attempt (.../pull/99/../../x) are both refusedAll run with
POST_LINK_CHECK_DRY_RUN=1, so nothing is ever posted.How to test
To confirm the example is a loadable agent, from a fullsend checkout:
Validated commands
fullsend agent new link-check --role review --on pr-opened --slug fullsend-ai-link-check --no-registerbash scripts/example-link-check-test.shmake lintmake lintwith a plantedFILL IN+TODOWarnings: 1, exit 1 — proves the new scope worksmake check-bundle.src.shbundle)make testfullsend lock link-check --fullsend-dir examples/link-check --offlineDocumented but not executed: a real dispatch of the example agent. That needs GCP credentials, a sandbox image and a live pull request, and the post-script's non-dry-run path comments on a real work item.
Pre-existing
make testflake, not from this changescripts/harness-jira-test.shreports 2–4 failures per run, non-deterministically —yq | grep -qreturns 141 underset -o pipefail. Three consecutive runs of the unchanged test on this branch gave 3, then 4, then 2 failures. The values it reports as missing are present inharness/triage.yaml:This branch touches neither
harness/nor that test.Notes
fullsend agent new— generate a complete custom agent from minimal parameters fullsend#6966. Independent of feat(#6966): addfullsend agent newto generate a complete custom agent fullsend#6972 — neither blocks the other, and the CLI does not fetch anything from here.--strictturns into a failure.authoring-custom-agentsrather thanagent-scaffolding: that name is taken, and the existing skill is a diagnostic lens for evaluating agent infrastructure, not an authoring procedure. The new skill cross-references it rather than overlapping.