feat(#6214): add keep_history config option for sticky comments - #6930
Conversation
Add a per-repo config.yaml option (keep_history) that controls whether sticky comment updates append previous content as collapsed "Previous run" <details> 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 <details>/<summary> 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
|
🤖 Review · Commit: |
Site previewPreview: https://0ae7593a-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 8:32 PM UTC · Completed 8:54 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.87 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsAnchored to prior score of 2 (moderate): Tier 1 signals unchanged (same file count range, no protected/security paths, no dependency changes, bot author, good test ratio), Tier 2 churn and contention in config/CLI packages consistent with prior assessment, well-scoped issue with opt-in config option preserving backward compatibility. Previous runRisk Assessment: moderate (2/5) DetailsModerate risk driven primarily by high churn and multi-author contention in the config and CLI packages (Tier 2), offset by bot authorship, no protected/security paths, good test ratio, and a well-scoped issue with the new behavior gated behind an opt-in config option that preserves backward compatibility. Previous run (2)Risk Assessment: moderate (2/5) DetailsModerate risk driven primarily by high churn and multi-author contention in the config and CLI packages (Tier 2), offset by bot authorship, no protected/security paths, good test ratio, and a well-scoped issue with the new behavior gated behind an opt-in config option that preserves backward compatibility. Previous run (3)Risk Assessment: moderate (2/5) DetailsModerate risk driven primarily by high churn and multi-author contention in the config and CLI packages (Tier 2). The change itself is well-scoped: it adds a new pointer-bool config option following established patterns, plumbs it through CLI commands, and preserves backward compatibility by defaulting to current behavior. Previous run (4)Risk Assessment: moderate (2/5) DetailsModerate risk driven primarily by high churn and multi-author contention in the config and CLI packages (Tier 2). The change itself is well-scoped: it adds a new pointer-bool config option following established patterns, plumbs it through CLI commands, and preserves backward compatibility by defaulting to current behavior. Previous run (5)Risk Assessment: moderate (2/5) DetailsModerate risk driven primarily by high churn and multi-author contention in the config and CLI packages (Tier 2). The change itself is well-scoped: it adds a new pointer-bool config option following established patterns, plumbs it through CLI commands, and preserves backward compatibility by defaulting to current behavior. |
ReviewFindingsLow
Next steps:
Previous runReviewFindingsLow
Previous run (2)ReviewFindingsLow
Next steps:
Previous run (3)ReviewFindingsLow
Next steps:
Previous run (4)ReviewFindingsLow
Next steps:
Previous run (5)ReviewFindingsMedium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 8:57 PM UTC · Completed 9:09 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.02 |
- 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
🔧 Fix agent — iteration 1 (bot-triggered)Addressed 6 of 7 review findings. Added config.yaml resolution to post-review via --fullsend-dir (scope-gap). Changed resolveKeepHistory to propagate config load errors (error-handling-gap). Fixed field ordering in perRepoConfig (field-ordering). Updated three stale documentation entries (cli-internals.md and issues-commands.md). Disagreed on adding config resolution to the deprecated post-comment command (scope-minor). Fixed (6):
Disagreed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 9:11 PM UTC · Completed 9:32 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.62 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 9:33 PM UTC · Completed 9:43 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.07 |
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
🔧 Fix agent — iteration 2 (bot-triggered)Addressed 3 of 4 review findings. Registered --keep-history flag on issues post-comment for CLI parity (scope-mismatch). Removed confusing circular reference to deprecated post-comment in docs (docs-accuracy). Updated CLI command tree with new flags. Disagreed on adding config.yaml resolution to the deprecated post-comment command (scope-minor) — same rationale as iteration 1. Fixed (3):
Disagreed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 9:45 PM UTC · Completed 10:08 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.43 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 10:09 PM UTC · Completed 10:20 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.04 |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
ready-for-merge
|
/fs-fix address the medium scope-gap problem, so that post-review also respects config.yaml |
|
🤖 Finished Fix · ✅ Success · Started 8:40 PM UTC · Completed 8:53 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.94 |
…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
🔧 Fix agent — iteration 5 (human-triggered)Closed the medium scope-gap by defaulting --fullsend-dir to $FULLSEND_DIR in post-review and post-comment. Previous iterations added the flag and wiring but left the default empty, so callers that did not explicitly pass --fullsend-dir (e.g., agent post-scripts) silently ignored config.yaml keep_history settings. The new approach uses the FULLSEND_DIR env var that the runner already injects, making config.yaml auto-discovery work without changing protected post-scripts. Fixed (3):
Tests: passed
Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 8:54 PM UTC · Completed 9:15 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.87 |
fullsend-ai-review
Bot
removed
the
ready-for-merge
|
🤖 Finished Fix · ❌ Failure (running pre-script: exit status 1) · Started 9:17 PM UTC · Completed 9:17 PM UTC Commit: Effort: high |
|
🤖 Finished Retro · ✅ Success · Started 5:51 PM UTC · Completed 6:04 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.49 |
Retro: PR #6930 —
|
2 participants
Summary
Add a
keep_historyper-repo config.yaml option that controls whether sticky comment updates append previous content as collapsed "Previous run"<details>blocks. When disabled (keep_history: false), updates replace the comment body in-place with no history — reducing noise when reading via the GitHub API and avoiding raw HTML rendering issues in Jira.Related Issue
Closes #6214
Changes
internal/config/): AddKeepHistory *boolfield toperRepoConfigusing the pointer-bool pattern (same askill_switch). AddConfigKeepHistory()accessor andSetKeepHistory()setter toPerRepoConfigReader/PerRepoConfigWriterinterfaces. Code default istrue(preserving existing behavior).internal/sticky/): AddKeepHistory booltosticky.Config.BuildUpdatedBodyshort-circuits whenKeepHistoryisfalse, discarding old content while preserving footer content.internal/cli/): Add--keep-historyflag (defaulttrue) topost-review,post-comment, andissues post-comment. Theissues post-commentcommand also resolves the setting from config.yaml via--fullsend-dirwhen the flag is not explicitly set.docs/guides/infrastructure/layered-config-reference.md): Document the new field in the per-field merge rules table, scalar override section, and code defaults reference.Testing
BuildUpdatedBodywithKeepHistory: false(no history, footer preserved, multiple runs)keep_historygo vet ./...passesKeepHistory: trueexplicitlyChecklist
!for breaking changes)Closes #6214
Post-script verification
agent/6214-optional-sticky-history)9f10cac80818e71e57e92eabd7c71470c1e1f55d..HEAD)