Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 17 additions & 1 deletion internal/repos/scaffold_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,26 @@ type ScaffoldPRMetadata struct {
}

const (
// gettingStartedCatalog documents the primary /fs-* slash commands so users
// discover them at their first touchpoint — the fresh-install PR. It mirrors
// the per-org onboarding catalog (GETTING_STARTED_SECTION in
// scripts/reconcile-repos.sh); both surfaces are independently pinned to
// dispatch.yml's routing (per-repo by TestPerRepoOnboardingCatalog, per-org by
// TestReconcileReposSlashCommandCatalog) so they cannot drift apart. See #2165.
gettingStartedCatalog = "\n\n## Getting started\n\n" +
"Once this PR is merged, interact with fullsend by commenting one of these " +
"slash commands. The supported target (issue and/or pull request) is shown for each:\n\n" +
"- `/fs-triage` (issue or PR) — Invoke the [triage](https://fullsend.sh/docs/agents/triage) agent to categorize, label, and assess an issue.\n" +
"- `/fs-code` (issue only) — Invoke the [code](https://fullsend.sh/docs/agents/code) agent to implement a fix for an issue and open a PR.\n" +
"- `/fs-review` (PR only) — Invoke the [review](https://fullsend.sh/docs/agents/review) agent to review a pull request.\n" +
"- `/fs-fix` (PR only) — Invoke the [fix](https://fullsend.sh/docs/agents/fix) agent to address review feedback on a pull request.\n" +
"- `/fs-retro` (issue or PR) — Invoke the [retro](https://fullsend.sh/docs/agents/retro) agent to analyze completed work and propose improvements.\n" +
"- `/fs-prioritize` (issue or PR) — Invoke the [prioritize](https://fullsend.sh/docs/agents/prioritize) agent to score an issue for project board ranking."

// defaultScaffoldPRBody is the PR body for fresh installations.
// Only used within this package.
defaultScaffoldPRBody = "This PR adds the fullsend scaffold files for per-repo installation.\n\n" +
"Merge this PR to activate fullsend workflows."
"Merge this PR to activate fullsend workflows." + gettingStartedCatalog

// DefaultScaffoldBranch is the branch name for fresh installations.
DefaultScaffoldBranch = "fullsend/scaffold-install"
Expand Down
141 changes: 141 additions & 0 deletions internal/repos/scaffold_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ package repos

import (
"context"
"regexp"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/fullsend-ai/fullsend/internal/forge"
"github.com/fullsend-ai/fullsend/internal/scaffold"
)

func TestBuildScaffoldPRMetadata_FreshInstall(t *testing.T) {
Expand Down Expand Up @@ -170,6 +173,144 @@ func TestDetectExistingVersion(t *testing.T) {
})
}

// commandsNotInPerRepoCatalog mirrors commandsNotInOnboardingCatalog in the
// scaffold package: dispatch.yml routes these but they are deliberately omitted
// from the user-facing per-repo onboarding catalog.
// - /fullsend: backward-compat alias for the /fs-retro form; /fs-retro is the
// primary command documented in the catalog.
var commandsNotInPerRepoCatalog = map[string]bool{
"/fullsend": true,
}

// These drift-guard helpers mirror the ones in the scaffold package's test.
// They live in a different package, so they are duplicated here rather than
// shared. Route extraction is scoped to dispatch.yml's case-arm labels and
// catalog extraction to rendered "- `cmd`" bullets, so a command mentioned in a
// comment, URL, or prose on either side cannot spoof a match.
var (
// dispatchCaseArmRE matches a case-arm label in dispatch.yml's
// `case "${COMMAND}"` switch, e.g. "/fs-triage)" or "/fs-retro|/fullsend)".
dispatchCaseArmRE = regexp.MustCompile(`(?m)^[ \t]*(/(?:fs-[a-z0-9-]+|fullsend)(?:\|/(?:fs-[a-z0-9-]+|fullsend))*)\)`)
// slashCommandRE matches a single /fs-* or /fullsend command token.
slashCommandRE = regexp.MustCompile(`/(?:fs-[a-z0-9-]+|fullsend)`)
// catalogBulletRE matches a rendered onboarding-catalog bullet. Both catalogs
// use bare backticks at this point (the per-org shell escapes are normalized
// before comparison, and the per-repo Go catalog uses bare backticks).
catalogBulletRE = regexp.MustCompile("(?m)^- `(/(?:fs-[a-z0-9-]+|fullsend))`")
// catalogEntryRE additionally captures the full bullet text (target +
// description) after the command, for cross-catalog comparison.
catalogEntryRE = regexp.MustCompile("(?m)^- `(/(?:fs-[a-z0-9-]+|fullsend))` (.*)$")
)

// routedDispatchCommands returns the set of slash commands dispatch.yml routes
// on, scoped to case-arm labels.
func routedDispatchCommands(dispatchStr string) map[string]bool {
cmds := map[string]bool{}
for _, arm := range dispatchCaseArmRE.FindAllStringSubmatch(dispatchStr, -1) {
for _, cmd := range slashCommandRE.FindAllString(arm[1], -1) {
cmds[cmd] = true
}
}
return cmds
}

// catalogCommands returns the set of slash commands documented as bullets in an
// onboarding catalog block.
func catalogCommands(catalog string) map[string]bool {
cmds := map[string]bool{}
for _, m := range catalogBulletRE.FindAllStringSubmatch(catalog, -1) {
cmds[m[1]] = true
}
return cmds
}

// catalogEntries maps each documented command to its full rendered bullet text
// (target hint + description), for comparing two catalogs entry-for-entry.
func catalogEntries(catalog string) map[string]string {
entries := map[string]string{}
for _, m := range catalogEntryRE.FindAllStringSubmatch(catalog, -1) {
entries[m[1]] = strings.TrimSpace(m[2])
}
return entries
}

// extractPerOrgCatalog pulls the GETTING_STARTED_SECTION assignment out of
// reconcile-repos.sh and normalizes the shell backtick-escapes (\`) to the
// rendered backtick form, so it compares directly against the per-repo catalog.
func extractPerOrgCatalog(t *testing.T, script string) string {
t.Helper()
const marker = `GETTING_STARTED_SECTION="`
start := strings.Index(script, marker)
require.NotEqual(t, -1, start, "GETTING_STARTED_SECTION marker not found in reconcile-repos.sh")
rest := script[start+len(marker):]
end := strings.IndexByte(rest, '"')
require.NotEqual(t, -1, end, "unterminated GETTING_STARTED_SECTION assignment")
return strings.ReplaceAll(rest[:end], "\\`", "`")
}

// TestPerRepoOnboardingCatalog guards the per-repo install PR body's
// slash-command catalog against drift from dispatch.yml's routing, in both
// directions — the per-repo analogue of TestReconcileReposSlashCommandCatalog in
// the scaffold package (which guards the per-org onboarding catalog). Pinning
// both catalogs to the same source (dispatch.yml) keeps the two onboarding
// surfaces from diverging.
func TestPerRepoOnboardingCatalog(t *testing.T) {
dispatch, err := scaffold.FullsendRepoFile(".github/workflows/dispatch.yml")
require.NoError(t, err)

dispatchCmds := routedDispatchCommands(string(dispatch))
require.NotEmpty(t, dispatchCmds, "expected dispatch.yml to route on /fs-* commands")

catalogCmds := catalogCommands(gettingStartedCatalog)
require.NotEmpty(t, catalogCmds, "expected the per-repo catalog to document /fs-* commands")

// Forward: dispatch.yml commands must be documented (unless deliberately omitted).
for cmd := range dispatchCmds {
if commandsNotInPerRepoCatalog[cmd] {
continue
}
assert.True(t, catalogCmds[cmd],
"dispatch.yml routes on %s but the per-repo onboarding catalog does not document it "+
"(add it to gettingStartedCatalog, or to commandsNotInPerRepoCatalog if intentional)", cmd)
}

// Reverse: every documented command must be routed by dispatch.yml.
for cmd := range catalogCmds {
assert.True(t, dispatchCmds[cmd],
"per-repo onboarding catalog documents %s but dispatch.yml does not route on it", cmd)
}
}

// TestOnboardingCatalogsMatch pins the per-org (reconcile-repos.sh) and per-repo
// (gettingStartedCatalog) onboarding catalogs to each other, so the two surfaces
// cannot drift apart in the commands they list or in each command's target hint
// and description. The drift guards ensure each catalog matches dispatch.yml's
// routing; this ensures they also match each other verbatim.
func TestOnboardingCatalogsMatch(t *testing.T) {
script, err := scaffold.FullsendRepoFile("scripts/reconcile-repos.sh")
require.NoError(t, err)

perOrg := catalogEntries(extractPerOrgCatalog(t, string(script)))
perRepo := catalogEntries(gettingStartedCatalog)
require.NotEmpty(t, perOrg, "expected the per-org catalog to document /fs-* commands")
require.NotEmpty(t, perRepo, "expected the per-repo catalog to document /fs-* commands")

assert.Equal(t, perOrg, perRepo,
"per-org (reconcile-repos.sh) and per-repo (gettingStartedCatalog) onboarding catalogs "+
"must document the same commands with the same target hint and description")
}

// TestFreshInstallBodyIncludesCatalog verifies the fresh-install PR body carries
// the Getting started catalog, so dropping the append is caught by CI.
func TestFreshInstallBodyIncludesCatalog(t *testing.T) {
fc := forge.NewFakeClient()
notInstalled := false
meta := BuildScaffoldPRMetadata(context.Background(), fc, "acme", "widget", "v0.28.0",
ScaffoldMetadataOpts{GuardInstalled: &notInstalled})
assert.Contains(t, meta.PRBody, "## Getting started")
assert.Contains(t, meta.PRBody, "`/fs-triage`")
}

func TestRuntimeSection(t *testing.T) {
t.Parallel()
def := RuntimeSection("")
Expand Down
22 changes: 20 additions & 2 deletions internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,33 @@ ENROLL_PR_TITLE="chore: connect to fullsend agent pipeline"
UNENROLL_PR_TITLE="chore: disconnect from fullsend agent pipeline"
UPDATE_PR_TITLE="chore: update fullsend shim workflow"

# Shared "Getting started" block appended to both enrollment and update PRs.
Comment thread
shairevivo marked this conversation as resolved.
# The update path is the first touchpoint for already-enrolled repos (see #2165),
# so it must document the slash commands too.
GETTING_STARTED_SECTION="## Getting started

Once this PR is merged, interact with fullsend by commenting one of these slash commands. The supported target (issue and/or pull request) is shown for each:

- \`/fs-triage\` (issue or PR) — Invoke the [triage](https://fullsend.sh/docs/agents/triage) agent to categorize, label, and assess an issue.
- \`/fs-code\` (issue only) — Invoke the [code](https://fullsend.sh/docs/agents/code) agent to implement a fix for an issue and open a PR.
- \`/fs-review\` (PR only) — Invoke the [review](https://fullsend.sh/docs/agents/review) agent to review a pull request.
- \`/fs-fix\` (PR only) — Invoke the [fix](https://fullsend.sh/docs/agents/fix) agent to address review feedback on a pull request.
- \`/fs-retro\` (issue or PR) — Invoke the [retro](https://fullsend.sh/docs/agents/retro) agent to analyze completed work and propose improvements.
- \`/fs-prioritize\` (issue or PR) — Invoke the [prioritize](https://fullsend.sh/docs/agents/prioritize) agent to score an issue for project board ranking."

ENROLL_PR_BODY="This PR adds a shim workflow that routes repository events to the fullsend agent dispatch workflow in the \`.fullsend\` config repo.

Once merged, issues, PRs, and comments in this repo will be handled by the fullsend agent pipeline."
Once merged, issues, PRs, and comments in this repo will be handled by the fullsend agent pipeline.

${GETTING_STARTED_SECTION}"
UNENROLL_PR_BODY="This PR removes the fullsend shim workflow. The repo has been set to \`enabled: false\` in the fullsend config.

Comment thread
shairevivo marked this conversation as resolved.
Once merged, this repo will no longer dispatch events to the fullsend agent pipeline."
UPDATE_PR_BODY="This PR updates the fullsend shim workflow to match the current template in the \`.fullsend\` config repo.

The shim content has drifted from the template — this brings it back in sync."
The shim content has drifted from the template — this brings it back in sync.

${GETTING_STARTED_SECTION}"

UPDATE_COMMIT_MSG="chore: update fullsend shim workflow

Expand Down
130 changes: 130 additions & 0 deletions internal/scaffold/scaffold_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -802,6 +803,135 @@ func TestReconcileReposContent(t *testing.T) {
"reconcile-repos.sh should not parse dispatch mode")
assert.Contains(t, s, "private repos cannot be enrolled",
"reconcile-repos.sh should skip private repos to prevent log exposure")

// The "Getting started" slash-command catalog (#2165) must appear in both the
// enrollment and update PR bodies. The update path is the first touchpoint for
// already-enrolled repos, which is the scenario the original incident hit.
assert.Contains(t, s, "## Getting started",
"reconcile-repos.sh PR bodies should include the Getting started section")
assert.Contains(t, s, `GETTING_STARTED_SECTION`,
"Getting started block should be shared so it appears in both enroll and update PRs")
assert.Contains(t, s, `ENROLL_PR_BODY=`)
assert.Contains(t, s, `UPDATE_PR_BODY=`)
// Both PR bodies interpolate the shared block.
assert.Equal(t, 2, strings.Count(s, `${GETTING_STARTED_SECTION}`),
"shared Getting started block should be appended to both the enroll and update PR bodies")
}

// commandsNotInOnboardingCatalog lists slash commands that dispatch.yml routes
// on but that are deliberately omitted from the user-facing onboarding catalog,
// so the omission is a recorded decision rather than a regex accident. Anything
// routed by dispatch.yml and not listed here must appear in the catalog.
// - /fullsend: backward-compat alias for the /fs-retro form; /fs-retro is the
// primary command documented in the catalog.
var commandsNotInOnboardingCatalog = map[string]bool{
"/fullsend": true,
}

// extractGettingStartedSection returns the body of the GETTING_STARTED_SECTION
// shell assignment in reconcile-repos.sh — the exact block rendered into the
// onboarding PR bodies. Assertions scope to this block rather than the whole
// script so a command name appearing in an unrelated comment or code path cannot
// satisfy the catalog guard.
func extractGettingStartedSection(t *testing.T, scriptStr string) string {
t.Helper()
const marker = `GETTING_STARTED_SECTION="`
start := strings.Index(scriptStr, marker)
require.GreaterOrEqual(t, start, 0,
"expected GETTING_STARTED_SECTION assignment in reconcile-repos.sh")
rest := scriptStr[start+len(marker):]
// The block contains no embedded double quotes, so the next quote closes it.
end := strings.Index(rest, `"`)
require.GreaterOrEqual(t, end, 0,
"GETTING_STARTED_SECTION assignment should be closed with a double quote")
return rest[:end]
}

// dispatchCaseArmRE matches a case-arm label line in dispatch.yml's
// `case "${COMMAND}"` switch, e.g. " /fs-triage)" or
// " /fs-retro|/fullsend)". Scoping route extraction to these lines
// keeps a command mentioned in a comment, URL, or unrelated shell statement from
// being counted as routed.
var dispatchCaseArmRE = regexp.MustCompile(`(?m)^[ \t]*(/(?:fs-[a-z0-9-]+|fullsend)(?:\|/(?:fs-[a-z0-9-]+|fullsend))*)\)`)

// slashCommandRE matches a single /fs-* or /fullsend command token.
var slashCommandRE = regexp.MustCompile(`/(?:fs-[a-z0-9-]+|fullsend)`)

// catalogBulletRE matches a rendered onboarding-catalog bullet, e.g.
// "- `/fs-triage`". The optional leading backslash accommodates the shell
// assignment (backticks are escaped as \` there); the per-repo Go catalog uses
// bare backticks. Anchoring to the "- " bullet keeps a command mentioned in a
// docs URL or prose from counting as documented.
var catalogBulletRE = regexp.MustCompile("(?m)^- \\\\?`(/(?:fs-[a-z0-9-]+|fullsend))\\\\?`")

// routedDispatchCommands returns the set of slash commands dispatch.yml routes
// on, scoped to case-arm labels (see dispatchCaseArmRE).
func routedDispatchCommands(dispatchStr string) map[string]bool {
cmds := map[string]bool{}
for _, arm := range dispatchCaseArmRE.FindAllStringSubmatch(dispatchStr, -1) {
for _, cmd := range slashCommandRE.FindAllString(arm[1], -1) {
cmds[cmd] = true
}
}
return cmds
}

// catalogCommands returns the set of slash commands documented as bullets in an
// onboarding catalog block (see catalogBulletRE).
func catalogCommands(catalog string) map[string]bool {
cmds := map[string]bool{}
for _, m := range catalogBulletRE.FindAllStringSubmatch(catalog, -1) {
cmds[m[1]] = true
}
return cmds
}

// TestReconcileReposSlashCommandCatalog guards against the onboarding PR body's
// slash-command catalog drifting from dispatch.yml's routing, in both directions:
// - forward: every command dispatch.yml routes on (except deliberately-omitted
// aliases in commandsNotInOnboardingCatalog) must appear in the catalog, so a
// command added/renamed in dispatch.yml without updating the catalog fails CI.
// - reverse: every command documented in the catalog must actually be routed by
// dispatch.yml, so a command removed from dispatch.yml but left in the
// user-facing catalog also fails CI.
//
// Routed commands are extracted only from dispatch.yml's case-arm labels, and
// documented commands only from rendered catalog bullets, so comments, URLs, or
// prose on either side cannot spoof a match. Membership is compared as exact
// tokens (via sets) rather than substring containment so a hyphenated command
// (e.g. /fs-fix-stop) cannot satisfy the guard against an unrelated prefix
// (/fs-fix). The omission allow-list is applied to the forward check only: a
// command written into the catalog and later dropped from dispatch must fail even
// if it is allow-listed.
func TestReconcileReposSlashCommandCatalog(t *testing.T) {
dispatch, err := FullsendRepoFile(".github/workflows/dispatch.yml")
require.NoError(t, err)
script, err := FullsendRepoFile("scripts/reconcile-repos.sh")
require.NoError(t, err)

catalog := extractGettingStartedSection(t, string(script))

dispatchCmds := routedDispatchCommands(string(dispatch))
require.NotEmpty(t, dispatchCmds, "expected dispatch.yml to route on /fs-* commands")

catalogCmds := catalogCommands(catalog)
require.NotEmpty(t, catalogCmds, "expected the onboarding catalog to document /fs-* commands")

// Forward: dispatch.yml commands must be documented (unless deliberately omitted).
for cmd := range dispatchCmds {
if commandsNotInOnboardingCatalog[cmd] {
continue
}
assert.True(t, catalogCmds[cmd],
"dispatch.yml routes on %s but the onboarding catalog does not document it "+
"(add it to GETTING_STARTED_SECTION, or to commandsNotInOnboardingCatalog if intentional)", cmd)
}

// Reverse: every documented command must be routed by dispatch.yml.
for cmd := range catalogCmds {
assert.True(t, dispatchCmds[cmd],
"onboarding catalog documents %s but dispatch.yml does not route on it", cmd)
}
}

func TestPrioritizeWorkflowContent(t *testing.T) {
Expand Down
Loading