Skip to content

Commit 0cfc0d2

Browse files
Feature/add grace period minutes (#94)
* add grace-period-minutes Signed-off-by: SachinduNethmin <108050026+Sachindu-Nethmin@users.noreply.github.com> * fix(workflow): fall back to github.token when GH_PAT lacks issues scope The built-in github.token already has issues:write via the workflow permissions block, so prefer it when GH_PAT is absent or misconfigured. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: SachinduNethmin <108050026+Sachindu-Nethmin@users.noreply.github.com> * fix(workflow): use built-in github.token instead of GH_PAT GH_PAT is set but missing issues:write scope, causing 403 errors. The workflow already declares permissions: issues: write, so the built-in github.token has everything needed to close and label issues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: SachinduNethmin <108050026+Sachindu-Nethmin@users.noreply.github.com> * test(auto-close): add grace-period-minutes tests and docs Tests added: - TestGracePeriodFromConfig: validates minutes override > hours config > 72h default precedence in the grace period computation logic - TestGracePeriodMinutesExpiry: validates expiry checks across several minutes-based grace periods and label ages - TestGracePeriodMinutesCLIMapping: validates that CLI flag 0/negative maps to 1, positive values are stored as-is, and absent flag leaves override at 0 - TestRepoFlagParsing: validates owner/repo split for valid and invalid inputs Docs added: - README: new simili auto-close CLI section covering flags, grace period precedence table, simili.yaml config, human activity signals, and workflow_dispatch usage example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: SachinduNethmin <108050026+Sachindu-Nethmin@users.noreply.github.com> * fix(workflow): prevent shell injection in grace_period_minutes input inputs.grace_period_minutes is a free-text string that was previously interpolated directly into the shell script body, allowing a crafted value (e.g. "1 $(malicious)") to execute arbitrary commands. Fix: - Assign the input to GRACE_PERIOD_INPUT env var so it never reaches the script body as a template expression. - Validate with ^[0-9]+$ before use; emit a ::warning:: and skip the flag if the value is non-numeric. - Build the argument as a bash array (GRACE_ARGS) so the flag name and value are always passed as two separate quoted tokens, eliminating word-splitting and command-substitution risks. Signed-off-by: SachinduNethmin <108050026+Sachindu-Nethmin@users.noreply.github.com> --------- Signed-off-by: SachinduNethmin <108050026+Sachindu-Nethmin@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 62b867d commit 0cfc0d2

4 files changed

Lines changed: 317 additions & 2 deletions

File tree

.github/workflows/auto-close.yml

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ on:
2424
options:
2525
- 'false'
2626
- 'true'
27+
grace_period_minutes:
28+
description: 'Grace period in minutes before a potential-duplicate issue is labelled duplicate and closed (overrides config). Leave empty to use the configured default.'
29+
required: false
30+
default: ''
31+
type: string
2732

2833
permissions:
2934
contents: read
@@ -47,15 +52,26 @@ jobs:
4752

4853
- name: Run auto-close
4954
env:
50-
GITHUB_TOKEN: ${{ secrets.GH_PAT }}
55+
GITHUB_TOKEN: ${{ github.token }}
56+
GRACE_PERIOD_INPUT: ${{ inputs.grace_period_minutes }}
5157
run: |
5258
DRY_RUN_FLAG=""
5359
if [ "${{ inputs.dry_run }}" = "true" ]; then
5460
DRY_RUN_FLAG="--dry-run"
5561
fi
5662
63+
GRACE_ARGS=()
64+
if [[ -n "$GRACE_PERIOD_INPUT" ]]; then
65+
if [[ "$GRACE_PERIOD_INPUT" =~ ^[0-9]+$ ]]; then
66+
GRACE_ARGS=("--grace-period-minutes" "$GRACE_PERIOD_INPUT")
67+
else
68+
echo "::warning::grace_period_minutes must be a positive integer; ignoring value '$GRACE_PERIOD_INPUT'"
69+
fi
70+
fi
71+
5772
./simili-cli auto-close \
5873
--repo "${{ github.repository }}" \
5974
--config .github/simili.yaml \
6075
--verbose \
61-
$DRY_RUN_FLAG
76+
$DRY_RUN_FLAG \
77+
"${GRACE_ARGS[@]}"

README.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,50 @@ Notes:
220220
- `llm.api_key` can be omitted if `GEMINI_API_KEY` is set.
221221
- You can override the model at runtime with `LLM_MODEL`.
222222

223+
### `simili auto-close`
224+
225+
Scan all open issues labelled `potential-duplicate` and close those whose grace period has expired with no human activity. Closed issues are relabelled from `potential-duplicate` → `duplicate`.
226+
227+
```bash
228+
simili auto-close --repo owner/repo --grace-period-minutes 60
229+
```
230+
231+
**Flags:**
232+
- `--repo` (required): Target repository (`owner/name`); falls back to `GITHUB_REPOSITORY` env var
233+
- `--grace-period-minutes`: Override the grace period in minutes for this run (see precedence below)
234+
- `--dry-run`: Print what would be closed without making any changes
235+
- `--config`: Path to `simili.yaml` (auto-discovered if omitted)
236+
237+
**Grace period precedence** (highest → lowest):
238+
239+
| Source | How to set |
240+
|--------|-----------|
241+
| `--grace-period-minutes` CLI flag | Pass at runtime — overrides everything |
242+
| `auto_close.grace_period_hours` in `simili.yaml` | Persistent per-repo config |
243+
| Built-in default | 72 hours (3 days) |
244+
245+
**`simili.yaml` configuration:**
246+
247+
```yaml
248+
auto_close:
249+
grace_period_hours: 48 # default: 72
250+
dry_run: false
251+
```
252+
253+
**Human activity signals** — any of these prevent auto-close:
254+
1. A negative reaction (👎 or 😕) on the bot's triage comment by a non-bot user.
255+
2. The issue was reopened by a human after the `potential-duplicate` label was applied.
256+
3. A non-bot comment posted after the label was applied.
257+
258+
**GitHub Actions usage** — the `auto-close.yml` workflow runs daily at 10:00 UTC and can be triggered manually via `workflow_dispatch` with an optional `grace_period_minutes` input:
259+
260+
```yaml
261+
# Trigger from GitHub UI or gh CLI:
262+
gh workflow run auto-close.yml -f grace_period_minutes=60 -f dry_run=false
263+
```
264+
265+
Leaving `grace_period_minutes` empty uses the value from `simili.yaml` (or the 72 h default).
266+
223267
## Development
224268

225269
```bash
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
package commands
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/similigh/simili-bot/internal/core/config"
8+
)
9+
10+
// TestGracePeriodMinutesCLIMapping validates the flag-to-config mapping logic:
11+
// - An explicit 0 is stored as 1 (smallest positive value, triggers instant expiry in tests).
12+
// - Any positive value is stored as-is.
13+
// - When the flag was not set, GracePeriodMinutesOverride remains 0 (no override).
14+
func TestGracePeriodMinutesCLIMapping(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
flagChanged bool
18+
flagValue int
19+
wantOverride int
20+
}{
21+
{
22+
name: "flag not provided - no override",
23+
flagChanged: false,
24+
flagValue: 0,
25+
wantOverride: 0,
26+
},
27+
{
28+
name: "flag set to 0 - stored as 1 (instant expire)",
29+
flagChanged: true,
30+
flagValue: 0,
31+
wantOverride: 1,
32+
},
33+
{
34+
name: "flag set to negative - stored as 1 (instant expire)",
35+
flagChanged: true,
36+
flagValue: -5,
37+
wantOverride: 1,
38+
},
39+
{
40+
name: "flag set to 1 - stored as 1",
41+
flagChanged: true,
42+
flagValue: 1,
43+
wantOverride: 1,
44+
},
45+
{
46+
name: "flag set to 30 - stored as 30",
47+
flagChanged: true,
48+
flagValue: 30,
49+
wantOverride: 30,
50+
},
51+
{
52+
name: "flag set to 1440 (1 day in minutes) - stored as 1440",
53+
flagChanged: true,
54+
flagValue: 1440,
55+
wantOverride: 1440,
56+
},
57+
}
58+
59+
for _, tt := range tests {
60+
t.Run(tt.name, func(t *testing.T) {
61+
cfg := &config.Config{}
62+
63+
// Mirrors the mapping logic in runAutoClose.
64+
if tt.flagChanged {
65+
if tt.flagValue <= 0 {
66+
cfg.AutoClose.GracePeriodMinutesOverride = 1
67+
} else {
68+
cfg.AutoClose.GracePeriodMinutesOverride = tt.flagValue
69+
}
70+
}
71+
72+
if cfg.AutoClose.GracePeriodMinutesOverride != tt.wantOverride {
73+
t.Errorf("GracePeriodMinutesOverride = %d, want %d",
74+
cfg.AutoClose.GracePeriodMinutesOverride, tt.wantOverride)
75+
}
76+
})
77+
}
78+
}
79+
80+
// TestRepoFlagParsing validates owner/repo splitting used in runAutoClose.
81+
func TestRepoFlagParsing(t *testing.T) {
82+
tests := []struct {
83+
name string
84+
input string
85+
wantOrg string
86+
wantRepo string
87+
wantValid bool
88+
}{
89+
{
90+
name: "valid org/repo",
91+
input: "acme-corp/my-service",
92+
wantOrg: "acme-corp",
93+
wantRepo: "my-service",
94+
wantValid: true,
95+
},
96+
{
97+
name: "valid single-word org and repo",
98+
input: "owner/repo",
99+
wantOrg: "owner",
100+
wantRepo: "repo",
101+
wantValid: true,
102+
},
103+
{
104+
name: "missing slash - invalid",
105+
input: "ownerrepo",
106+
wantValid: false,
107+
},
108+
{
109+
name: "empty org - invalid",
110+
input: "/repo",
111+
wantValid: false,
112+
},
113+
{
114+
name: "empty repo - invalid",
115+
input: "owner/",
116+
wantValid: false,
117+
},
118+
{
119+
name: "empty string - invalid",
120+
input: "",
121+
wantValid: false,
122+
},
123+
}
124+
125+
for _, tt := range tests {
126+
t.Run(tt.name, func(t *testing.T) {
127+
org, repo, ok := strings.Cut(tt.input, "/")
128+
valid := ok && org != "" && repo != ""
129+
130+
if valid != tt.wantValid {
131+
t.Errorf("repo %q: valid=%v, want %v", tt.input, valid, tt.wantValid)
132+
return
133+
}
134+
if tt.wantValid {
135+
if org != tt.wantOrg {
136+
t.Errorf("org = %q, want %q", org, tt.wantOrg)
137+
}
138+
if repo != tt.wantRepo {
139+
t.Errorf("repo = %q, want %q", repo, tt.wantRepo)
140+
}
141+
}
142+
})
143+
}
144+
}

internal/steps/auto_closer_test.go

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,117 @@ func TestReopenedByHuman(t *testing.T) {
189189
}
190190
}
191191

192+
func TestGracePeriodFromConfig(t *testing.T) {
193+
tests := []struct {
194+
name string
195+
minutesOverride int
196+
hoursConfig int
197+
wantDuration time.Duration
198+
}{
199+
{
200+
name: "minutes override takes precedence over hours config",
201+
minutesOverride: 5,
202+
hoursConfig: 24,
203+
wantDuration: 5 * time.Minute,
204+
},
205+
{
206+
name: "minutes override of 1 takes precedence over default",
207+
minutesOverride: 1,
208+
hoursConfig: 0,
209+
wantDuration: 1 * time.Minute,
210+
},
211+
{
212+
name: "hours config used when no minutes override",
213+
minutesOverride: 0,
214+
hoursConfig: 48,
215+
wantDuration: 48 * time.Hour,
216+
},
217+
{
218+
name: "72-hour default applied when both are zero",
219+
minutesOverride: 0,
220+
hoursConfig: 0,
221+
wantDuration: 72 * time.Hour,
222+
},
223+
{
224+
name: "hours config of 1 is respected (no default override)",
225+
minutesOverride: 0,
226+
hoursConfig: 1,
227+
wantDuration: 1 * time.Hour,
228+
},
229+
}
230+
231+
for _, tt := range tests {
232+
t.Run(tt.name, func(t *testing.T) {
233+
var got time.Duration
234+
if tt.minutesOverride > 0 {
235+
got = time.Duration(tt.minutesOverride) * time.Minute
236+
} else {
237+
h := tt.hoursConfig
238+
if h <= 0 {
239+
h = 72
240+
}
241+
got = time.Duration(h) * time.Hour
242+
}
243+
if got != tt.wantDuration {
244+
t.Errorf("grace period = %v, want %v", got, tt.wantDuration)
245+
}
246+
})
247+
}
248+
}
249+
250+
func TestGracePeriodMinutesExpiry(t *testing.T) {
251+
tests := []struct {
252+
name string
253+
minutesOverride int
254+
labeledAgo time.Duration
255+
wantExpired bool
256+
}{
257+
{
258+
name: "1-minute override: labeled 2 minutes ago - expired",
259+
minutesOverride: 1,
260+
labeledAgo: 2 * time.Minute,
261+
wantExpired: true,
262+
},
263+
{
264+
name: "1-minute override: labeled 30 seconds ago - not expired",
265+
minutesOverride: 1,
266+
labeledAgo: 30 * time.Second,
267+
wantExpired: false,
268+
},
269+
{
270+
name: "5-minute override: labeled 4 minutes ago - not expired",
271+
minutesOverride: 5,
272+
labeledAgo: 4 * time.Minute,
273+
wantExpired: false,
274+
},
275+
{
276+
name: "5-minute override: labeled 6 minutes ago - expired",
277+
minutesOverride: 5,
278+
labeledAgo: 6 * time.Minute,
279+
wantExpired: true,
280+
},
281+
{
282+
name: "60-minute override: labeled 59 minutes ago - not expired",
283+
minutesOverride: 60,
284+
labeledAgo: 59 * time.Minute,
285+
wantExpired: false,
286+
},
287+
}
288+
289+
for _, tt := range tests {
290+
t.Run(tt.name, func(t *testing.T) {
291+
gracePeriod := time.Duration(tt.minutesOverride) * time.Minute
292+
labeledAt := time.Now().Add(-tt.labeledAgo)
293+
elapsed := time.Since(labeledAt)
294+
got := elapsed >= gracePeriod
295+
if got != tt.wantExpired {
296+
t.Errorf("expiry check: grace=%v, elapsed≈%v => expired=%v, want %v",
297+
gracePeriod, tt.labeledAgo, got, tt.wantExpired)
298+
}
299+
})
300+
}
301+
}
302+
192303
func TestAutoCloseResultCounts(t *testing.T) {
193304
tests := []struct {
194305
name string

0 commit comments

Comments
 (0)