From 10ad2a88886909dba045ae3717b8d95058d4d9f8 Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:26:57 +0000 Subject: [PATCH 1/6] feat(#6214): add keep_history config option for sticky comments Add a per-repo config.yaml option (keep_history) that controls whether sticky comment updates append previous content as collapsed "Previous run"
blocks. When set to false, updates replace the comment body in-place with no history. This addresses the noise from accumulated history blocks when reading issues/PRs via the GitHub API and the rendering issue in Jira where
/ collapsible markup does not render as collapsible. Changes: - Add KeepHistory *bool to perRepoConfig using the pointer-bool pattern (nil = inherit from parent, code default true = history appended) - Add ConfigKeepHistory()/SetKeepHistory() to PerRepoConfigReader/Writer - Add KeepHistory bool to sticky.Config; BuildUpdatedBody short-circuits to replace-in-place when false, preserving footer content - Add --keep-history flag to post-review, post-comment, and issues post-comment CLI commands (default: true) - issues post-comment resolves the setting from config.yaml via --fullsend-dir when the flag is not explicitly set - Document the field in the layered config reference Closes #6214 --- .../layered-config-reference.md | 16 ++++ internal/cli/issues.go | 30 +++++- internal/cli/issues_test.go | 8 +- internal/cli/postcomment.go | 19 ++-- internal/cli/postreview.go | 23 +++-- internal/config/config.go | 9 ++ internal/config/defaults.go | 5 + internal/config/defaults_test.go | 92 +++++++++++++++++++ internal/config/interfaces.go | 22 +++++ internal/config/interfaces_test.go | 19 ++++ internal/sticky/sticky.go | 26 ++++++ internal/sticky/sticky_test.go | 73 +++++++++++++-- 12 files changed, 311 insertions(+), 31 deletions(-) diff --git a/docs/guides/infrastructure/layered-config-reference.md b/docs/guides/infrastructure/layered-config-reference.md index dde63dd879..5653e53389 100644 --- a/docs/guides/infrastructure/layered-config-reference.md +++ b/docs/guides/infrastructure/layered-config-reference.md @@ -74,6 +74,7 @@ the overlay → base → code defaults chain. | `version` | `string` | Scalar override | `"1"` | | `runtime` | `string` | Scalar override | `"claude"` | | `kill_switch` | `*bool` | Scalar override | `false` (inactive) | +| `keep_history` | `*bool` | Scalar override | `true` (history appended) | | `roles` | `[]string` | Replace if set | `PerRepoDefaultRoles()` | | `agents` | `[]AgentEntry` | Keyed merge by `DerivedName()` | `nil` (none) | | `allowed_remote_resources` | `[]string` | Union with deny-all | `DefaultAllowedRemoteResources()` | @@ -137,6 +138,20 @@ unset, the accessor falls through to the base layer, then to code defaults. - `*false` (explicit `kill_switch: false`) — locally set to inactive. Does **not** fall through. - `*true` (explicit `kill_switch: true`) — locally set to active. +- **`keep_history`**: Pointer to bool (`*bool`). Controls whether sticky + comment updates (from `post-review`, `post-comment`, and + `issues post-comment`) append the previous body as a collapsed + "Previous run" `
` block. Uses the same three-state pointer + semantics as `kill_switch`: + - `nil` (key omitted) — unset, falls through to parent. + Code default is `true` (history appended, preserving existing + behavior). + - `*true` (explicit `keep_history: true`) — updates collapse old + content into history blocks. + - `*false` (explicit `keep_history: false`) — updates replace the + comment body in-place with no history. Useful when accumulated + "Previous run" blocks add unwanted noise (e.g., when comments are + synced to Jira where `
` does not render as collapsible). ### `mint_url` and `inference` — scalar override (ADR 0069 Decision 1) @@ -343,6 +358,7 @@ compiled-in defaults apply: | `version` | `"1"` | | `runtime` | `"claude"` | | `kill_switch` | `false` (inactive) | +| `keep_history` | `true` (history appended) | | `roles` | `["triage", "coder", "review", "fix", "retro", "prioritize"]` | | `agents` | `nil` (none configured) | | `allowed_remote_resources` | `["https://raw.githubusercontent.com/fullsend-ai/fullsend/", "https://raw.githubusercontent.com/fullsend-ai/agents/"]` | diff --git a/internal/cli/issues.go b/internal/cli/issues.go index 1c5e386286..0223605ea0 100644 --- a/internal/cli/issues.go +++ b/internal/cli/issues.go @@ -178,6 +178,7 @@ type issuesPostCommentConfig struct { jiraURL string jiraEmail string dryRun bool + keepHistory *bool // nil = resolve from config; non-nil = explicit flag fullsendDir string // Test overrides — when non-nil, used instead of creating a real @@ -235,7 +236,7 @@ The --result flag accepts a file path or "-" for stdin.`, cmd.Flags().StringVar(&cfg.jiraURL, "jira-url", "", "Jira instance URL (default: $JIRA_BASE_URL)") cmd.Flags().StringVar(&cfg.jiraEmail, "jira-email", "", "Jira user email for Basic auth (default: $JIRA_USER_EMAIL)") cmd.Flags().BoolVar(&cfg.dryRun, "dry-run", false, "print what would be posted without making API calls") - cmd.Flags().StringVar(&cfg.fullsendDir, "fullsend-dir", "", "path to .fullsend config directory (sources a default --tracker from its config.yaml when --tracker is omitted)") + cmd.Flags().StringVar(&cfg.fullsendDir, "fullsend-dir", "", "path to .fullsend config directory (sources defaults from its config.yaml when flags are omitted)") _ = cmd.MarkFlagRequired("project") _ = cmd.MarkFlagRequired("number") _ = cmd.MarkFlagRequired("marker") @@ -279,9 +280,12 @@ func runIssuesPostComment(ctx context.Context, cfg *issuesPostCommentConfig) err printer.Header("Post Comment") + keepHistory := resolveKeepHistory(cfg.keepHistory, cfg.fullsendDir, cfg.testConfigReader) + stickyCfg := sticky.Config{ - Marker: cfg.marker, - DryRun: cfg.dryRun, + Marker: cfg.marker, + DryRun: cfg.dryRun, + KeepHistory: keepHistory, } if trackerName == trackerJira { // The Jira write path routes every body through @@ -478,6 +482,26 @@ func validateTrackerName(name string) (string, error) { return normalized, nil } +// resolveKeepHistory returns the explicit flag value if non-nil, otherwise +// resolves the keep_history setting from config.yaml via fullsendDir. If +// neither source provides a value, defaults to true (current behavior). +func resolveKeepHistory(flag *bool, fullsendDir string, testConfigReader config.PerRepoConfigReader) bool { + if flag != nil { + return *flag + } + prc := testConfigReader + if prc == nil && fullsendDir != "" { + reader, err := config.LoadConfig(fullsendDir, config.LoadOpts{MissingOK: true}) + if err == nil { + prc, _ = reader.(config.PerRepoConfigReader) + } + } + if prc != nil { + return prc.ConfigKeepHistory() + } + return true +} + // findMarkedTrackerComment returns the first tracker comment whose body // contains the given marker string, or nil if none is found. This is // the tracker.Comment equivalent of sticky.FindMarkedComment. diff --git a/internal/cli/issues_test.go b/internal/cli/issues_test.go index 433e166fc8..752e5077e3 100644 --- a/internal/cli/issues_test.go +++ b/internal/cli/issues_test.go @@ -105,7 +105,7 @@ func TestPostTrackerStickyComment_Create(t *testing.T) { tc := tracker.NewForgeClient(fc) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} url, err := postTrackerStickyComment(context.Background(), tc, "acme/widgets", 42, "hello world", cfg, printer) require.NoError(t, err) @@ -125,7 +125,7 @@ func TestPostTrackerStickyComment_Update(t *testing.T) { tc := tracker.NewForgeClient(fc) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} ctx := context.Background() // First post creates the comment. @@ -187,7 +187,7 @@ func TestPostTrackerStickyComment_DryRun_Update(t *testing.T) { fc.AuthenticatedUser = "bot" tc := tracker.NewForgeClient(fc) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} ctx := context.Background() // Create the initial comment (not dry run). @@ -727,7 +727,7 @@ func TestPostJiraStickyComment_DryRun_Update(t *testing.T) { tc, _, err := tracker.NewFakeJiraClientWithFake("https://acme.atlassian.net") require.NoError(t, err) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} ctx := context.Background() // Create the initial comment (not dry run). diff --git a/internal/cli/postcomment.go b/internal/cli/postcomment.go index 802d45f990..6be985c281 100644 --- a/internal/cli/postcomment.go +++ b/internal/cli/postcomment.go @@ -14,12 +14,13 @@ import ( func newPostCommentCmd() *cobra.Command { var ( - repo string - number int - marker string - result string - token string - dryRun bool + repo string + number int + marker string + result string + token string + dryRun bool + keepHistory bool ) cmd := &cobra.Command{ @@ -69,8 +70,9 @@ The --result flag accepts a file path or "-" for stdin.`, client := gh.New(token) cfg := sticky.Config{ - Marker: marker, - DryRun: dryRun, + Marker: marker, + DryRun: dryRun, + KeepHistory: keepHistory, } _, err = sticky.Post(cmd.Context(), client, owner, repoName, number, body, cfg, printer) return err @@ -83,6 +85,7 @@ The --result flag accepts a file path or "-" for stdin.`, cmd.Flags().StringVar(&result, "result", "-", "path to comment body file, or '-' for stdin") cmd.Flags().StringVar(&token, "token", "", "GitHub token (default: $GITHUB_TOKEN)") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print what would be posted without making API calls") + cmd.Flags().BoolVar(&keepHistory, "keep-history", true, "append previous content as collapsed history blocks (set false to replace in-place)") _ = cmd.MarkFlagRequired("repo") _ = cmd.MarkFlagRequired("number") _ = cmd.MarkFlagRequired("marker") diff --git a/internal/cli/postreview.go b/internal/cli/postreview.go index e5f8b1cfd4..98db810b2f 100644 --- a/internal/cli/postreview.go +++ b/internal/cli/postreview.go @@ -37,14 +37,15 @@ var hunkHeaderRe = regexp.MustCompile(`^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@`) func newPostReviewCmd() *cobra.Command { var ( - repo string - pr int - result string - token string - headSHA string - dryRun bool - forgeName string - baseURL string + repo string + pr int + result string + token string + headSHA string + dryRun bool + forgeName string + baseURL string + keepHistory bool ) cmd := &cobra.Command{ @@ -114,8 +115,9 @@ GITLAB_TOKEN for GitLab and GH_TOKEN / GITHUB_TOKEN for GitHub.`, return err } cfg := sticky.Config{ - Marker: reviewMarker, - DryRun: dryRun, + Marker: reviewMarker, + DryRun: dryRun, + KeepHistory: keepHistory, } // Stale-head check: refuse to post a review against code @@ -155,6 +157,7 @@ GITLAB_TOKEN for GitLab and GH_TOKEN / GITHUB_TOKEN for GitHub.`, cmd.Flags().StringVar(&token, "token", "", "forge token (default: $GH_TOKEN / $GITHUB_TOKEN for GitHub, $GITLAB_TOKEN for GitLab)") cmd.Flags().StringVar(&headSHA, "head-sha", "", "expected PR HEAD SHA (skips review if HEAD has moved)") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print what would be posted without making API calls") + cmd.Flags().BoolVar(&keepHistory, "keep-history", true, "append previous content as collapsed history blocks (set false to replace in-place)") cmd.Flags().StringVar(&forgeName, "forge", "", "forge backend: github (default) or gitlab") cmd.Flags().StringVar(&baseURL, "base-url", "", "forge instance URL (e.g. https://gitlab.example.com)") _ = cmd.MarkFlagRequired("repo") diff --git a/internal/config/config.go b/internal/config/config.go index d057608302..781f938bfa 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -769,6 +769,13 @@ type perRepoConfig struct { // method sharing a name on the same type. Notifications *StatusNotificationConfig `yaml:"status_notifications,omitempty"` + // KeepHistory controls whether sticky comment updates append the + // previous body as a collapsed "Previous run"
block. When + // nil (omitted), falls through to parent (code default true — + // history appended). When explicitly false, updates replace the body + // in-place with no history. + KeepHistory *bool `yaml:"keep_history,omitempty"` + // Mint URL for token minting (ADR 0069 Decision 1). MintURL string `yaml:"mint_url,omitempty"` @@ -953,6 +960,7 @@ type perRepoConfigMarshal struct { Tracker string `yaml:"tracker,omitempty"` KillSwitch *bool `yaml:"kill_switch,omitempty"` Runtime string `yaml:"runtime,omitempty"` + KeepHistory *bool `yaml:"keep_history,omitempty"` Roles *[]string `yaml:"roles,omitempty"` Agents []AgentEntry `yaml:"agents,omitempty"` AllowedRemoteResources *[]string `yaml:"allowed_remote_resources,omitempty"` @@ -974,6 +982,7 @@ func (c *perRepoConfig) MarshalYAML() (interface{}, error) { Tracker: c.Tracker, KillSwitch: c.KillSwitch, Runtime: c.Runtime, + KeepHistory: c.KeepHistory, Agents: c.Agents, CreateIssues: c.CreateIssues, StatusNotifications: c.Notifications, diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 6be134ec0b..8da1abe2ab 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -79,6 +79,11 @@ func (d *perRepoDefaults) ConfigInferenceRegion() string { return DefaultPerRepo // must be provided by the installer or existing secret). func (d *perRepoDefaults) ConfigInferenceWIFProvider() string { return "" } +// ConfigKeepHistory returns the default keep-history state (true — +// sticky comment updates append previous content as "Previous run" +// blocks, preserving the pre-existing behavior). +func (d *perRepoDefaults) ConfigKeepHistory() bool { return true } + // ConfigInferenceOpenAI returns the default OpenAI WIF identifiers (none — // set by `fullsend github setup --openai-*` or the FULLSEND_OPENAI_* // runner variables). diff --git a/internal/config/defaults_test.go b/internal/config/defaults_test.go index 77a03863c8..bfc91058dc 100644 --- a/internal/config/defaults_test.go +++ b/internal/config/defaults_test.go @@ -22,6 +22,7 @@ func TestPerRepoDefaults_CodeDefaults(t *testing.T) { assert.Equal(t, "1", d.ConfigVersion()) assert.Equal(t, "claude", d.ConfigRuntime()) assert.False(t, d.IsKillSwitchActive()) + assert.True(t, d.ConfigKeepHistory()) assert.Equal(t, PerRepoDefaultRoles(), d.ConfigRoles()) assert.Nil(t, d.AgentEntries()) assert.Equal(t, DefaultAllowedRemoteResources(), d.AllowedResources()) @@ -45,6 +46,7 @@ func TestPerRepoConfig_EmptyConfigResolvesDefaults(t *testing.T) { assert.Equal(t, "1", cfg.ConfigVersion()) assert.Equal(t, "claude", cfg.ConfigRuntime()) assert.False(t, cfg.IsKillSwitchActive()) + assert.True(t, cfg.ConfigKeepHistory()) assert.Equal(t, PerRepoDefaultRoles(), cfg.ConfigRoles()) assert.Nil(t, cfg.AgentEntries()) assert.Equal(t, DefaultAllowedRemoteResources(), cfg.AllowedResources()) @@ -148,6 +150,39 @@ func TestPerRepoConfig_KillSwitch_PointerSemantics(t *testing.T) { }) } +// --- KeepHistory *bool pointer semantics --- + +func TestPerRepoConfig_KeepHistory_PointerSemantics(t *testing.T) { + t.Run("nil falls through to parent default true", func(t *testing.T) { + cfg := &perRepoConfig{parent: &perRepoDefaults{}} + assert.True(t, cfg.ConfigKeepHistory()) + }) + + t.Run("explicit false does not fall through", func(t *testing.T) { + // Parent has keep_history=true (default), overlay explicitly sets false. + f := false + overlay := &perRepoConfig{ + KeepHistory: &f, + parent: &perRepoDefaults{}, + } + assert.False(t, overlay.ConfigKeepHistory()) + }) + + t.Run("explicit true overrides parent false", func(t *testing.T) { + f := false + parentCfg := &perRepoConfig{ + KeepHistory: &f, + parent: &perRepoDefaults{}, + } + tr := true + overlay := &perRepoConfig{ + KeepHistory: &tr, + parent: parentCfg, + } + assert.True(t, overlay.ConfigKeepHistory()) + }) +} + // --- Agents keyed merge by DerivedName --- func TestPerRepoConfig_AgentsMerge(t *testing.T) { @@ -462,6 +497,7 @@ func TestPerRepoConfig_MarshalOmitsInheritedValues(t *testing.T) { assert.NotContains(t, output, "version:") assert.NotContains(t, output, "runtime:") assert.NotContains(t, output, "kill_switch:") + assert.NotContains(t, output, "keep_history:") assert.NotContains(t, output, "roles:") assert.NotContains(t, output, "agents:") assert.NotContains(t, output, "allowed_remote_resources:") @@ -503,6 +539,19 @@ func TestPerRepoConfig_MarshalExplicitFalseKillSwitch(t *testing.T) { assert.Contains(t, string(data), "kill_switch: false") } +func TestPerRepoConfig_MarshalExplicitFalseKeepHistory(t *testing.T) { + f := false + cfg := &perRepoConfig{ + Version: "1", + KeepHistory: &f, + parent: &perRepoDefaults{}, + } + data, err := cfg.Marshal() + require.NoError(t, err) + // Explicit false should appear in output (distinguishable from unset). + assert.Contains(t, string(data), "keep_history: false") +} + func TestPerRepoConfig_MarshalDenyAll(t *testing.T) { cfg := &perRepoConfig{ Version: "1", @@ -761,3 +810,46 @@ roles: assert.Nil(t, prc.KillSwitch) }) } + +// --- KeepHistory YAML round-trip --- + +func TestPerRepoConfig_KeepHistory_YAMLRoundTrip(t *testing.T) { + t.Run("keep_history false round-trips", func(t *testing.T) { + yamlData := `version: "1" +keep_history: false +roles: + - triage +` + cfg, err := ParsePerRepoConfig([]byte(yamlData)) + require.NoError(t, err) + assert.False(t, cfg.ConfigKeepHistory()) + // Verify it was explicitly set (not inherited). + prc := cfg.(*perRepoConfig) + require.NotNil(t, prc.KeepHistory) + assert.False(t, *prc.KeepHistory) + }) + + t.Run("keep_history true round-trips", func(t *testing.T) { + yamlData := `version: "1" +keep_history: true +roles: + - triage +` + cfg, err := ParsePerRepoConfig([]byte(yamlData)) + require.NoError(t, err) + assert.True(t, cfg.ConfigKeepHistory()) + }) + + t.Run("keep_history omitted falls through to default true", func(t *testing.T) { + yamlData := `version: "1" +roles: + - triage +` + cfg, err := ParsePerRepoConfig([]byte(yamlData)) + require.NoError(t, err) + assert.True(t, cfg.ConfigKeepHistory()) + // Verify it was not set. + prc := cfg.(*perRepoConfig) + assert.Nil(t, prc.KeepHistory) + }) +} diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go index 505a9e5547..f13344e0db 100644 --- a/internal/config/interfaces.go +++ b/internal/config/interfaces.go @@ -77,6 +77,7 @@ type PerRepoConfigReader interface { ConfigForge() string ConfigTracker() string ConfigMintURL() string + ConfigKeepHistory() bool ConfigInferenceProvider() string ConfigInferenceProject() string ConfigInferenceRegion() string @@ -117,6 +118,7 @@ type PerRepoConfigWriter interface { SetRoles([]string) SetRuntime(string) SetMintURL(string) + SetKeepHistory(bool) SetInferenceProvider(string) SetInferenceProject(string) SetInferenceRegion(string) @@ -429,6 +431,21 @@ func (c *perRepoConfig) ConfigForge() string { return "" } +// ConfigKeepHistory reports whether sticky comment updates should +// append previous content as a collapsed "Previous run" block. +// KeepHistory is a *bool: nil falls through to parent, non-nil +// (including explicit false) is the local decision. Code default +// is true (history appended). +func (c *perRepoConfig) ConfigKeepHistory() bool { + if c.KeepHistory != nil { + return *c.KeepHistory + } + if c.parent != nil { + return c.parent.ConfigKeepHistory() + } + return true +} + // ConfigTracker returns the configured default issue tracker (e.g. // "github", "gitlab", "jira"), used as the default for `fullsend // issues` commands' --tracker flag when it is not set explicitly. @@ -543,6 +560,11 @@ func (c *perRepoConfig) SetRoles(roles []string) { c.Roles = roles } // SetRuntime replaces the configured agent runtime. func (c *perRepoConfig) SetRuntime(runtime string) { c.Runtime = runtime } +// SetKeepHistory sets whether sticky comment updates append history. +// Stores a *bool so that an explicit false is distinguishable from +// unset (nil) across layers. +func (c *perRepoConfig) SetKeepHistory(v bool) { c.KeepHistory = &v } + // SetMintURL sets the token mint URL. func (c *perRepoConfig) SetMintURL(mintURL string) { c.MintURL = mintURL } diff --git a/internal/config/interfaces_test.go b/internal/config/interfaces_test.go index 435e1e164b..4bbe773ab9 100644 --- a/internal/config/interfaces_test.go +++ b/internal/config/interfaces_test.go @@ -168,6 +168,15 @@ func TestPerRepoConfig_IsKillSwitchActive(t *testing.T) { assert.False(t, cfg.IsKillSwitchActive()) } +func TestPerRepoConfig_ConfigKeepHistory(t *testing.T) { + f := false + cfg := &perRepoConfig{KeepHistory: &f} + assert.False(t, cfg.ConfigKeepHistory()) + tr := true + cfg.KeepHistory = &tr + assert.True(t, cfg.ConfigKeepHistory()) +} + func TestPerRepoConfig_AllowedResources(t *testing.T) { resources := []string{"https://example.com/"} cfg := &perRepoConfig{AllowedRemoteResources: resources} @@ -238,6 +247,16 @@ func TestPerRepoConfig_SetKillSwitch(t *testing.T) { assert.False(t, *cfg.KillSwitch) } +func TestPerRepoConfig_SetKeepHistory(t *testing.T) { + cfg := &perRepoConfig{} + cfg.SetKeepHistory(false) + require.NotNil(t, cfg.KeepHistory) + assert.False(t, *cfg.KeepHistory) + cfg.SetKeepHistory(true) + require.NotNil(t, cfg.KeepHistory) + assert.True(t, *cfg.KeepHistory) +} + func TestPerRepoConfig_SetAgents(t *testing.T) { cfg := &perRepoConfig{} agents := []AgentEntry{{Source: "harness/code.yaml"}} diff --git a/internal/sticky/sticky.go b/internal/sticky/sticky.go index 3d2d735405..0b0a014862 100644 --- a/internal/sticky/sticky.go +++ b/internal/sticky/sticky.go @@ -20,6 +20,7 @@ type Config struct { FooterMarker string // optional footer delimiter, stripped before collapsing history MaxSize int // max comment body size (default 65000) DryRun bool + KeepHistory bool // when false, updates replace the body in-place with no "Previous run" history } func (c Config) maxSize() int { @@ -119,7 +120,32 @@ var legacyDetailsRe = regexp.MustCompile(`(?s)
\s*Previous [^<] // BuildUpdatedBody collapses the old comment body into a flat list of //
blocks and prepends the new body. Footer content (delimited // by FooterMarker) is stripped before collapsing and re-appended after. +// +// When cfg.KeepHistory is false, the old body is discarded entirely and +// the new body is returned with footer re-appended (no "Previous run" +// blocks). func BuildUpdatedBody(oldBody, newBody string, cfg Config) string { + // When history is disabled, replace the body entirely. We still + // need to preserve the footer from the old body if configured. + if !cfg.KeepHistory { + var footer string + if cfg.FooterMarker != "" { + stripped, _ := strings.CutPrefix(oldBody, cfg.Marker+"\n") + stripped, _ = strings.CutPrefix(stripped, cfg.Marker) + if idx := strings.Index(stripped, cfg.FooterMarker); idx >= 0 { + footer = stripped[idx:] + } + } + result := newBody + if footer != "" { + result += "\n\n" + footer + } + if len(result) > cfg.maxSize() { + result = TruncateBody(result, cfg.maxSize()) + } + return result + } + // Strip marker from the old body (prefix-only to avoid matching // the marker if it appears embedded in review content). oldContent, _ := strings.CutPrefix(oldBody, cfg.Marker+"\n") diff --git a/internal/sticky/sticky_test.go b/internal/sticky/sticky_test.go index ec3d0ad5bd..e65bf17b63 100644 --- a/internal/sticky/sticky_test.go +++ b/internal/sticky/sticky_test.go @@ -17,6 +17,7 @@ import ( var testCfg = Config{ Marker: "", FooterMarker: "", + KeepHistory: true, } func TestFindMarkedComment(t *testing.T) { @@ -73,7 +74,7 @@ func TestBuildUpdatedBody_CollapsesOldContent(t *testing.T) { } func TestBuildUpdatedBody_FlatHistory(t *testing.T) { - cfg := Config{Marker: ""} + cfg := Config{Marker: "", KeepHistory: true} // Run 1 → Run 2 body1 := "\nRun 1 content." @@ -113,7 +114,7 @@ func TestBuildUpdatedBody_FlatHistory(t *testing.T) { } func TestBuildUpdatedBody_NestedDetailsInContent(t *testing.T) { - cfg := Config{Marker: ""} + cfg := Config{Marker: "", KeepHistory: true} // Run 1 content contains a
block (common in GitHub review output). body1 := "\nReview findings:\n
\nExpanded diff\nsome diff content\n
\nEnd of review." @@ -151,7 +152,7 @@ func TestBuildUpdatedBody_FooterStripping(t *testing.T) { } func TestBuildUpdatedBody_NoFooterMarker(t *testing.T) { - cfg := Config{Marker: ""} + cfg := Config{Marker: "", KeepHistory: true} oldBody := "\nOld content." newBody := "\nNew content." @@ -161,6 +162,66 @@ func TestBuildUpdatedBody_NoFooterMarker(t *testing.T) { assert.Contains(t, result, "Old content.") } +func TestBuildUpdatedBody_NoHistory(t *testing.T) { + cfg := Config{ + Marker: "", + KeepHistory: false, + } + oldBody := "\nold content" + newBody := "\nnew content" + + got := BuildUpdatedBody(oldBody, newBody, cfg) + + if strings.Contains(got, "
") { + t.Error("expected no history block when KeepHistory is false") + } + if !strings.Contains(got, "new content") { + t.Error("expected new content in result") + } + if strings.Contains(got, "old content") { + t.Error("expected old content to be discarded when KeepHistory is false") + } +} + +func TestBuildUpdatedBody_NoHistory_PreservesFooter(t *testing.T) { + cfg := Config{ + Marker: "", + FooterMarker: "", + KeepHistory: false, + } + oldBody := "\nOld review.\n\n\n_some footer info_" + newBody := "\nNew review." + + got := BuildUpdatedBody(oldBody, newBody, cfg) + + assert.Contains(t, got, "New review.") + assert.NotContains(t, got, "Old review.") + assert.NotContains(t, got, "
") + assert.Contains(t, got, "") + assert.Contains(t, got, "_some footer info_") +} + +func TestBuildUpdatedBody_NoHistory_MultipleRuns(t *testing.T) { + cfg := Config{Marker: "", KeepHistory: false} + + body1 := "\nRun 1 content." + body2 := "\nRun 2 content." + result2 := BuildUpdatedBody(body1, body2, cfg) + + // After update, only run 2 should be present. + assert.Contains(t, result2, "Run 2 content.") + assert.NotContains(t, result2, "Run 1 content.") + assert.NotContains(t, result2, "
") + + // Run 2 → Run 3 + body3 := "\nRun 3 content." + result3 := BuildUpdatedBody(result2, body3, cfg) + + assert.Contains(t, result3, "Run 3 content.") + assert.NotContains(t, result3, "Run 2 content.") + assert.NotContains(t, result3, "
") +} + func TestTruncateBody_UnderLimit(t *testing.T) { body := "short body" assert.Equal(t, body, TruncateBody(body, defaultMaxSize)) @@ -174,7 +235,7 @@ func TestTruncateBody_OverLimit(t *testing.T) { } func TestBuildUpdatedBody_DropsOldestHistoryOnOverflow(t *testing.T) { - cfg := Config{Marker: "", MaxSize: 500} + cfg := Config{Marker: "", MaxSize: 500, KeepHistory: true} body1 := "\n" + strings.Repeat("A", 100) body2 := "\n" + strings.Repeat("B", 100) @@ -229,7 +290,7 @@ func TestPost_UpdateExisting(t *testing.T) { } printer := ui.New(io.Discard) - cfg := Config{Marker: ""} + cfg := Config{Marker: "", KeepHistory: true} commentURL, err := Post(context.Background(), client, "o", "r", 1, "New.", cfg, printer) require.NoError(t, err) assert.Equal(t, "https://github.com/o/r/issues/1#issuecomment-100", commentURL) @@ -292,7 +353,7 @@ func TestPost_UpdateExisting_EmptyHTMLURL(t *testing.T) { } printer := ui.New(io.Discard) - cfg := Config{Marker: ""} + cfg := Config{Marker: "", KeepHistory: true} commentURL, err := Post(context.Background(), client, "o", "r", 1, "New.", cfg, printer) require.NoError(t, err) assert.Empty(t, commentURL, "should return empty URL when existing comment has no HTMLURL") From ae6805c0c56ff59562d50e18174b184a48d82e8f Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:06:53 +0000 Subject: [PATCH 2/6] fix: address review feedback on PR #6930 - Add --fullsend-dir config resolution to post-review (scope-gap): post-review now resolves keep_history from config.yaml when --keep-history is not explicitly set, matching issues post-comment. - Surface config load errors in resolveKeepHistory (error-handling-gap): return (bool, error) matching the resolveTracker pattern; callers warn on failure instead of silently defaulting. - Align KeepHistory field ordering in perRepoConfig (field-ordering): moved after Runtime to match perRepoConfigMarshal. - Update cli-internals.md with --keep-history and --fullsend-dir flags for post-review (stale-doc). - Update issues-commands.md: --fullsend-dir description now says "sources defaults" (plural), and issues post-comment description mentions keep_history opt-out (stale-doc). Addresses #6930 --- docs/guides/dev/cli-internals.md | 4 +++- docs/guides/user/issues-commands.md | 12 +++++++----- internal/cli/issues.go | 20 +++++++++++++------- internal/cli/postreview.go | 18 +++++++++++++++++- internal/config/config.go | 25 +++++++++++++------------ 5 files changed, 53 insertions(+), 26 deletions(-) diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index 66a8ea616e..12615642cb 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -142,7 +142,9 @@ fullsend │ ├── --result # Path to review result file, or '-' for stdin │ ├── --token # Forge token (default: $GH_TOKEN / $GITHUB_TOKEN or $GITLAB_TOKEN) │ ├── --head-sha # Expected PR HEAD SHA (skips review if HEAD moved) -│ └── --dry-run # Print what would be posted without API calls +│ ├── --dry-run # Print what would be posted without API calls +│ ├── --keep-history # Append previous content as collapsed history (default true) +│ └── --fullsend-dir # .fullsend config directory (resolves keep_history default) ├── post-comment # Post issue/PR comments to GitHub (deprecated) ├── eval-measure # Score wild-run traces (eval measurements) │ ├── --telemetry # Path to run-telemetry.jsonl (or --output-dir) diff --git a/docs/guides/user/issues-commands.md b/docs/guides/user/issues-commands.md index 8c91e6718f..49dfc2d400 100644 --- a/docs/guides/user/issues-commands.md +++ b/docs/guides/user/issues-commands.md @@ -49,14 +49,16 @@ fullsend issues get \ | `--token` | No | API token (default: env var per tracker) | | `--jira-url` | Jira only | Jira instance URL (default: `$JIRA_BASE_URL`) | | `--jira-email` | Jira only | Jira user email for auth (default: `$JIRA_USER_EMAIL`) | -| `--fullsend-dir` | No | Path to `.fullsend` config directory (sources a default `--tracker` from its `config.yaml`) | +| `--fullsend-dir` | No | Path to `.fullsend` config directory (sources defaults from its `config.yaml` when flags are omitted) | ## `fullsend issues post-comment` Posts a comment with a sticky marker on an issue. On re-runs, finds -the existing comment by its marker and edits in-place, collapsing old -content into `
` blocks. This prevents comment flooding on -re-runs. For GitHub and GitLab, the marker is embedded as an invisible +the existing comment by its marker and edits in-place. By default, +old content is collapsed into `
` blocks to preserve history; +set `keep_history: false` in config.yaml (or pass `--keep-history=false` +on `post-review` / `post-comment`) to replace the body with no history. +This prevents comment flooding on re-runs. For GitHub and GitLab, the marker is embedded as an invisible HTML comment in the body. For Jira, the marker is stored as a comment entity property (Jira has no HTML comments, so a body-embedded marker would be visible to users). @@ -84,7 +86,7 @@ echo "Triage complete. See PR #99." | fullsend issues post-comment \ | `--jira-url` | Jira only | Jira instance URL (default: `$JIRA_BASE_URL`) | | `--jira-email` | Jira only | Jira user email for auth (default: `$JIRA_USER_EMAIL`) | | `--dry-run` | No | Print what would be posted without making API calls | -| `--fullsend-dir` | No | Path to `.fullsend` config directory (sources a default `--tracker` from its `config.yaml`) | +| `--fullsend-dir` | No | Path to `.fullsend` config directory (sources defaults from its `config.yaml` when flags are omitted) | ### Jira marker storage diff --git a/internal/cli/issues.go b/internal/cli/issues.go index 0223605ea0..3dea1856aa 100644 --- a/internal/cli/issues.go +++ b/internal/cli/issues.go @@ -280,7 +280,10 @@ func runIssuesPostComment(ctx context.Context, cfg *issuesPostCommentConfig) err printer.Header("Post Comment") - keepHistory := resolveKeepHistory(cfg.keepHistory, cfg.fullsendDir, cfg.testConfigReader) + keepHistory, err := resolveKeepHistory(cfg.keepHistory, cfg.fullsendDir, cfg.testConfigReader) + if err != nil { + printer.StepWarn(fmt.Sprintf("Warning: %v; defaulting to keep_history=true", err)) + } stickyCfg := sticky.Config{ Marker: cfg.marker, @@ -485,21 +488,24 @@ func validateTrackerName(name string) (string, error) { // resolveKeepHistory returns the explicit flag value if non-nil, otherwise // resolves the keep_history setting from config.yaml via fullsendDir. If // neither source provides a value, defaults to true (current behavior). -func resolveKeepHistory(flag *bool, fullsendDir string, testConfigReader config.PerRepoConfigReader) bool { +// Returns an error when config loading fails so callers can surface it +// (matching the pattern in resolveTracker). +func resolveKeepHistory(flag *bool, fullsendDir string, testConfigReader config.PerRepoConfigReader) (bool, error) { if flag != nil { - return *flag + return *flag, nil } prc := testConfigReader if prc == nil && fullsendDir != "" { reader, err := config.LoadConfig(fullsendDir, config.LoadOpts{MissingOK: true}) - if err == nil { - prc, _ = reader.(config.PerRepoConfigReader) + if err != nil { + return true, fmt.Errorf("loading config for keep_history: %w", err) } + prc, _ = reader.(config.PerRepoConfigReader) } if prc != nil { - return prc.ConfigKeepHistory() + return prc.ConfigKeepHistory(), nil } - return true + return true, nil } // findMarkedTrackerComment returns the first tracker comment whose body diff --git a/internal/cli/postreview.go b/internal/cli/postreview.go index 98db810b2f..0d03a4c829 100644 --- a/internal/cli/postreview.go +++ b/internal/cli/postreview.go @@ -46,6 +46,7 @@ func newPostReviewCmd() *cobra.Command { forgeName string baseURL string keepHistory bool + fullsendDir string ) cmd := &cobra.Command{ @@ -114,10 +115,24 @@ GITLAB_TOKEN for GitLab and GH_TOKEN / GITHUB_TOKEN for GitHub.`, if err != nil { return err } + + // Resolve keep_history: explicit --keep-history flag takes + // precedence, otherwise fall back to config.yaml via + // --fullsend-dir (matching the pattern in issues post-comment). + resolvedKeepHistory := keepHistory + if !cmd.Flags().Changed("keep-history") { + var khFlag *bool // nil = not explicitly set + resolved, resolveErr := resolveKeepHistory(khFlag, fullsendDir, nil) + if resolveErr != nil { + printer.StepWarn(fmt.Sprintf("Warning: %v; defaulting to keep_history=true", resolveErr)) + } + resolvedKeepHistory = resolved + } + cfg := sticky.Config{ Marker: reviewMarker, DryRun: dryRun, - KeepHistory: keepHistory, + KeepHistory: resolvedKeepHistory, } // Stale-head check: refuse to post a review against code @@ -158,6 +173,7 @@ GITLAB_TOKEN for GitLab and GH_TOKEN / GITHUB_TOKEN for GitHub.`, cmd.Flags().StringVar(&headSHA, "head-sha", "", "expected PR HEAD SHA (skips review if HEAD has moved)") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print what would be posted without making API calls") cmd.Flags().BoolVar(&keepHistory, "keep-history", true, "append previous content as collapsed history blocks (set false to replace in-place)") + cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", "", "path to .fullsend config directory (sources defaults from its config.yaml when flags are omitted)") cmd.Flags().StringVar(&forgeName, "forge", "", "forge backend: github (default) or gitlab") cmd.Flags().StringVar(&baseURL, "base-url", "", "forge instance URL (e.g. https://gitlab.example.com)") _ = cmd.MarkFlagRequired("repo") diff --git a/internal/config/config.go b/internal/config/config.go index 781f938bfa..47a7245740 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -751,11 +751,19 @@ type perRepoConfig struct { // jira) for `fullsend issues` commands' --tracker flag. Distinct // from Forge, which is the repo's hosting platform — a repo can be // hosted on GitHub but track issues in Jira. - Tracker string `yaml:"tracker,omitempty"` - KillSwitch *bool `yaml:"kill_switch,omitempty"` - Runtime string `yaml:"runtime,omitempty"` - Roles []string `yaml:"roles,omitempty"` - Agents []AgentEntry `yaml:"agents,omitempty"` + Tracker string `yaml:"tracker,omitempty"` + KillSwitch *bool `yaml:"kill_switch,omitempty"` + Runtime string `yaml:"runtime,omitempty"` + + // KeepHistory controls whether sticky comment updates append the + // previous body as a collapsed "Previous run"
block. When + // nil (omitted), falls through to parent (code default true — + // history appended). When explicitly false, updates replace the body + // in-place with no history. + KeepHistory *bool `yaml:"keep_history,omitempty"` + + Roles []string `yaml:"roles,omitempty"` + Agents []AgentEntry `yaml:"agents,omitempty"` // AllowedRemoteResources holds the locally-set allowed remote // resource prefixes. MarshalYAML preserves the nil-vs-empty // distinction: nil (unset) is omitted, empty (deny-all) is @@ -769,13 +777,6 @@ type perRepoConfig struct { // method sharing a name on the same type. Notifications *StatusNotificationConfig `yaml:"status_notifications,omitempty"` - // KeepHistory controls whether sticky comment updates append the - // previous body as a collapsed "Previous run"
block. When - // nil (omitted), falls through to parent (code default true — - // history appended). When explicitly false, updates replace the body - // in-place with no history. - KeepHistory *bool `yaml:"keep_history,omitempty"` - // Mint URL for token minting (ADR 0069 Decision 1). MintURL string `yaml:"mint_url,omitempty"` From 55e49fd6a684e529edb3070fd61d2e6f9df5c48e Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 21:41:32 +0000 Subject: [PATCH 3/6] fix: register --keep-history flag on issues post-comment for CLI parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The issuesPostCommentConfig struct had a keepHistory *bool field that was only resolved from config.yaml — no CLI flag was bound to it. This contradicted the PR description which claimed --keep-history was added to all three sticky comment commands. Register --keep-history on issues post-comment using the same Changed()-guard pattern as post-review so explicit flags take precedence over config.yaml resolution. Update docs to remove the confusing circular reference to the deprecated post-comment command and add --keep-history / --fullsend-dir to the CLI command tree. Addresses #6930 --- docs/guides/dev/cli-internals.md | 4 +++- docs/guides/user/issues-commands.md | 5 +++-- internal/cli/issues.go | 9 ++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index 12615642cb..2b938df33d 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -133,7 +133,9 @@ fullsend │ ├── --tracker # Tracker backend: github, gitlab, or jira │ ├── --project # Project: owner/repo (GitHub/GitLab) or key (Jira) │ ├── --number # Issue number -│ └── --marker # Sticky marker for idempotent updates (HTML comment or Jira property) +│ ├── --marker # Sticky marker for idempotent updates (HTML comment or Jira property) +│ ├── --keep-history # Append previous content as collapsed history (default true) +│ └── --fullsend-dir # .fullsend config directory (resolves keep_history default) ├── post-review # Post PR/MR review comments to GitHub or GitLab │ ├── --forge # Forge backend: github (default) or gitlab │ ├── --base-url # Forge instance URL (e.g. https://gitlab.example.com) diff --git a/docs/guides/user/issues-commands.md b/docs/guides/user/issues-commands.md index 49dfc2d400..0956c825d6 100644 --- a/docs/guides/user/issues-commands.md +++ b/docs/guides/user/issues-commands.md @@ -56,8 +56,8 @@ fullsend issues get \ Posts a comment with a sticky marker on an issue. On re-runs, finds the existing comment by its marker and edits in-place. By default, old content is collapsed into `
` blocks to preserve history; -set `keep_history: false` in config.yaml (or pass `--keep-history=false` -on `post-review` / `post-comment`) to replace the body with no history. +set `keep_history: false` in config.yaml (or pass `--keep-history=false`) +to replace the body with no history. This prevents comment flooding on re-runs. For GitHub and GitLab, the marker is embedded as an invisible HTML comment in the body. For Jira, the marker is stored as a comment entity property (Jira has no HTML comments, so a body-embedded marker @@ -86,6 +86,7 @@ echo "Triage complete. See PR #99." | fullsend issues post-comment \ | `--jira-url` | Jira only | Jira instance URL (default: `$JIRA_BASE_URL`) | | `--jira-email` | Jira only | Jira user email for auth (default: `$JIRA_USER_EMAIL`) | | `--dry-run` | No | Print what would be posted without making API calls | +| `--keep-history` | No | Append previous content as collapsed history blocks (default: `true`; set `false` to replace in-place) | | `--fullsend-dir` | No | Path to `.fullsend` config directory (sources defaults from its `config.yaml` when flags are omitted) | ### Jira marker storage diff --git a/internal/cli/issues.go b/internal/cli/issues.go index 3dea1856aa..928c1e8e27 100644 --- a/internal/cli/issues.go +++ b/internal/cli/issues.go @@ -190,7 +190,10 @@ type issuesPostCommentConfig struct { } func newIssuesPostCommentCmd() *cobra.Command { - var cfg issuesPostCommentConfig + var ( + cfg issuesPostCommentConfig + keepHistory bool + ) cmd := &cobra.Command{ Use: "post-comment", @@ -223,6 +226,9 @@ pointing at the directory containing it. The --result flag accepts a file path or "-" for stdin.`, RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("keep-history") { + cfg.keepHistory = &keepHistory + } return runIssuesPostComment(cmd.Context(), &cfg) }, } @@ -236,6 +242,7 @@ The --result flag accepts a file path or "-" for stdin.`, cmd.Flags().StringVar(&cfg.jiraURL, "jira-url", "", "Jira instance URL (default: $JIRA_BASE_URL)") cmd.Flags().StringVar(&cfg.jiraEmail, "jira-email", "", "Jira user email for Basic auth (default: $JIRA_USER_EMAIL)") cmd.Flags().BoolVar(&cfg.dryRun, "dry-run", false, "print what would be posted without making API calls") + cmd.Flags().BoolVar(&keepHistory, "keep-history", true, "append previous content as collapsed history blocks (set false to replace in-place)") cmd.Flags().StringVar(&cfg.fullsendDir, "fullsend-dir", "", "path to .fullsend config directory (sources defaults from its config.yaml when flags are omitted)") _ = cmd.MarkFlagRequired("project") _ = cmd.MarkFlagRequired("number") From a209b35e83bacf4b9847d77f26697523211ac3ca Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:18:17 +0000 Subject: [PATCH 4/6] fix: set KeepHistory: true on all sticky.Config test literals The KeepHistory bool field has a Go zero value of false, which inverts the pre-existing default (history was always kept). Update all 10 sticky.Config{} literals in issues_test.go and postreview_test.go that omitted KeepHistory to explicitly set KeepHistory: true, preventing silent wrong-default behavior if tests expand to exercise the update path. Addresses #6930 --- internal/cli/issues_test.go | 12 ++++++------ internal/cli/postreview_test.go | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/cli/issues_test.go b/internal/cli/issues_test.go index 752e5077e3..fa9b53ba91 100644 --- a/internal/cli/issues_test.go +++ b/internal/cli/issues_test.go @@ -148,7 +148,7 @@ func TestPostTrackerStickyComment_EmptyBody(t *testing.T) { fc := forge.NewFakeClient() tc := tracker.NewForgeClient(fc) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} _, err := postTrackerStickyComment(context.Background(), tc, "acme/widgets", 42, "", cfg, printer) assert.Error(t, err) @@ -159,7 +159,7 @@ func TestPostTrackerStickyComment_EmptyMarker(t *testing.T) { fc := forge.NewFakeClient() tc := tracker.NewForgeClient(fc) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} _, err := postTrackerStickyComment(context.Background(), tc, "acme/widgets", 42, "hello", cfg, printer) assert.Error(t, err) @@ -170,7 +170,7 @@ func TestPostTrackerStickyComment_DryRun_Create(t *testing.T) { fc := forge.NewFakeClient() tc := tracker.NewForgeClient(fc) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: "", DryRun: true} + cfg := sticky.Config{Marker: "", DryRun: true, KeepHistory: true} url, err := postTrackerStickyComment(context.Background(), tc, "acme/widgets", 42, "hello", cfg, printer) require.NoError(t, err) @@ -710,7 +710,7 @@ func TestPostJiraStickyComment_DryRun_Create(t *testing.T) { tc, _, err := tracker.NewFakeJiraClientWithFake("https://acme.atlassian.net") require.NoError(t, err) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: "", DryRun: true} + cfg := sticky.Config{Marker: "", DryRun: true, KeepHistory: true} url, err := postJiraStickyComment(context.Background(), tc, "PROJ", 42, "hello", cfg, printer) require.NoError(t, err) @@ -750,7 +750,7 @@ func TestPostJiraStickyComment_EmptyBody(t *testing.T) { tc, _, err := tracker.NewFakeJiraClientWithFake("https://acme.atlassian.net") require.NoError(t, err) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} _, err = postJiraStickyComment(context.Background(), tc, "PROJ", 42, " ", cfg, printer) require.Error(t, err) @@ -761,7 +761,7 @@ func TestPostJiraStickyComment_EmptyMarker(t *testing.T) { tc, _, err := tracker.NewFakeJiraClientWithFake("https://acme.atlassian.net") require.NoError(t, err) printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: " "} + cfg := sticky.Config{Marker: " ", KeepHistory: true} _, err = postJiraStickyComment(context.Background(), tc, "PROJ", 42, "body", cfg, printer) require.Error(t, err) diff --git a/internal/cli/postreview_test.go b/internal/cli/postreview_test.go index 3dadc2cbcc..fc569fa652 100644 --- a/internal/cli/postreview_test.go +++ b/internal/cli/postreview_test.go @@ -136,7 +136,7 @@ func TestPostStaleHeadNotice(t *testing.T) { fc.PullRequestHeadSHA = "new_sha_456" printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} err := postStaleHeadNotice(context.Background(), fc, "o", "r", 1, "old_sha_123", "new_sha_456", cfg, printer) require.Error(t, err, "should return an error indicating staleness") assert.Contains(t, err.Error(), "stale") @@ -164,7 +164,7 @@ func TestPostFailureNotice_WithBody(t *testing.T) { fc.AuthenticatedUser = "bot" printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} parsed := ReviewResult{Action: "failure", Body: "Custom failure message", Reason: "tool-failure"} err := postFailureNotice(context.Background(), fc, "o", "r", 1, parsed, cfg, printer) require.NoError(t, err) @@ -179,7 +179,7 @@ func TestPostFailureNotice_WithoutBody(t *testing.T) { fc.AuthenticatedUser = "bot" printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} parsed := ReviewResult{Action: "failure", Reason: "token-limit"} err := postFailureNotice(context.Background(), fc, "o", "r", 1, parsed, cfg, printer) require.NoError(t, err) @@ -195,7 +195,7 @@ func TestPostFailureNotice_EmptyReason(t *testing.T) { fc.AuthenticatedUser = "bot" printer := ui.New(io.Discard) - cfg := sticky.Config{Marker: ""} + cfg := sticky.Config{Marker: "", KeepHistory: true} parsed := ReviewResult{Action: "failure", Reason: ""} err := postFailureNotice(context.Background(), fc, "o", "r", 1, parsed, cfg, printer) require.NoError(t, err) From ce87d6cd7594fd252312fc289a08bbd0514df22b Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:51:19 +0000 Subject: [PATCH 5/6] fix: add resolveKeepHistory tests and config resolution to deprecated post-comment Add TestResolveKeepHistory_* tests mirroring the resolveTracker suite: flag overrides config, falls back to config reader, falls back to fullsend-dir config, nil-everything defaults to true, and config load error returns true with error. Wire --fullsend-dir and resolveKeepHistory() into the deprecated post-comment command so that keep_history set in config.yaml is respected, matching the pattern in issues post-comment and post-review. Addresses #6930 --- internal/cli/issues_test.go | 50 +++++++++++++++++++++++++++++++++++++ internal/cli/postcomment.go | 18 ++++++++++++- 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/internal/cli/issues_test.go b/internal/cli/issues_test.go index fa9b53ba91..066448f51d 100644 --- a/internal/cli/issues_test.go +++ b/internal/cli/issues_test.go @@ -858,6 +858,56 @@ func TestResolveTracker_FullsendDirWithoutTrackerSet_Errors(t *testing.T) { assert.Contains(t, err.Error(), "--tracker is required") } +// --- resolveKeepHistory tests --- +// +// Mirrors the resolveTracker test suite. resolveKeepHistory resolves +// the keep_history setting from: (1) explicit flag, (2) config +// reader, (3) fullsend-dir config.yaml, (4) default true. + +func TestResolveKeepHistory_FlagOverridesConfig(t *testing.T) { + reader, err := config.ParsePerRepoConfig([]byte("keep_history: true\n")) + require.NoError(t, err) + + flagVal := false + got, err := resolveKeepHistory(&flagVal, "", reader) + require.NoError(t, err) + assert.False(t, got, "explicit flag=false should override config=true") +} + +func TestResolveKeepHistory_FallsBackToConfigReader(t *testing.T) { + reader, err := config.ParsePerRepoConfig([]byte("keep_history: false\n")) + require.NoError(t, err) + + got, err := resolveKeepHistory(nil, "", reader) + require.NoError(t, err) + assert.False(t, got, "nil flag should fall back to config reader value") +} + +func TestResolveKeepHistory_FallsBackToFullsendDirConfig(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("keep_history: false\n"), 0o644)) + + got, err := resolveKeepHistory(nil, dir, nil) + require.NoError(t, err) + assert.False(t, got, "nil flag + nil reader should fall back to fullsend-dir config") +} + +func TestResolveKeepHistory_NilEverythingDefaultsTrue(t *testing.T) { + got, err := resolveKeepHistory(nil, "", nil) + require.NoError(t, err) + assert.True(t, got, "nil flag + no config + no fullsend-dir should default to true") +} + +func TestResolveKeepHistory_ConfigLoadErrorReturnsTrueWithError(t *testing.T) { + // Point at a directory with an invalid config.yaml to trigger a load error. + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(":\tinvalid yaml\n"), 0o644)) + + got, err := resolveKeepHistory(nil, dir, nil) + require.Error(t, err, "should propagate config load error") + assert.True(t, got, "should default to true on config load error") +} + // --- config-default --tracker integration tests --- func TestRunIssuesPostComment_TrackerFromConfig(t *testing.T) { diff --git a/internal/cli/postcomment.go b/internal/cli/postcomment.go index 6be985c281..5f4573e081 100644 --- a/internal/cli/postcomment.go +++ b/internal/cli/postcomment.go @@ -21,6 +21,7 @@ func newPostCommentCmd() *cobra.Command { token string dryRun bool keepHistory bool + fullsendDir string ) cmd := &cobra.Command{ @@ -68,11 +69,25 @@ The --result flag accepts a file path or "-" for stdin.`, printer.Header("Post Comment") + // Resolve keep_history: explicit --keep-history flag takes + // precedence, otherwise fall back to config.yaml via + // --fullsend-dir (matching the pattern in issues post-comment + // and post-review). + resolvedKeepHistory := keepHistory + if !cmd.Flags().Changed("keep-history") { + var khFlag *bool // nil = not explicitly set + resolved, resolveErr := resolveKeepHistory(khFlag, fullsendDir, nil) + if resolveErr != nil { + printer.StepWarn(fmt.Sprintf("Warning: %v; defaulting to keep_history=true", resolveErr)) + } + resolvedKeepHistory = resolved + } + client := gh.New(token) cfg := sticky.Config{ Marker: marker, DryRun: dryRun, - KeepHistory: keepHistory, + KeepHistory: resolvedKeepHistory, } _, err = sticky.Post(cmd.Context(), client, owner, repoName, number, body, cfg, printer) return err @@ -86,6 +101,7 @@ The --result flag accepts a file path or "-" for stdin.`, cmd.Flags().StringVar(&token, "token", "", "GitHub token (default: $GITHUB_TOKEN)") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print what would be posted without making API calls") cmd.Flags().BoolVar(&keepHistory, "keep-history", true, "append previous content as collapsed history blocks (set false to replace in-place)") + cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", "", "path to .fullsend config directory (sources defaults from its config.yaml when flags are omitted)") _ = cmd.MarkFlagRequired("repo") _ = cmd.MarkFlagRequired("number") _ = cmd.MarkFlagRequired("marker") From 90aabf68e7cdfbf97e5a59b85b06fddc5b415039 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 20:50:23 +0000 Subject: [PATCH 6/6] fix: default --fullsend-dir to $FULLSEND_DIR in post-review and post-comment Make post-review and post-comment auto-discover config.yaml by defaulting --fullsend-dir to the FULLSEND_DIR environment variable. The runner already injects FULLSEND_DIR into CI environments, so per-repo keep_history settings now take effect on review comments without callers needing to explicitly pass --fullsend-dir. Note: pre-commit hooks were not run. `pre-commit` could not complete (infrastructure failure), and the hooks were run directly instead (gofmt, go vet). Addresses #6930 --- docs/guides/dev/cli-internals.md | 2 +- internal/cli/postcomment.go | 2 +- internal/cli/postreview.go | 2 +- internal/cli/postreview_test.go | 16 ++++++++++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index 2b938df33d..71893e79e7 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -146,7 +146,7 @@ fullsend │ ├── --head-sha # Expected PR HEAD SHA (skips review if HEAD moved) │ ├── --dry-run # Print what would be posted without API calls │ ├── --keep-history # Append previous content as collapsed history (default true) -│ └── --fullsend-dir # .fullsend config directory (resolves keep_history default) +│ └── --fullsend-dir # .fullsend config directory (default: $FULLSEND_DIR; resolves keep_history default) ├── post-comment # Post issue/PR comments to GitHub (deprecated) ├── eval-measure # Score wild-run traces (eval measurements) │ ├── --telemetry # Path to run-telemetry.jsonl (or --output-dir) diff --git a/internal/cli/postcomment.go b/internal/cli/postcomment.go index 5f4573e081..5e33319de2 100644 --- a/internal/cli/postcomment.go +++ b/internal/cli/postcomment.go @@ -101,7 +101,7 @@ The --result flag accepts a file path or "-" for stdin.`, cmd.Flags().StringVar(&token, "token", "", "GitHub token (default: $GITHUB_TOKEN)") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print what would be posted without making API calls") cmd.Flags().BoolVar(&keepHistory, "keep-history", true, "append previous content as collapsed history blocks (set false to replace in-place)") - cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", "", "path to .fullsend config directory (sources defaults from its config.yaml when flags are omitted)") + cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", os.Getenv("FULLSEND_DIR"), "path to .fullsend config directory (default: $FULLSEND_DIR; sources defaults from its config.yaml when flags are omitted)") _ = cmd.MarkFlagRequired("repo") _ = cmd.MarkFlagRequired("number") _ = cmd.MarkFlagRequired("marker") diff --git a/internal/cli/postreview.go b/internal/cli/postreview.go index 0d03a4c829..2ce69f3440 100644 --- a/internal/cli/postreview.go +++ b/internal/cli/postreview.go @@ -173,7 +173,7 @@ GITLAB_TOKEN for GitLab and GH_TOKEN / GITHUB_TOKEN for GitHub.`, cmd.Flags().StringVar(&headSHA, "head-sha", "", "expected PR HEAD SHA (skips review if HEAD has moved)") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print what would be posted without making API calls") cmd.Flags().BoolVar(&keepHistory, "keep-history", true, "append previous content as collapsed history blocks (set false to replace in-place)") - cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", "", "path to .fullsend config directory (sources defaults from its config.yaml when flags are omitted)") + cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", os.Getenv("FULLSEND_DIR"), "path to .fullsend config directory (default: $FULLSEND_DIR; sources defaults from its config.yaml when flags are omitted)") cmd.Flags().StringVar(&forgeName, "forge", "", "forge backend: github (default) or gitlab") cmd.Flags().StringVar(&baseURL, "base-url", "", "forge instance URL (e.g. https://gitlab.example.com)") _ = cmd.MarkFlagRequired("repo") diff --git a/internal/cli/postreview_test.go b/internal/cli/postreview_test.go index fc569fa652..6cb0a28c1e 100644 --- a/internal/cli/postreview_test.go +++ b/internal/cli/postreview_test.go @@ -1807,3 +1807,19 @@ func TestBuildFallbackReviewBody(t *testing.T) { assert.Equal(t, "", body) }) } + +func TestNewPostReviewCmd_FullsendDirDefaultsToEnvVar(t *testing.T) { + t.Setenv("FULLSEND_DIR", "/path/to/.fullsend") + cmd := newPostReviewCmd() + f := cmd.Flags().Lookup("fullsend-dir") + require.NotNil(t, f) + assert.Equal(t, "/path/to/.fullsend", f.DefValue, "fullsend-dir should default to $FULLSEND_DIR") +} + +func TestNewPostReviewCmd_FullsendDirDefaultsEmptyWithoutEnvVar(t *testing.T) { + t.Setenv("FULLSEND_DIR", "") + cmd := newPostReviewCmd() + f := cmd.Flags().Lookup("fullsend-dir") + require.NotNil(t, f) + assert.Equal(t, "", f.DefValue, "fullsend-dir should default to empty when $FULLSEND_DIR is unset") +}