diff --git a/e2e/admin/admin_test.go b/e2e/admin/admin_test.go index 6449b4e5..816a9b61 100644 --- a/e2e/admin/admin_test.go +++ b/e2e/admin/admin_test.go @@ -417,7 +417,6 @@ func verifyInstalled(t *testing.T, env *e2eEnv, orgCfg *config.OrgConfig, enable "env/code-agent.env", "env/gcp-vertex.env", "scripts/scan-secrets", - "scripts/pre-code.sh", "scripts/post-code.sh", "scripts/post-triage.sh", "scripts/reconcile-repos.sh", @@ -429,7 +428,6 @@ func verifyInstalled(t *testing.T, env *e2eEnv, orgCfg *config.OrgConfig, enable "policies/fix.yaml", "env/fix-agent.env", "schemas/fix-result.schema.json", - "scripts/pre-fix.sh", "scripts/post-fix.sh", "scripts/process-fix-result.py", "templates/shim-workflow.yaml", diff --git a/internal/cli/gate.go b/internal/cli/gate.go new file mode 100644 index 00000000..0da6c2a3 --- /dev/null +++ b/internal/cli/gate.go @@ -0,0 +1,412 @@ +package cli + +import ( + "context" + "fmt" + "net/url" + "os" + "regexp" + "strconv" + "strings" + + "github.com/spf13/cobra" + + "github.com/fullsend-ai/fullsend/internal/forge" + gh "github.com/fullsend-ai/fullsend/internal/forge/github" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +func newGateCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "gate", + Short: "Pre-agent gates that run before sandbox creation", + Long: `Gates validate inputs and check preconditions on the GitHub Actions +runner BEFORE sandbox creation. Each subcommand corresponds to an +agent stage (code, triage, review).`, + } + cmd.AddCommand(newGateCodeCmd()) + cmd.AddCommand(newGateFixCmd()) + return cmd +} + +func newGateCodeCmd() *cobra.Command { + return &cobra.Command{ + Use: "code", + Short: "Gate for the code agent — validates inputs and checks for existing PRs", + Long: `Validates workflow_dispatch inputs and checks whether a human PR +already addresses the target issue. If a human PR exists, applies +the pr-open label, posts an informational comment, and writes +skip=true to GITHUB_OUTPUT so downstream workflow steps can be +conditionally skipped. + +Required environment variables: + ISSUE_NUMBER — positive integer + REPO_FULL_NAME — owner/repo format + GITHUB_ISSUE_URL — valid GitHub issue URL + +Optional: + GH_TOKEN / GITHUB_TOKEN — required for PR check + CODE_FORCE=true — skip PR check + FULLSEND_BOT_LOGIN — bot login to filter (default: fullsend-ai[bot])`, + RunE: func(cmd *cobra.Command, args []string) error { + printer := ui.New(os.Stdout) + + cfg := gateCodeConfig{ + IssueNumber: os.Getenv("ISSUE_NUMBER"), + RepoFullName: os.Getenv("REPO_FULL_NAME"), + IssueURL: os.Getenv("GITHUB_ISSUE_URL"), + BotLogin: os.Getenv("FULLSEND_BOT_LOGIN"), + Force: os.Getenv("CODE_FORCE") == "true", + OutputFile: os.Getenv("GITHUB_OUTPUT"), + } + + token, err := resolveToken() + if err != nil { + printer.Raw(fmt.Sprintf("::warning::No GitHub token available: %v\n", err)) + } + if token != "" { + cfg.Client = gh.New(token) + } + + return runGateCode(cmd.Context(), cfg, printer) + }, + } +} + +type gateCodeConfig struct { + IssueNumber string + RepoFullName string + IssueURL string + BotLogin string + Force bool + Client forge.Client + OutputFile string // GITHUB_OUTPUT path for writing step outputs +} + +var ( + issueNumberRe = regexp.MustCompile(`^[1-9][0-9]*$`) + repoFullNameRe = regexp.MustCompile(`^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$`) + issueURLRe = regexp.MustCompile(`^https://github\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/issues/[0-9]+$`) + botLoginRe = regexp.MustCompile(`^[a-zA-Z0-9._-]+(\[bot\])?$`) +) + +func validateGateCodeInputs(issueNumber, repoFullName, issueURL string) []string { + var errs []string + + if !issueNumberRe.MatchString(issueNumber) { + errs = append(errs, fmt.Sprintf("ISSUE_NUMBER must be a positive integer, got: '%s'", issueNumber)) + } + if !repoFullNameRe.MatchString(repoFullName) { + errs = append(errs, fmt.Sprintf("REPO_FULL_NAME must be owner/repo format, got: '%s'", repoFullName)) + } + if !issueURLRe.MatchString(issueURL) { + errs = append(errs, fmt.Sprintf("GITHUB_ISSUE_URL format invalid, got: '%s'", issueURL)) + } + + if issueURLRe.MatchString(issueURL) && repoFullNameRe.MatchString(repoFullName) { + u, err := url.Parse(issueURL) + if err == nil { + parts := strings.Split(strings.TrimPrefix(u.Path, "/"), "/") + if len(parts) >= 4 { + urlRepo := parts[0] + "/" + parts[1] + urlIssue := parts[3] + if urlRepo != repoFullName { + errs = append(errs, fmt.Sprintf("REPO_FULL_NAME does not match issue URL repo ('%s' vs '%s')", repoFullName, urlRepo)) + } + if urlIssue != issueNumber { + errs = append(errs, fmt.Sprintf("ISSUE_NUMBER does not match issue URL number ('%s' vs '%s')", issueNumber, urlIssue)) + } + } + } + } + + return errs +} + +func runGateCode(ctx context.Context, cfg gateCodeConfig, printer *ui.Printer) error { + errs := validateGateCodeInputs(cfg.IssueNumber, cfg.RepoFullName, cfg.IssueURL) + if len(errs) > 0 { + for _, e := range errs { + printer.Raw(fmt.Sprintf("::error::%s\n", e)) + } + return fmt.Errorf("input validation failed with %d error(s)", len(errs)) + } + + printer.Raw(fmt.Sprintf("::notice::Code target: %s\n", cfg.IssueURL)) + printer.StepDone("Input validation passed") + printer.KeyValue("ISSUE_NUMBER", cfg.IssueNumber) + printer.KeyValue("REPO_FULL_NAME", cfg.RepoFullName) + printer.KeyValue("GITHUB_ISSUE_URL", cfg.IssueURL) + + if cfg.Client == nil { + printer.StepInfo("GH_TOKEN not set — skipping existing-PR check") + return nil + } + + if cfg.Force { + printer.StepInfo("CODE_FORCE=true — skipping existing-PR check") + return nil + } + + botLogin := cfg.BotLogin + if botLogin == "" { + botLogin = "fullsend-ai[bot]" + } + if !botLoginRe.MatchString(botLogin) { + printer.Raw(fmt.Sprintf("::error::FULLSEND_BOT_LOGIN contains invalid characters: '%s'\n", botLogin)) + return fmt.Errorf("FULLSEND_BOT_LOGIN contains invalid characters: '%s'", botLogin) + } + + parts := strings.SplitN(cfg.RepoFullName, "/", 2) + owner, repo := parts[0], parts[1] + issueNum, _ := strconv.Atoi(cfg.IssueNumber) + + printer.StepStart(fmt.Sprintf("Checking for existing open PRs linked to issue #%d...", issueNum)) + + events, err := cfg.Client.ListIssueTimeline(ctx, owner, repo, issueNum) + if err != nil { + printer.Raw(fmt.Sprintf("::warning::Failed to check for existing PRs: %v\n", err)) + printer.StepInfo("No existing human PRs found — proceeding with code agent") + return nil + } + + var humanPRs []forge.TimelineEvent + for _, e := range events { + if e.PRState == "open" && e.PRAuthor != botLogin { + humanPRs = append(humanPRs, e) + } + } + + if len(humanPRs) == 0 { + printer.StepDone("No existing human PRs found — proceeding with code agent") + return nil + } + + first := humanPRs[0] + printer.Raw(fmt.Sprintf("::notice::Found existing human PR #%d by @%s\n", first.PRNumber, first.PRAuthor)) + + if err := cfg.Client.EnsureLabel(ctx, owner, repo, "pr-open", "An open PR already addresses this issue", "D4C5F9"); err != nil { + printer.Raw(fmt.Sprintf("::warning::Failed to ensure pr-open label: %v\n", err)) + } + if err := cfg.Client.AddIssueLabels(ctx, owner, repo, issueNum, []string{"pr-open"}); err != nil { + printer.Raw(fmt.Sprintf("::warning::Failed to add pr-open label to issue #%d: %v\n", issueNum, err)) + } + + var prListMD string + for _, pr := range humanPRs { + prListMD += fmt.Sprintf("\n- #%d by @%s", pr.PRNumber, pr.PRAuthor) + } + + commentBody := fmt.Sprintf(`An open PR already addresses this issue — skipping automated implementation. +%s + +To override, comment `+"`/code --force`"+` on this issue. + +Posted by fullsend pre-code check`, prListMD) + + comments, err := cfg.Client.ListIssueComments(ctx, owner, repo, issueNum) + if err != nil { + printer.Raw(fmt.Sprintf("::warning::Failed to check existing comments on issue #%d: %v\n", issueNum, err)) + comments = nil + } + + hasExisting := false + for _, c := range comments { + if strings.HasPrefix(c.Body, "An open PR already addresses") { + hasExisting = true + break + } + } + + if !hasExisting { + if _, err := cfg.Client.CreateIssueComment(ctx, owner, repo, issueNum, commentBody); err != nil { + printer.Raw(fmt.Sprintf("::warning::Failed to post comment on issue #%d: %v\n", issueNum, err)) + } + } else { + printer.Raw(fmt.Sprintf("::notice::Skipping duplicate comment — bot already posted on issue #%d\n", issueNum)) + } + + if err := writeGateOutput(cfg.OutputFile, "skip", "true"); err != nil { + printer.Raw(fmt.Sprintf("::warning::Failed to write GITHUB_OUTPUT: %v\n", err)) + } + + printer.StepDone(fmt.Sprintf("Skipping code agent — existing PR(s) found for issue #%d", issueNum)) + return nil +} + +func newGateFixCmd() *cobra.Command { + return &cobra.Command{ + Use: "fix", + Short: "Gate for the fix agent — validates inputs and enforces iteration caps", + Long: `Validates workflow_dispatch inputs for the fix agent and enforces +iteration caps to prevent unbounded fix loops. + +Required environment variables: + PR_NUMBER — positive integer + REPO_FULL_NAME — owner/repo format + TRIGGER_SOURCE — GitHub username that triggered the fix + +Optional: + FIX_ITERATION — current iteration count (default: 1) + ITERATION_CAP — max bot-triggered iterations (default: 5) + ITERATION_CAP_HUMAN — max human-triggered iterations (default: 10) + HUMAN_INSTRUCTION — instruction text (validated for length)`, + RunE: func(cmd *cobra.Command, args []string) error { + printer := ui.New(os.Stdout) + + cfg := gateFixConfig{ + PRNumber: os.Getenv("PR_NUMBER"), + RepoFullName: os.Getenv("REPO_FULL_NAME"), + TriggerSource: os.Getenv("TRIGGER_SOURCE"), + Iteration: os.Getenv("FIX_ITERATION"), + IterationCap: os.Getenv("ITERATION_CAP"), + IterationCapHuman: os.Getenv("ITERATION_CAP_HUMAN"), + HumanInstruction: os.Getenv("HUMAN_INSTRUCTION"), + } + + return runGateFix(cmd.Context(), cfg, printer) + }, + } +} + +type gateFixConfig struct { + PRNumber string + RepoFullName string + TriggerSource string + Iteration string + IterationCap string + IterationCapHuman string + HumanInstruction string +} + +const ( + maxInstructionBytes = 10000 + maxBotInstructionBytes = 1048576 // 1 MB — matches review body cap in fix.yml +) + +func isBotUser(username string) bool { + return strings.HasSuffix(username, "[bot]") +} + +func parsePositiveIntOrDefault(envName, raw string, defaultVal int) (int, error) { + if raw == "" { + return defaultVal, nil + } + v, err := strconv.Atoi(raw) + if err != nil { + return 0, fmt.Errorf("%s must be a valid integer, got: '%s'", envName, raw) + } + if v < 1 { + return 0, fmt.Errorf("%s must be positive, got: %d", envName, v) + } + return v, nil +} + +var triggerSourceRe = regexp.MustCompile(`^[a-zA-Z0-9._-]+(\[bot\])?$`) + +func validateGateFixInputs(prNumber, repoFullName, triggerSource string) []string { + var errs []string + + if !issueNumberRe.MatchString(prNumber) { + errs = append(errs, fmt.Sprintf("PR_NUMBER must be a positive integer, got: '%s'", prNumber)) + } + if !repoFullNameRe.MatchString(repoFullName) { + errs = append(errs, fmt.Sprintf("REPO_FULL_NAME must be owner/repo format, got: '%s'", repoFullName)) + } + if triggerSource == "" { + errs = append(errs, "TRIGGER_SOURCE is required (GitHub username that triggered the fix)") + } else if !triggerSourceRe.MatchString(triggerSource) { + errs = append(errs, fmt.Sprintf("TRIGGER_SOURCE must be a valid GitHub username, got: '%s'", triggerSource)) + } + + return errs +} + +func runGateFix(_ context.Context, cfg gateFixConfig, printer *ui.Printer) error { + errs := validateGateFixInputs(cfg.PRNumber, cfg.RepoFullName, cfg.TriggerSource) + if len(errs) > 0 { + for _, e := range errs { + printer.Raw(fmt.Sprintf("::error::%s\n", e)) + } + return fmt.Errorf("input validation failed with %d error(s)", len(errs)) + } + + // Instruction length cap — lower for humans, higher for bots. + instrCap := maxInstructionBytes + if isBotUser(cfg.TriggerSource) { + instrCap = maxBotInstructionBytes + } + if len(cfg.HumanInstruction) > instrCap { + printer.Raw(fmt.Sprintf("::error::HUMAN_INSTRUCTION is %d bytes (max: %d). Truncate the instruction.\n", + len(cfg.HumanInstruction), instrCap)) + return fmt.Errorf("HUMAN_INSTRUCTION is %d bytes (max: %d)", len(cfg.HumanInstruction), instrCap) + } + + iteration, err := parsePositiveIntOrDefault("FIX_ITERATION", cfg.Iteration, 1) + if err != nil { + printer.Raw(fmt.Sprintf("::error::%v\n", err)) + return err + } + + botCap, err := parsePositiveIntOrDefault("ITERATION_CAP", cfg.IterationCap, 5) + if err != nil { + printer.Raw(fmt.Sprintf("::error::%v\n", err)) + return err + } + + humanCap, err := parsePositiveIntOrDefault("ITERATION_CAP_HUMAN", cfg.IterationCapHuman, 10) + if err != nil { + printer.Raw(fmt.Sprintf("::error::%v\n", err)) + return err + } + + cap := humanCap + if isBotUser(cfg.TriggerSource) { + cap = botCap + } + + if iteration > cap { + if isBotUser(cfg.TriggerSource) { + printer.Raw(fmt.Sprintf("::error::Fix iteration %d exceeds bot cap of %d. Escalating to human.\n", iteration, cap)) + printer.Raw(fmt.Sprintf("::error::The review→fix loop has run %d times without converging.\n", iteration)) + printer.Raw(fmt.Sprintf("::error::A human can still direct the agent with /fix (up to %d total iterations).\n", humanCap)) + } else { + printer.Raw(fmt.Sprintf("::error::Fix iteration %d exceeds human cap of %d.\n", iteration, cap)) + printer.Raw(fmt.Sprintf("::error::The /fix loop has run %d times. Further attempts are blocked.\n", iteration)) + } + return fmt.Errorf("fix iteration %d exceeds cap of %d", iteration, cap) + } + + printer.StepDone("Input validation passed") + printer.KeyValue("PR_NUMBER", cfg.PRNumber) + printer.KeyValue("REPO_FULL_NAME", cfg.RepoFullName) + printer.KeyValue("TRIGGER_SOURCE", cfg.TriggerSource) + printer.KeyValue("FIX_ITERATION", fmt.Sprintf("%d of %d", iteration, cap)) + if !isBotUser(cfg.TriggerSource) && cfg.HumanInstruction != "" { + preview := cfg.HumanInstruction + if len(preview) > 200 { + preview = preview[:200] + "..." + } + printer.KeyValue("HUMAN_INSTRUCTION", preview) + } + + return nil +} + +func writeGateOutput(path, key, value string) error { + if path == "" { + return nil + } + if strings.ContainsAny(key, "\n\r") || strings.ContainsAny(value, "\n\r") { + return fmt.Errorf("GITHUB_OUTPUT key/value must not contain newlines") + } + f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + return fmt.Errorf("opening GITHUB_OUTPUT: %w", err) + } + _, writeErr := fmt.Fprintf(f, "%s=%s\n", key, value) + if closeErr := f.Close(); writeErr == nil { + writeErr = closeErr + } + return writeErr +} diff --git a/internal/cli/gate_test.go b/internal/cli/gate_test.go new file mode 100644 index 00000000..93bb320f --- /dev/null +++ b/internal/cli/gate_test.go @@ -0,0 +1,519 @@ +package cli + +import ( + "context" + "fmt" + "os" + "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/ui" +) + +func newTestPrinter() *ui.Printer { + return ui.New(&discardWriter{}) +} + +type discardWriter struct{} + +func (d *discardWriter) Write(p []byte) (int, error) { return len(p), nil } + +func validConfig(client forge.Client) gateCodeConfig { + return gateCodeConfig{ + IssueNumber: "42", + RepoFullName: "test-org/test-repo", + IssueURL: "https://github.com/test-org/test-repo/issues/42", + Client: client, + } +} + +func TestValidateGateCodeInputs_Valid(t *testing.T) { + errs := validateGateCodeInputs("42", "test-org/test-repo", "https://github.com/test-org/test-repo/issues/42") + assert.Empty(t, errs) +} + +func TestValidateGateCodeInputs_InvalidNumber(t *testing.T) { + errs := validateGateCodeInputs("0", "test-org/test-repo", "https://github.com/test-org/test-repo/issues/42") + assert.Len(t, errs, 2) // invalid number + cross-validation mismatch +} + +func TestValidateGateCodeInputs_InvalidRepo(t *testing.T) { + errs := validateGateCodeInputs("42", "bad repo!", "https://github.com/test-org/test-repo/issues/42") + require.NotEmpty(t, errs) + assert.Contains(t, errs[0], "REPO_FULL_NAME") +} + +func TestValidateGateCodeInputs_InvalidURL(t *testing.T) { + errs := validateGateCodeInputs("42", "test-org/test-repo", "http://evil.com/issues/42") + require.NotEmpty(t, errs) + assert.Contains(t, errs[0], "GITHUB_ISSUE_URL") +} + +func TestValidateGateCodeInputs_CrossValidationMismatch(t *testing.T) { + errs := validateGateCodeInputs("42", "other-org/other-repo", "https://github.com/test-org/test-repo/issues/42") + assert.Len(t, errs, 1) + assert.Contains(t, errs[0], "does not match") +} + +func TestValidateGateCodeInputs_MultipleErrors(t *testing.T) { + errs := validateGateCodeInputs("", "", "") + assert.True(t, len(errs) >= 3, "expected at least 3 errors, got %d", len(errs)) +} + +func TestRunGateCode_NoToken(t *testing.T) { + cfg := validConfig(nil) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateCode_ForceOverride(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "open", PRAuthor: "human-dev", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + cfg := validConfig(fc) + cfg.Force = true + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + assert.Empty(t, fc.AddedComments) +} + +func TestRunGateCode_HumanPRFound_WritesSkipOutput(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "open", PRAuthor: "human-dev", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + outFile := t.TempDir() + "/github-output" + cfg := validConfig(fc) + cfg.OutputFile = outFile + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + data, err := os.ReadFile(outFile) + require.NoError(t, err) + assert.Contains(t, string(data), "skip=true") +} + +func TestRunGateCode_NoExistingPRs_NoSkipOutput(t *testing.T) { + fc := forge.NewFakeClient() + outFile := t.TempDir() + "/github-output" + cfg := validConfig(fc) + cfg.OutputFile = outFile + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + _, err = os.ReadFile(outFile) + assert.True(t, os.IsNotExist(err), "output file should not exist when no PRs found") +} + +func TestRunGateCode_HumanPRFound_AppliesLabel(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "open", PRAuthor: "human-dev", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + require.Len(t, fc.EnsuredLabels, 1) + assert.Equal(t, []string{"pr-open"}, fc.EnsuredLabels[0].Labels) + require.Len(t, fc.AddedIssueLabels, 1) + assert.Equal(t, []string{"pr-open"}, fc.AddedIssueLabels[0].Labels) + assert.Equal(t, 42, fc.AddedIssueLabels[0].Number) +} + +func TestRunGateCode_HumanPRFound_PostsComment(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "open", PRAuthor: "human-dev", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + require.Len(t, fc.AddedComments, 1) + assert.Contains(t, fc.AddedComments[0].Body, "An open PR already addresses") + assert.Contains(t, fc.AddedComments[0].Body, "#99 by @human-dev") +} + +func TestRunGateCode_BotPRFiltered(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "open", PRAuthor: "fullsend-ai[bot]", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + assert.Empty(t, fc.AddedComments) + assert.Empty(t, fc.EnsuredLabels) +} + +func TestRunGateCode_MultiplePRs(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 50, PRState: "open", PRAuthor: "dev-a", PRURL: "https://github.com/test-org/test-repo/pull/50"}, + {PRNumber: 51, PRState: "open", PRAuthor: "dev-b", PRURL: "https://github.com/test-org/test-repo/pull/51"}, + }, + } + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + require.Len(t, fc.AddedComments, 1) + assert.Contains(t, fc.AddedComments[0].Body, "#50 by @dev-a") + assert.Contains(t, fc.AddedComments[0].Body, "#51 by @dev-b") +} + +func TestRunGateCode_CommentIdempotency_SkipsDuplicate(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "open", PRAuthor: "human-dev", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + fc.IssueComments = map[string][]forge.IssueComment{ + "test-org/test-repo/42": { + {ID: 1, Body: "An open PR already addresses this issue — skipping automated implementation.", Author: "fullsend-ai[bot]"}, + }, + } + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + assert.Empty(t, fc.AddedComments) +} + +func TestRunGateCode_CommentIdempotency_PostsNew(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "open", PRAuthor: "human-dev", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + fc.IssueComments = map[string][]forge.IssueComment{ + "test-org/test-repo/42": {}, + } + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + require.Len(t, fc.AddedComments, 1) +} + +func TestRunGateCode_TimelineAPIFailure(t *testing.T) { + fc := forge.NewFakeClient() + fc.Errors["ListIssueTimeline"] = fmt.Errorf("API error") + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateCode_InvalidBotLogin(t *testing.T) { + fc := forge.NewFakeClient() + cfg := validConfig(fc) + cfg.BotLogin = "evil$(whoami)" + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid characters") +} + +func TestRunGateCode_InvalidBotLoginLeadingBracket(t *testing.T) { + fc := forge.NewFakeClient() + cfg := validConfig(fc) + cfg.BotLogin = "]evil" + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid characters") +} + +func TestRunGateCode_ValidCustomBotLogin(t *testing.T) { + fc := forge.NewFakeClient() + cfg := validConfig(fc) + cfg.BotLogin = "my-bot[bot]" + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateCode_ClosedPRIgnored(t *testing.T) { + fc := forge.NewFakeClient() + fc.TimelineEvents = map[string][]forge.TimelineEvent{ + "test-org/test-repo/42": { + {PRNumber: 99, PRState: "closed", PRAuthor: "human-dev", PRURL: "https://github.com/test-org/test-repo/pull/99"}, + }, + } + cfg := validConfig(fc) + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) + assert.Empty(t, fc.AddedComments) +} + +func TestRunGateCode_ValidationFailure(t *testing.T) { + cfg := gateCodeConfig{ + IssueNumber: "bad", + RepoFullName: "bad!", + IssueURL: "bad", + } + err := runGateCode(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "validation failed") +} + +// --------------------------------------------------------------------------- +// gate fix tests +// --------------------------------------------------------------------------- + +func TestValidateGateFixInputs_Valid(t *testing.T) { + errs := validateGateFixInputs("42", "test-org/test-repo", "human-dev") + assert.Empty(t, errs) +} + +func TestValidateGateFixInputs_InvalidPRNumber(t *testing.T) { + errs := validateGateFixInputs("0", "test-org/test-repo", "human-dev") + assert.Len(t, errs, 1) + assert.Contains(t, errs[0], "PR_NUMBER") +} + +func TestValidateGateFixInputs_InvalidRepo(t *testing.T) { + errs := validateGateFixInputs("42", "bad repo!", "human-dev") + assert.Len(t, errs, 1) + assert.Contains(t, errs[0], "REPO_FULL_NAME") +} + +func TestValidateGateFixInputs_EmptyTriggerSource(t *testing.T) { + errs := validateGateFixInputs("42", "test-org/test-repo", "") + assert.Len(t, errs, 1) + assert.Contains(t, errs[0], "TRIGGER_SOURCE") +} + +func TestValidateGateFixInputs_MultipleErrors(t *testing.T) { + errs := validateGateFixInputs("", "", "") + assert.True(t, len(errs) >= 3, "expected at least 3 errors, got %d", len(errs)) +} + +func TestRunGateFix_Valid(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateFix_ValidationFailure(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "bad", + RepoFullName: "bad!", + TriggerSource: "", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "validation failed") +} + +func TestRunGateFix_BotIterationCapExceeded(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "review-bot[bot]", + Iteration: "6", + IterationCap: "5", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "exceeds cap") +} + +func TestRunGateFix_BotIterationAtCap(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "review-bot[bot]", + Iteration: "5", + IterationCap: "5", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateFix_HumanIterationCapExceeded(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + Iteration: "11", + IterationCapHuman: "10", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "exceeds cap") +} + +func TestRunGateFix_HumanIterationAtCap(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + Iteration: "10", + IterationCapHuman: "10", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateFix_DefaultIterationCaps(t *testing.T) { + // Bot default cap is 5 + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "review-bot[bot]", + Iteration: "6", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + + // Human default cap is 10 + cfg = gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + Iteration: "11", + } + err = runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) +} + +func TestRunGateFix_HumanInstructionTooLong(t *testing.T) { + longInstruction := strings.Repeat("x", 10001) + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + HumanInstruction: longInstruction, + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "HUMAN_INSTRUCTION") +} + +func TestRunGateFix_HumanInstructionAtLimit(t *testing.T) { + instruction := strings.Repeat("x", 10000) + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + HumanInstruction: instruction, + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateFix_BotHigherInstructionCap(t *testing.T) { + // Bot cap is 1 MB, human cap is 10 KB. 20 KB passes for bots. + instruction := strings.Repeat("x", 20000) + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "review-bot[bot]", + HumanInstruction: instruction, + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateFix_BotInstructionCapExceeded(t *testing.T) { + instruction := strings.Repeat("x", 1048577) // 1 MB + 1 + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "review-bot[bot]", + HumanInstruction: instruction, + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "HUMAN_INSTRUCTION") +} + +func TestRunGateFix_DefaultIteration(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.NoError(t, err) +} + +func TestRunGateFix_NegativeIteration(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "review-bot[bot]", + Iteration: "-5", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "must be positive") +} + +func TestRunGateFix_NonNumericIteration(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + Iteration: "abc", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "valid integer") +} + +func TestRunGateFix_NonNumericCap(t *testing.T) { + cfg := gateFixConfig{ + PRNumber: "42", + RepoFullName: "test-org/test-repo", + TriggerSource: "human-dev", + IterationCap: "xyz", + } + err := runGateFix(context.Background(), cfg, newTestPrinter()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "ITERATION_CAP") +} + +func TestValidateGateFixInputs_InvalidTriggerSource(t *testing.T) { + errs := validateGateFixInputs("42", "test-org/test-repo", "evil$(whoami)") + assert.Len(t, errs, 1) + assert.Contains(t, errs[0], "TRIGGER_SOURCE") +} + +func TestWriteGateOutput_Error(t *testing.T) { + err := writeGateOutput("/no-such-dir/no-such-file", "skip", "true") + assert.Error(t, err) +} + +func TestWriteGateOutput_Empty(t *testing.T) { + err := writeGateOutput("", "skip", "true") + assert.NoError(t, err) +} + +func TestWriteGateOutput_RejectsNewlines(t *testing.T) { + outFile := t.TempDir() + "/github-output" + err := writeGateOutput(outFile, "skip\ninjected=pwned", "true") + assert.Error(t, err) + assert.Contains(t, err.Error(), "newlines") +} + +func TestIsBotUser(t *testing.T) { + assert.True(t, isBotUser("fullsend-ai[bot]")) + assert.True(t, isBotUser("review-bot[bot]")) + assert.False(t, isBotUser("human-dev")) + assert.False(t, isBotUser("bot-like-name")) +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 5bda415b..00262a22 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -20,6 +20,7 @@ func newRootCmd() *cobra.Command { cmd.AddCommand(newScanCmd()) cmd.AddCommand(newPostReviewCmd()) cmd.AddCommand(newPostCommentCmd()) + cmd.AddCommand(newGateCmd()) return cmd } diff --git a/internal/forge/fake.go b/internal/forge/fake.go index 48b4d89f..5f072728 100644 --- a/internal/forge/fake.go +++ b/internal/forge/fake.go @@ -26,6 +26,20 @@ type FileRecord struct { Content []byte } +// CommentRecord records an AddIssueComment call. +type CommentRecord struct { + Owner, Repo string + Number int + Body string +} + +// LabelRecord records an EnsureLabel or AddIssueLabels call. +type LabelRecord struct { + Owner, Repo string + Number int // 0 for EnsureLabel + Labels []string +} + // SecretRecord records a secret creation call. type SecretRecord struct { Owner, Repo, Name, Value string @@ -71,14 +85,18 @@ type FakeClient struct { // Pre-populated data Repos []Repository - FileContents map[string][]byte // key: "owner/repo/path" - WorkflowRuns map[string]*WorkflowRun // key: "owner/repo/workflow" + FileContents map[string][]byte // key: "owner/repo/path" + WorkflowRuns map[string]*WorkflowRun // key: "owner/repo/workflow" AuthenticatedUser string Installations []Installation Secrets map[string]bool // key: "owner/repo/name" PullRequests map[string][]ChangeProposal // key: "owner/repo" TokenScopes []string // scopes returned by GetTokenScopes VariablesExist map[string]bool // key: "owner/repo/name" + IssueComments map[string][]IssueComment // key: "owner/repo/number" + TimelineEvents map[string][]TimelineEvent // key: "owner/repo/number" + PullRequestHeadSHA string + PRReviews map[string][]PullRequestReview // key: "owner/repo/number" // App client IDs for GetAppClientID AppClientIDs map[string]string // key: app slug → client ID @@ -90,15 +108,6 @@ type FakeClient struct { // Error injection: key is method name, value is error to return. Errors map[string]error - // Issue comments for ListIssueComments / UpdateIssueComment. - IssueComments map[string][]IssueComment // key: "owner/repo/number" - - // Pull request head SHA for GetPullRequestHeadSHA. - PullRequestHeadSHA string - - // Pull request reviews for ListPullRequestReviews. - PRReviews map[string][]PullRequestReview // key: "owner/repo/number" - // Call recorders CreatedRepos []Repository CreatedFiles []FileRecord @@ -113,6 +122,9 @@ type FakeClient struct { UpdatedComments []UpdatedCommentRecord MinimizedComments []MinimizedCommentRecord CreatedReviews []ReviewRecord + AddedComments []CommentRecord + EnsuredLabels []LabelRecord + AddedIssueLabels []LabelRecord // internal counters proposalCounter int @@ -576,6 +588,12 @@ func (f *FakeClient) CreateIssueComment(_ context.Context, owner, repo string, n if e := f.err("CreateIssueComment"); e != nil { return nil, e } + f.AddedComments = append(f.AddedComments, CommentRecord{ + Owner: owner, + Repo: repo, + Number: number, + Body: body, + }) f.commentCounter++ comment := IssueComment{ ID: f.commentCounter, @@ -683,6 +701,50 @@ func (f *FakeClient) ListPullRequestReviews(_ context.Context, owner, repo strin return nil, nil } +func (f *FakeClient) ListIssueTimeline(_ context.Context, owner, repo string, number int) ([]TimelineEvent, error) { + f.mu.Lock() + defer f.mu.Unlock() + if e := f.err("ListIssueTimeline"); e != nil { + return nil, e + } + if f.TimelineEvents != nil { + key := fmt.Sprintf("%s/%s/%d", owner, repo, number) + if events, ok := f.TimelineEvents[key]; ok { + return events, nil + } + } + return nil, nil +} + +func (f *FakeClient) EnsureLabel(_ context.Context, owner, repo, name, description, color string) error { + f.mu.Lock() + defer f.mu.Unlock() + if e := f.err("EnsureLabel"); e != nil { + return e + } + f.EnsuredLabels = append(f.EnsuredLabels, LabelRecord{ + Owner: owner, + Repo: repo, + Labels: []string{name}, + }) + return nil +} + +func (f *FakeClient) AddIssueLabels(_ context.Context, owner, repo string, number int, labels []string) error { + f.mu.Lock() + defer f.mu.Unlock() + if e := f.err("AddIssueLabels"); e != nil { + return e + } + f.AddedIssueLabels = append(f.AddedIssueLabels, LabelRecord{ + Owner: owner, + Repo: repo, + Number: number, + Labels: labels, + }) + return nil +} + func (f *FakeClient) MergeChangeProposal(_ context.Context, _, _ string, _ int) error { f.mu.Lock() defer f.mu.Unlock() diff --git a/internal/forge/forge.go b/internal/forge/forge.go index a50ddcfa..e5114662 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -75,6 +75,15 @@ type PullRequestReview struct { SubmittedAt string } +// TimelineEvent represents a cross-reference event from the issue timeline, +// linking an issue to a pull request. +type TimelineEvent struct { + PRNumber int + PRState string // "open", "closed" + PRAuthor string + PRURL string +} + // Installation represents an app installation on an org. type Installation struct { ID int @@ -157,6 +166,9 @@ type Client interface { CreateIssueComment(ctx context.Context, owner, repo string, number int, body string) (*IssueComment, error) UpdateIssueComment(ctx context.Context, owner, repo string, commentID int, body string) error MinimizeComment(ctx context.Context, nodeID, reason string) error + ListIssueTimeline(ctx context.Context, owner, repo string, number int) ([]TimelineEvent, error) + EnsureLabel(ctx context.Context, owner, repo, name, description, color string) error + AddIssueLabels(ctx context.Context, owner, repo string, number int, labels []string) error // Pull request operations GetPullRequestHeadSHA(ctx context.Context, owner, repo string, number int) (string, error) diff --git a/internal/forge/github/github.go b/internal/forge/github/github.go index c73cc56a..7cafa247 100644 --- a/internal/forge/github/github.go +++ b/internal/forge/github/github.go @@ -1195,6 +1195,97 @@ func (c *LiveClient) ListPullRequestReviews(ctx context.Context, owner, repo str return result, nil } +// ListIssueTimeline returns cross-reference events linking PRs to an issue. +// It paginates through the timeline API and filters for cross-referenced +// events that have an associated pull request. +func (c *LiveClient) ListIssueTimeline(ctx context.Context, owner, repo string, number int) ([]forge.TimelineEvent, error) { + var result []forge.TimelineEvent + seen := make(map[int]bool) + + for page := 1; page <= 100; page++ { + path := fmt.Sprintf("/repos/%s/%s/issues/%d/timeline?per_page=100&page=%d", owner, repo, number, page) + resp, err := c.do(ctx, http.MethodGet, path, nil) + if err != nil { + return nil, fmt.Errorf("list issue timeline page %d: %w", page, err) + } + if err := checkStatus(resp, http.StatusOK); err != nil { + return nil, fmt.Errorf("list issue timeline page %d: %w", page, err) + } + + var events []struct { + Event string `json:"event"` + Source struct { + Issue struct { + Number int `json:"number"` + State string `json:"state"` + HTMLURL string `json:"html_url"` + PullRequest *struct{} `json:"pull_request"` + User struct { + Login string `json:"login"` + } `json:"user"` + } `json:"issue"` + } `json:"source"` + } + if err := decodeJSON(resp, &events); err != nil { + return nil, fmt.Errorf("decode timeline page %d: %w", page, err) + } + + for _, e := range events { + if e.Event != "cross-referenced" || e.Source.Issue.PullRequest == nil { + continue + } + prNum := e.Source.Issue.Number + if seen[prNum] { + continue + } + seen[prNum] = true + result = append(result, forge.TimelineEvent{ + PRNumber: prNum, + PRState: e.Source.Issue.State, + PRAuthor: e.Source.Issue.User.Login, + PRURL: e.Source.Issue.HTMLURL, + }) + } + + if len(events) < 100 { + break + } + } + + return result, nil +} + +// EnsureLabel creates a label if it doesn't already exist. A 422 response +// (label already exists) is treated as success. +func (c *LiveClient) EnsureLabel(ctx context.Context, owner, repo, name, description, color string) error { + payload := map[string]string{ + "name": name, + "description": description, + "color": color, + } + resp, err := c.do(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/labels", owner, repo), payload) + if err != nil { + return fmt.Errorf("ensure label %s: %w", name, err) + } + defer resp.Body.Close() + // 201 = created, 422 = already exists — both are fine. + if err := checkStatus(resp, http.StatusCreated, http.StatusUnprocessableEntity); err != nil { + return fmt.Errorf("ensure label %s: %w", name, err) + } + return nil +} + +// AddIssueLabels adds labels to an issue. +func (c *LiveClient) AddIssueLabels(ctx context.Context, owner, repo string, number int, labels []string) error { + payload := map[string][]string{"labels": labels} + resp, err := c.post(ctx, fmt.Sprintf("/repos/%s/%s/issues/%d/labels", owner, repo, number), payload) + if err != nil { + return fmt.Errorf("add issue labels: %w", err) + } + resp.Body.Close() + return nil +} + // MergeChangeProposal squash-merges a pull request by number. func (c *LiveClient) MergeChangeProposal(ctx context.Context, owner, repo string, number int) error { resp, err := c.put(ctx, fmt.Sprintf("/repos/%s/%s/pulls/%d/merge", owner, repo, number), map[string]string{"merge_method": "squash"}) diff --git a/internal/scaffold/fullsend-repo/.github/workflows/code.yml b/internal/scaffold/fullsend-repo/.github/workflows/code.yml index db20dce0..d85416b2 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/code.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/code.yml @@ -94,25 +94,38 @@ jobs: fetch-depth: 0 persist-credentials: false + - name: Install fullsend CLI + run: | + if [[ -f "${GITHUB_WORKSPACE}/bin/fullsend" ]]; then + chmod +x "${GITHUB_WORKSPACE}/bin/fullsend" + echo "${GITHUB_WORKSPACE}/bin" >> "${GITHUB_PATH}" + else + echo "::error::fullsend binary not found; run: fullsend admin install --vendor-fullsend-binary" + exit 1 + fi + - name: Validate inputs + id: gate env: ISSUE_NUMBER: ${{ fromJSON(inputs.event_payload).issue.number }} REPO_FULL_NAME: ${{ inputs.source_repo }} GITHUB_ISSUE_URL: ${{ fromJSON(inputs.event_payload).issue.html_url }} - run: bash scripts/pre-code.sh + GH_TOKEN: ${{ steps.push-token.outputs.token }} + run: fullsend gate code - name: Pre-mask GCP credential file path + if: steps.gate.outputs.skip != 'true' run: echo "::add-mask::${GITHUB_WORKSPACE}/gha-creds-" - name: Authenticate to Google Cloud (WIF) - if: vars.FULLSEND_GCP_AUTH_MODE == 'wif' + if: steps.gate.outputs.skip != 'true' && vars.FULLSEND_GCP_AUTH_MODE == 'wif' uses: google-github-actions/auth@v3 with: workload_identity_provider: ${{ secrets.FULLSEND_GCP_WIF_PROVIDER }} service_account: ${{ secrets.FULLSEND_GCP_WIF_SA_EMAIL }} - name: Authenticate to Google Cloud (SA key) - if: vars.FULLSEND_GCP_AUTH_MODE != 'wif' + if: steps.gate.outputs.skip != 'true' && vars.FULLSEND_GCP_AUTH_MODE != 'wif' uses: google-github-actions/auth@v3 with: credentials_json: ${{ secrets.FULLSEND_GCP_SA_KEY_JSON }} @@ -121,12 +134,13 @@ jobs: # WIF. For non-WIF (SA key), we set it to an empty file to avoid undefined # variable errors in downstream steps that may reference it. - name: Set GCP_OIDC_TOKEN_FILE for non-WIF - if: vars.FULLSEND_GCP_AUTH_MODE != 'wif' + if: steps.gate.outputs.skip != 'true' && vars.FULLSEND_GCP_AUTH_MODE != 'wif' run: | touch "$RUNNER_TEMP/empty-oidc-token" echo "GCP_OIDC_TOKEN_FILE=$RUNNER_TEMP/empty-oidc-token" >> "${GITHUB_ENV}" - name: Mask GCP credential file paths + if: steps.gate.outputs.skip != 'true' run: | for var in GOOGLE_GHA_CREDS_PATH GOOGLE_APPLICATION_CREDENTIALS CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE; do val="${!var:-}" @@ -136,9 +150,11 @@ jobs: done - name: Prepare sandbox credentials + if: steps.gate.outputs.skip != 'true' run: bash scripts/prepare-sandbox-credentials.sh - name: Setup agent environment + if: steps.gate.outputs.skip != 'true' env: AGENT_PREFIX: CODE_ CODE_GH_TOKEN: ${{ steps.sandbox-token.outputs.token }} @@ -149,6 +165,7 @@ jobs: run: bash .github/scripts/setup-agent-env.sh - name: Run code agent + if: steps.gate.outputs.skip != 'true' uses: ./.github/actions/fullsend env: GITHUB_ISSUE_URL: ${{ fromJSON(inputs.event_payload).issue.html_url }} diff --git a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml index b0b0715e..5de9baa7 100644 --- a/internal/scaffold/fullsend-repo/.github/workflows/fix.yml +++ b/internal/scaffold/fullsend-repo/.github/workflows/fix.yml @@ -261,14 +261,25 @@ jobs: fi echo "review_file=${REVIEW_FILE}" >> "${GITHUB_OUTPUT}" + - name: Install fullsend CLI + run: | + if [[ -f "${GITHUB_WORKSPACE}/bin/fullsend" ]]; then + chmod +x "${GITHUB_WORKSPACE}/bin/fullsend" + echo "${GITHUB_WORKSPACE}/bin" >> "${GITHUB_PATH}" + else + echo "::error::fullsend binary not found; run: fullsend admin install --vendor-fullsend-binary" + exit 1 + fi + - name: Validate inputs + id: gate env: PR_NUMBER: ${{ steps.context.outputs.pr_number }} REPO_FULL_NAME: ${{ inputs.source_repo }} TRIGGER_SOURCE: ${{ inputs.trigger_source }} FIX_ITERATION: ${{ steps.context.outputs.iteration }} HUMAN_INSTRUCTION: ${{ steps.context.outputs.instruction }} - run: bash scripts/pre-fix.sh + run: fullsend gate fix - name: Pre-mask GCP credential file path run: echo "::add-mask::${GITHUB_WORKSPACE}/gha-creds-" diff --git a/internal/scaffold/fullsend-repo/harness/code.yaml b/internal/scaffold/fullsend-repo/harness/code.yaml index 9d584934..f4becfb6 100644 --- a/internal/scaffold/fullsend-repo/harness/code.yaml +++ b/internal/scaffold/fullsend-repo/harness/code.yaml @@ -1,7 +1,7 @@ -# harness/code.yaml — code agent with pre/post script pipeline. +# harness/code.yaml — code agent with post-script pipeline. # -# Flow: pre_script → sandbox (agent) → post_script -# pre_script : validates inputs on the runner BEFORE sandbox creation +# Flow: (workflow: fullsend gate code) → sandbox (agent) → post_script +# gate : validates inputs and checks for existing PRs (runs in workflow) # agent : reads the issue, implements, tests, scans, commits locally # post_script : protected-path check, secret scan, push branch, create PR # @@ -12,7 +12,6 @@ model: opus image: ghcr.io/fullsend-ai/fullsend-code:latest policy: policies/code.yaml -pre_script: scripts/pre-code.sh post_script: scripts/post-code.sh host_files: diff --git a/internal/scaffold/fullsend-repo/harness/fix.yaml b/internal/scaffold/fullsend-repo/harness/fix.yaml index c5ea5487..251cd9a9 100644 --- a/internal/scaffold/fullsend-repo/harness/fix.yaml +++ b/internal/scaffold/fullsend-repo/harness/fix.yaml @@ -1,7 +1,7 @@ -# harness/fix.yaml — fix agent with pre/post script pipeline. +# harness/fix.yaml — fix agent with post script pipeline. # -# Flow: pre_script → sandbox (agent) → post_script -# pre_script : validates inputs, checks iteration cap +# Flow: (workflow: fullsend gate fix) → sandbox (agent) → post_script +# gate : validates inputs, checks iteration cap (runs on runner) # agent : reads pre-fetched review body, fixes code, tests, scans, commits # post_script : push commit, post summary comment on PR # @@ -12,8 +12,6 @@ model: opus image: ghcr.io/fullsend-ai/fullsend-code:latest policy: policies/fix.yaml -pre_script: scripts/pre-fix.sh - validation_loop: script: scripts/validate-output-schema.sh max_iterations: 2 diff --git a/internal/scaffold/fullsend-repo/scripts/pre-code.sh b/internal/scaffold/fullsend-repo/scripts/pre-code.sh deleted file mode 100755 index eae9c65b..00000000 --- a/internal/scaffold/fullsend-repo/scripts/pre-code.sh +++ /dev/null @@ -1,52 +0,0 @@ -#!/usr/bin/env bash -# Pre-script: validate workflow_dispatch inputs before the agent runs. -# -# Prevents malformed or malicious event_payload from reaching the sandbox. -# Runs on the GitHub Actions runner BEFORE sandbox creation. -# -# Required environment variables (set by the workflow): -# ISSUE_NUMBER — must be a positive integer -# REPO_FULL_NAME — must be owner/repo format -# GITHUB_ISSUE_URL — must be a valid GitHub issue URL -set -euo pipefail - -echo "::notice::🔗 Code target: ${GITHUB_ISSUE_URL:-}" - -errors=0 - -if [[ ! "${ISSUE_NUMBER:-}" =~ ^[1-9][0-9]*$ ]]; then - echo "::error::ISSUE_NUMBER must be a positive integer, got: '${ISSUE_NUMBER:-}'" - errors=$((errors + 1)) -fi - -if [[ ! "${REPO_FULL_NAME:-}" =~ ^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$ ]]; then - echo "::error::REPO_FULL_NAME must be owner/repo format, got: '${REPO_FULL_NAME:-}'" - errors=$((errors + 1)) -fi - -if [[ ! "${GITHUB_ISSUE_URL:-}" =~ ^https://github\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/issues/[0-9]+$ ]]; then - echo "::error::GITHUB_ISSUE_URL format invalid, got: '${GITHUB_ISSUE_URL:-}'" - errors=$((errors + 1)) -fi - -URL_REPO="$(echo "${GITHUB_ISSUE_URL:-}" | sed -E 's|https://github.com/([^/]+/[^/]+)/issues/.*|\1|')" -URL_ISSUE="$(echo "${GITHUB_ISSUE_URL:-}" | sed -E 's|.*/issues/([0-9]+)$|\1|')" - -if [[ -n "${URL_REPO}" && "${URL_REPO}" != "${REPO_FULL_NAME:-}" ]]; then - echo "::error::REPO_FULL_NAME does not match issue URL repo ('${REPO_FULL_NAME:-}' vs '${URL_REPO}')" - errors=$((errors + 1)) -fi -if [[ -n "${URL_ISSUE}" && "${URL_ISSUE}" != "${ISSUE_NUMBER:-}" ]]; then - echo "::error::ISSUE_NUMBER does not match issue URL number ('${ISSUE_NUMBER:-}' vs '${URL_ISSUE}')" - errors=$((errors + 1)) -fi - -if [[ "${errors}" -gt 0 ]]; then - echo "::error::Input validation failed with ${errors} error(s). Aborting." - exit 1 -fi - -echo "Input validation passed:" -echo " ISSUE_NUMBER=${ISSUE_NUMBER}" -echo " REPO_FULL_NAME=${REPO_FULL_NAME}" -echo " GITHUB_ISSUE_URL=${GITHUB_ISSUE_URL}" diff --git a/internal/scaffold/fullsend-repo/scripts/pre-fix.sh b/internal/scaffold/fullsend-repo/scripts/pre-fix.sh deleted file mode 100644 index 090696f5..00000000 --- a/internal/scaffold/fullsend-repo/scripts/pre-fix.sh +++ /dev/null @@ -1,101 +0,0 @@ -#!/usr/bin/env bash -# Pre-script: validate workflow_dispatch inputs before the fix agent runs. -# -# Prevents malformed or malicious event_payload from reaching the sandbox. -# Also enforces the iteration cap — blocks the run if too many fix cycles -# have already occurred on this PR. -# -# Required environment variables (set by the workflow): -# PR_NUMBER — must be a positive integer -# REPO_FULL_NAME — must be owner/repo format -# TRIGGER_SOURCE — GitHub username that triggered the fix (usernames ending in [bot] are bot triggers) -# -# Optional environment variables: -# FIX_ITERATION — current iteration count (default: 1) -# ITERATION_CAP — max bot-triggered iterations (default: 5) -# ITERATION_CAP_HUMAN — max human-triggered iterations (default: 10) -# HUMAN_INSTRUCTION — instruction text (only when TRIGGER_SOURCE doesn't end in [bot]) -set -euo pipefail - -# --------------------------------------------------------------------------- -# Helper: Bot user detection -# --------------------------------------------------------------------------- -is_bot_user() { - [[ "${1:-}" =~ \[bot\]$ ]] -} - -errors=0 - -# --------------------------------------------------------------------------- -# Input validation -# --------------------------------------------------------------------------- -if [[ ! "${PR_NUMBER:-}" =~ ^[1-9][0-9]*$ ]]; then - echo "::error::PR_NUMBER must be a positive integer, got: '${PR_NUMBER:-}'" - errors=$((errors + 1)) -fi - -if [[ ! "${REPO_FULL_NAME:-}" =~ ^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$ ]]; then - echo "::error::REPO_FULL_NAME must be owner/repo format, got: '${REPO_FULL_NAME:-}'" - errors=$((errors + 1)) -fi - -if [[ -z "${TRIGGER_SOURCE:-}" ]]; then - echo "::error::TRIGGER_SOURCE is required (GitHub username that triggered the fix)" - errors=$((errors + 1)) -fi - -if [[ "${errors}" -gt 0 ]]; then - echo "::error::Input validation failed with ${errors} error(s). Aborting." - exit 1 -fi - -# --------------------------------------------------------------------------- -# Human instruction length cap (defense against DoS via oversized input) -# --------------------------------------------------------------------------- -MAX_INSTRUCTION_BYTES=10000 -if ! is_bot_user "${TRIGGER_SOURCE}" && [[ -n "${HUMAN_INSTRUCTION:-}" ]]; then - INSTRUCTION_LEN="${#HUMAN_INSTRUCTION}" - if [[ "${INSTRUCTION_LEN}" -gt "${MAX_INSTRUCTION_BYTES}" ]]; then - echo "::error::HUMAN_INSTRUCTION is ${INSTRUCTION_LEN} bytes (max: ${MAX_INSTRUCTION_BYTES}). Truncate the instruction." - exit 1 - fi -fi - -# --------------------------------------------------------------------------- -# Iteration cap check -# --------------------------------------------------------------------------- -ITERATION="${FIX_ITERATION:-1}" -BOT_CAP="${ITERATION_CAP:-5}" -HUMAN_CAP="${ITERATION_CAP_HUMAN:-10}" - -if is_bot_user "${TRIGGER_SOURCE}"; then - CAP="${BOT_CAP}" -else - CAP="${HUMAN_CAP}" -fi - -if [[ "${ITERATION}" -gt "${CAP}" ]]; then - if is_bot_user "${TRIGGER_SOURCE}"; then - echo "::error::Fix iteration ${ITERATION} exceeds bot cap of ${CAP}. Escalating to human." - echo "::error::The review→fix loop has run ${ITERATION} times without converging." - echo "::error::A human can still direct the agent with /fix (up to ${HUMAN_CAP} total iterations)." - else - echo "::error::Fix iteration ${ITERATION} exceeds human cap of ${CAP}." - echo "::error::The /fix loop has run ${ITERATION} times. Further attempts are blocked." - fi - exit 1 -fi - -# --------------------------------------------------------------------------- -# Summary -# --------------------------------------------------------------------------- -echo "Input validation passed:" -echo " PR_NUMBER=${PR_NUMBER}" -echo " REPO_FULL_NAME=${REPO_FULL_NAME}" -echo " TRIGGER_SOURCE=${TRIGGER_SOURCE}" -echo " FIX_ITERATION=${ITERATION} of ${CAP}" -if ! is_bot_user "${TRIGGER_SOURCE}" && [[ -n "${HUMAN_INSTRUCTION:-}" ]]; then - # Truncate instruction in logs to avoid leaking long user input. - INSTR_PREVIEW="${HUMAN_INSTRUCTION:0:200}" - echo " HUMAN_INSTRUCTION=${INSTR_PREVIEW}..." -fi diff --git a/internal/scaffold/scaffold_test.go b/internal/scaffold/scaffold_test.go index ca313547..138f431c 100644 --- a/internal/scaffold/scaffold_test.go +++ b/internal/scaffold/scaffold_test.go @@ -35,7 +35,6 @@ func TestFullsendRepoFilesExist(t *testing.T) { "scripts/post-triage.sh", "scripts/pre-triage.sh", "scripts/scan-secrets", - "scripts/pre-code.sh", "scripts/pre-review.sh", "scripts/post-code.sh", "scripts/reconcile-repos.sh", @@ -137,7 +136,7 @@ func TestWalkFullsendRepo(t *testing.T) { return nil }) require.NoError(t, err) - assert.True(t, len(paths) >= 29, "expected at least 29 files, got %d", len(paths)) + assert.True(t, len(paths) >= 27, "expected at least 27 files, got %d", len(paths)) } func TestTriageWorkflowContent(t *testing.T) { @@ -180,7 +179,7 @@ func TestCodeWorkflowContent(t *testing.T) { s := string(content) assert.Contains(t, s, "workflow_dispatch") assert.Contains(t, s, "FULLSEND_CODER_CLIENT_ID") - assert.Contains(t, s, "pre-code.sh") + assert.Contains(t, s, "fullsend gate code") assert.Contains(t, s, "PUSH_TOKEN") assert.Contains(t, s, "github-app") assert.Contains(t, s, "sandbox-token") @@ -190,6 +189,9 @@ func TestCodeWorkflowContent(t *testing.T) { assert.Contains(t, s, "concurrency:") assert.Contains(t, s, "fullsend-code-") assert.Contains(t, s, "cancel-in-progress: true") + // Verify gate step has id and downstream steps check skip output + assert.Contains(t, s, "id: gate") + assert.Contains(t, s, "steps.gate.outputs.skip") } func TestReviewWorkflowContent(t *testing.T) { @@ -228,6 +230,10 @@ func TestFixWorkflowContent(t *testing.T) { assert.Contains(t, s, "concurrency:") assert.Contains(t, s, "fullsend-fix-") assert.Contains(t, s, "cancel-in-progress: true") + // Verify gate command replaced bash script + assert.Contains(t, s, "fullsend gate fix") + // Verify gate step has id for downstream skip checks + assert.Contains(t, s, "id: gate") } func TestCodeHarnessContent(t *testing.T) { @@ -235,7 +241,6 @@ func TestCodeHarnessContent(t *testing.T) { require.NoError(t, err) s := string(content) assert.Contains(t, s, "agents/code.md") - assert.Contains(t, s, "pre_script") assert.Contains(t, s, "post_script") assert.Contains(t, s, "runner_env") assert.Contains(t, s, "PUSH_TOKEN") @@ -329,3 +334,13 @@ func TestValidateTriageDeleted(t *testing.T) { _, err := FullsendRepoFile("scripts/validate-triage.sh") assert.Error(t, err, "validate-triage.sh should have been deleted") } + +func TestPreCodeShellScriptDeleted(t *testing.T) { + _, err := FullsendRepoFile("scripts/pre-code.sh") + assert.Error(t, err, "pre-code.sh should have been deleted — replaced by fullsend gate code") +} + +func TestPreFixShellScriptDeleted(t *testing.T) { + _, err := FullsendRepoFile("scripts/pre-fix.sh") + assert.Error(t, err, "pre-fix.sh should have been deleted — replaced by fullsend gate fix") +}