From 22f2ad9c9d39e5d42a96e07afd3d1b77e3cebfa7 Mon Sep 17 00:00:00 2001 From: insleker Date: Sat, 11 Apr 2026 18:08:45 +0800 Subject: [PATCH] fix: unregister gosmee webhooks on forced shutdown (#47) --- .../issue_event_source/gitea_gosmee.dart | 39 ++++- .../issue_event_source/github_gosmee.dart | 39 ++++- .../issue_event_source/gitlab_gosmee.dart | 39 ++++- .../github_gosmee_test.dart | 135 ++++++++++++++++ .../gitlab_gosmee_test.dart | 150 ++++++++++++++++++ test/issue_event_source/gosmee_test.dart | 132 +++++++++++++++ test/support/fake_gosmee.dart | 3 +- test/support/script_utils.dart | 20 ++- 8 files changed, 537 insertions(+), 20 deletions(-) diff --git a/lib/src/issue_tracker/issue_event_source/gitea_gosmee.dart b/lib/src/issue_tracker/issue_event_source/gitea_gosmee.dart index 9fd78d2..dc506d4 100644 --- a/lib/src/issue_tracker/issue_event_source/gitea_gosmee.dart +++ b/lib/src/issue_tracker/issue_event_source/gitea_gosmee.dart @@ -20,6 +20,7 @@ class GiteaGosmeeIssueEventSource implements IssueEventSource { }) : _logger = logger; static const Duration _webhookCoalescingWindow = Duration(milliseconds: 500); + static const Duration _processShutdownTimeout = Duration(seconds: 2); final IssueTrackerClient issueTrackerClient; final String gosmeeCommand; @@ -206,12 +207,7 @@ class GiteaGosmeeIssueEventSource implements IssueEventSource { final processes = _gosmeeProcesses.values.toList(growable: false); _gosmeeProcesses.clear(); - for (final process in processes) { - process.kill(); - } - for (final process in processes) { - await process.exitCode.catchError((_) => 0); - } + await _terminateProcesses(processes); final webhookIds = Map.from(_webhookIdsByProject); _webhookIdsByProject.clear(); @@ -451,6 +447,37 @@ class GiteaGosmeeIssueEventSource implements IssueEventSource { _logger.error(message); } + Future _terminateProcesses( + List processes, + ) async { + for (final process in processes) { + process.kill(); + } + for (final process in processes) { + await _awaitProcessExit(process); + } + } + + Future _awaitProcessExit(StartedExternalCommand process) async { + try { + await process.exitCode.timeout(_processShutdownTimeout); + return; + } on TimeoutException { + process.kill(ProcessSignal.sigkill); + } catch (_) { + return; + } + + try { + await process.exitCode.timeout(_processShutdownTimeout); + } on TimeoutException { + _logError( + 'gitea/gosmee process did not exit after sigkill ' + 'timeout_ms=${_processShutdownTimeout.inMilliseconds}', + ); + } catch (_) {} + } + String _summarizeText(String value) { final singleLine = value.trim().replaceAll(RegExp(r'\s+'), ' '); if (singleLine.length <= 240) { diff --git a/lib/src/issue_tracker/issue_event_source/github_gosmee.dart b/lib/src/issue_tracker/issue_event_source/github_gosmee.dart index eec4e19..a1d2fbb 100644 --- a/lib/src/issue_tracker/issue_event_source/github_gosmee.dart +++ b/lib/src/issue_tracker/issue_event_source/github_gosmee.dart @@ -20,6 +20,7 @@ class GitHubGosmeeIssueEventSource implements IssueEventSource { }) : _logger = logger; static const Duration _webhookCoalescingWindow = Duration(milliseconds: 500); + static const Duration _processShutdownTimeout = Duration(seconds: 2); final IssueTrackerClient issueTrackerClient; final String gosmeeCommand; @@ -206,12 +207,7 @@ class GitHubGosmeeIssueEventSource implements IssueEventSource { final processes = _gosmeeProcesses.values.toList(growable: false); _gosmeeProcesses.clear(); - for (final process in processes) { - process.kill(); - } - for (final process in processes) { - await process.exitCode.catchError((_) => 0); - } + await _terminateProcesses(processes); final webhookIds = Map.from(_webhookIdsByProject); _webhookIdsByProject.clear(); @@ -447,6 +443,37 @@ class GitHubGosmeeIssueEventSource implements IssueEventSource { void _logError(String message) => _logger.error(message); + Future _terminateProcesses( + List processes, + ) async { + for (final process in processes) { + process.kill(); + } + for (final process in processes) { + await _awaitProcessExit(process); + } + } + + Future _awaitProcessExit(StartedExternalCommand process) async { + try { + await process.exitCode.timeout(_processShutdownTimeout); + return; + } on TimeoutException { + process.kill(ProcessSignal.sigkill); + } catch (_) { + return; + } + + try { + await process.exitCode.timeout(_processShutdownTimeout); + } on TimeoutException { + _logError( + 'github/gosmee process did not exit after sigkill ' + 'timeout_ms=${_processShutdownTimeout.inMilliseconds}', + ); + } catch (_) {} + } + String _summarizeText(String value) { final singleLine = value.trim().replaceAll(RegExp(r'\s+'), ' '); if (singleLine.length <= 240) { diff --git a/lib/src/issue_tracker/issue_event_source/gitlab_gosmee.dart b/lib/src/issue_tracker/issue_event_source/gitlab_gosmee.dart index 662b8c1..219afd4 100644 --- a/lib/src/issue_tracker/issue_event_source/gitlab_gosmee.dart +++ b/lib/src/issue_tracker/issue_event_source/gitlab_gosmee.dart @@ -18,6 +18,7 @@ class GitLabGosmeeIssueEventSource implements IssueEventSource { }) : _logger = logger; static const Duration _webhookCoalescingWindow = Duration(milliseconds: 500); + static const Duration _processShutdownTimeout = Duration(seconds: 2); final IssueTrackerClient issueTrackerClient; final String gosmeeCommand; @@ -204,12 +205,7 @@ class GitLabGosmeeIssueEventSource implements IssueEventSource { final processes = _gosmeeProcesses.values.toList(growable: false); _gosmeeProcesses.clear(); - for (final process in processes) { - process.kill(); - } - for (final process in processes) { - await process.exitCode.catchError((_) => 0); - } + await _terminateProcesses(processes); final webhookIds = Map.from(_webhookIdsByProject); _webhookIdsByProject.clear(); @@ -485,6 +481,37 @@ class GitLabGosmeeIssueEventSource implements IssueEventSource { void _logError(String message) => _logger.error(message); + Future _terminateProcesses( + List processes, + ) async { + for (final process in processes) { + process.kill(); + } + for (final process in processes) { + await _awaitProcessExit(process); + } + } + + Future _awaitProcessExit(StartedExternalCommand process) async { + try { + await process.exitCode.timeout(_processShutdownTimeout); + return; + } on TimeoutException { + process.kill(ProcessSignal.sigkill); + } catch (_) { + return; + } + + try { + await process.exitCode.timeout(_processShutdownTimeout); + } on TimeoutException { + _logError( + 'gitlab/gosmee process did not exit after sigkill ' + 'timeout_ms=${_processShutdownTimeout.inMilliseconds}', + ); + } catch (_) {} + } + String _summarizeText(String value) { final singleLine = value.trim().replaceAll(RegExp(r'\s+'), ' '); if (singleLine.length <= 240) { diff --git a/test/issue_event_source/github_gosmee_test.dart b/test/issue_event_source/github_gosmee_test.dart index 0d2a846..6b5ae08 100644 --- a/test/issue_event_source/github_gosmee_test.dart +++ b/test/issue_event_source/github_gosmee_test.dart @@ -217,6 +217,141 @@ projects: isTrue, ); }); + + /// ```gherkin + /// Scenario: Delete webhook even when gosmee ignores SIGTERM during shutdown + /// Given a temporary project checkout that can be used as the local repo path + /// And a gosmee command that forwards one GitHub webhook event but ignores SIGTERM + /// And a gh script that can manage webhooks and post issue comments + /// And a responder that emits a planning reply + /// And an orchestrator-style config that uses gh/gosmee + /// When the long-running app starts and then closes + /// Then the app still deletes the GitHub webhook for the project + /// ``` + test( + 'Delete webhook even when gosmee ignores SIGTERM during shutdown', + () async { + // Given a temporary project checkout that can be used as the local repo path. + final sandbox = await Directory.systemTemp.createTemp( + 'cws-gh-gosmee-shutdown-', + ); + final repoDir = await Directory(p.join(sandbox.path, 'repo')).create(); + await initGitRepo(repoDir); + + // And a gosmee command that forwards one GitHub webhook event but ignores SIGTERM. + final gosmeeScript = File(p.join(sandbox.path, 'gosmee')); + final issue = { + 'number': 21, + 'title': 'Shutdown should still unregister webhook', + 'body': 'Please react before shutdown.', + 'state': 'open', + 'html_url': 'https://example.test/issues/21', + 'updated_at': '2026-04-08T07:00:00Z', + 'user': {'login': 'reporter'}, + 'labels': const >[], + }; + final comments = [ + { + 'id': 151, + 'body': '@helper verify webhook cleanup on shutdown', + 'html_url': 'https://example.test/issues/21#issuecomment-151', + 'created_at': '2026-04-08T07:00:00Z', + 'updated_at': '2026-04-08T07:00:00Z', + 'user': {'login': 'reporter'}, + }, + ]; + registerFakeGosmeeCommand( + command: gosmeeScript.path, + publicUrl: 'https://gosmee.example.test/github-shutdown', + payload: { + 'action': 'created', + 'issue': issue, + 'comment': comments.last, + 'repository': {'full_name': 'owner/sample'}, + }, + webhookHeader: 'X-GitHub-Event', + exitOnSigterm: false, + ); + + // And a gh script that can manage webhooks and post issue comments. + final ghScript = File(p.join(sandbox.path, 'gh')); + final ghLog = File(p.join(sandbox.path, 'gh-log.jsonl')); + final postedBody = File(p.join(sandbox.path, 'posted-body.md')); + await writeGitHubGosmeeGhScript( + ghScript: ghScript, + repo: 'owner/sample', + webhookId: 81, + issueNumber: 21, + comments: comments, + ghLog: ghLog, + postedBodyFile: postedBody, + viewerLogin: 'octocat', + ); + + // And a responder that emits a planning reply. + final responderScript = File(p.join(sandbox.path, 'responder.sh')); + await writeResponderScript(responderScript, { + 'decision': 'reply', + 'mode': 'planning', + 'markdown': + 'Plan:\n- ensure webhook cleanup when shutdown races forwarding', + 'summary': 'posted shutdown cleanup advice', + }); + + // And an orchestrator-style config that uses gh/gosmee. + final configFile = File( + p.join(sandbox.path, 'agent-orchestrator.yaml'), + ); + await configFile.writeAsString(''' +defaults: + issueAssistant: + enabled: true + mentionTriggers: ["@helper"] + responders: + - id: primary + command: ${responderScript.path} + timeout: 1m +projects: + sample: + issueTracker: + provider: github + eventSource: + pollInterval: 5m + type: gh/gosmee + repo: owner/sample + path: ${repoDir.path} +'''); + final app = await IssueAssistantApp.open( + config: await AppConfig.load(configFile.path), + databasePath: p.join(sandbox.path, 'state.sqlite3'), + ghCommand: ghScript.path, + gosmeeCommand: gosmeeScript.path, + responderRunner: fakeResponderRunner(), + commandExecutor: fakeExternalCommandExecutor(), + ); + + // When the long-running app starts and then closes. + final runFuture = app.run(); + for (var attempt = 0; attempt < 200; attempt += 1) { + if (await postedBody.exists()) { + break; + } + await Future.delayed(const Duration(milliseconds: 100)); + } + await app.close(); + await runFuture; + + // Then the app still deletes the GitHub webhook for the project. + final ghLogLines = await ghLog.readAsLines(); + expect( + ghLogLines.any( + (line) => + line.contains('api -X DELETE repos/owner/sample/hooks/81'), + ), + isTrue, + ); + }, + ); }); } diff --git a/test/issue_event_source/gitlab_gosmee_test.dart b/test/issue_event_source/gitlab_gosmee_test.dart index 10a2f7e..d654c1e 100644 --- a/test/issue_event_source/gitlab_gosmee_test.dart +++ b/test/issue_event_source/gitlab_gosmee_test.dart @@ -236,6 +236,156 @@ projects: isTrue, ); }); + + /// ```gherkin + /// Scenario: Delete webhook even when gosmee ignores SIGTERM during shutdown + /// Given a temporary project checkout that can be used as the local repo path + /// And a gosmee command that forwards one GitLab webhook event but ignores SIGTERM + /// And a glab script that can manage webhooks and post issue comments + /// And a responder that emits a planning reply + /// And an orchestrator-style config that uses glab/gosmee + /// When the long-running app starts and then closes + /// Then the app still deletes the GitLab webhook for the project + /// ``` + test( + 'Delete webhook even when gosmee ignores SIGTERM during shutdown', + () async { + // Given a temporary project checkout that can be used as the local repo path. + final sandbox = await Directory.systemTemp.createTemp( + 'cws-glab-gosmee-shutdown-', + ); + final repoDir = await Directory(p.join(sandbox.path, 'repo')).create(); + await initGitRepo(repoDir); + + // And a gosmee command that forwards one GitLab webhook event but ignores SIGTERM. + final gosmeeScript = File(p.join(sandbox.path, 'gosmee')); + final issue = { + 'iid': 21, + 'title': 'Shutdown should still unregister webhook', + 'description': 'Please react before shutdown.', + 'state': 'opened', + 'web_url': + 'https://gitlab.example.test/group/subgroup/sample/-/issues/21', + 'updated_at': '2026-04-08T07:00:00Z', + 'author': {'username': 'reporter'}, + 'labels': const [], + }; + final comments = >[ + { + 'id': 151, + 'body': '@helper verify webhook cleanup on shutdown', + 'created_at': '2026-04-08T07:00:00Z', + 'updated_at': '2026-04-08T07:00:00Z', + 'author': {'username': 'reporter'}, + 'noteable_url': + 'https://gitlab.example.test/group/subgroup/sample/-/issues/21', + }, + ]; + registerFakeGosmeeCommand( + command: gosmeeScript.path, + publicUrl: 'https://gosmee.example.test/gitlab-shutdown', + payload: { + 'object_kind': 'note', + 'project': {'path_with_namespace': 'group/subgroup/sample'}, + 'issue': issue, + 'object_attributes': { + 'action': 'create', + 'note': comments.last['body'], + 'noteable_type': 'Issue', + 'url': comments.last['noteable_url'], + }, + 'user': {'username': 'reporter'}, + }, + webhookHeader: 'X-Gitlab-Event', + exitOnSigterm: false, + ); + + // And a glab script that can manage webhooks and post issue comments. + final glabScript = File(p.join(sandbox.path, 'glab')); + final glabLog = File(p.join(sandbox.path, 'glab-log.jsonl')); + final postedBody = File(p.join(sandbox.path, 'posted-body.md')); + await writeFakeGlabScript( + glabScript: glabScript, + issueListResponsesByRepo: { + 'group/subgroup/sample': const >[], + }, + commentResponsesByRepo: { + 'group/subgroup/sample': {21: comments}, + }, + postCommentIssueNumbersByRepo: { + 'group/subgroup/sample': {21}, + }, + glabLog: glabLog, + postedBodyFile: postedBody, + viewerLogin: 'glab-user', + webhookIdsByRepo: {'group/subgroup/sample': 81}, + ); + + // And a responder that emits a planning reply. + final responderScript = File(p.join(sandbox.path, 'responder.sh')); + await writeResponderScript(responderScript, { + 'decision': 'reply', + 'mode': 'planning', + 'markdown': + 'Plan:\n- ensure webhook cleanup when shutdown races forwarding', + 'summary': 'posted shutdown cleanup advice', + }); + + // And an orchestrator-style config that uses glab/gosmee. + final configFile = File( + p.join(sandbox.path, 'agent-orchestrator.yaml'), + ); + await configFile.writeAsString(''' +defaults: + issueAssistant: + enabled: true + mentionTriggers: ["@helper"] + responders: + - id: primary + command: ${responderScript.path} + timeout: 1m +projects: + sample: + issueTracker: + provider: gitlab + eventSource: + pollInterval: 5m + type: glab/gosmee + repo: group/subgroup/sample + path: ${repoDir.path} +'''); + final app = await IssueAssistantApp.open( + config: await AppConfig.load(configFile.path), + databasePath: p.join(sandbox.path, 'state.sqlite3'), + glabCommand: glabScript.path, + gosmeeCommand: gosmeeScript.path, + responderRunner: fakeResponderRunner(), + commandExecutor: fakeExternalCommandExecutor(), + ); + + // When the long-running app starts and then closes. + final runFuture = app.run(); + for (var attempt = 0; attempt < 200; attempt += 1) { + if (await postedBody.exists()) { + break; + } + await Future.delayed(const Duration(milliseconds: 100)); + } + await app.close(); + await runFuture; + + // Then the app still deletes the GitLab webhook for the project. + final glabLogLines = await glabLog.readAsLines(); + expect( + glabLogLines.any( + (line) => line.contains( + 'api -X DELETE projects/group%2Fsubgroup%2Fsample/hooks/81', + ), + ), + isTrue, + ); + }, + ); }); } diff --git a/test/issue_event_source/gosmee_test.dart b/test/issue_event_source/gosmee_test.dart index 95d5979..48b45e8 100644 --- a/test/issue_event_source/gosmee_test.dart +++ b/test/issue_event_source/gosmee_test.dart @@ -365,6 +365,138 @@ CWS_GITEA_TOKEN=test-token isTrue, ); }); + + /// ```gherkin + /// Scenario: Delete webhook even when gosmee ignores SIGTERM during shutdown + /// Given a temporary project checkout that can be used as the local repo path + /// And a gosmee command that forwards one Gitea webhook event but ignores SIGTERM + /// And a Gitea API fixture that can manage webhooks and post issue comments + /// And a responder that emits a planning reply + /// And an orchestrator-style config that uses tea/gosmee + /// When the long-running app starts and then closes + /// Then the app still deletes the Gitea webhook for the project + /// ``` + test('Delete webhook even when gosmee ignores SIGTERM during shutdown', () async { + // Given a temporary project checkout that can be used as the local repo path. + final sandbox = await Directory.systemTemp.createTemp( + 'cws-gosmee-shutdown-', + ); + final repoDir = await Directory(p.join(sandbox.path, 'repo')).create(); + await initGitRepo(repoDir); + + // And a gosmee command that forwards one Gitea webhook event but ignores SIGTERM. + final gosmeeScript = File(p.join(sandbox.path, 'gosmee')); + final issue = { + 'index': 21, + 'title': 'Shutdown should still unregister webhook', + 'body': 'Please react before shutdown.', + 'state': 'open', + 'url': 'https://gitea.example.test/owner/sample/issues/21', + 'updated_at': '2026-04-08T07:00:00Z', + 'poster': {'login': 'reporter'}, + 'labels': const >[], + }; + final comments = [ + { + 'id': 151, + 'body': '@helper verify webhook cleanup on shutdown', + 'url': + 'https://gitea.example.test/owner/sample/issues/21#issuecomment-151', + 'created_at': '2026-04-08T07:00:00Z', + 'updated_at': '2026-04-08T07:00:00Z', + 'poster': {'login': 'reporter'}, + }, + ]; + registerFakeGosmeeCommand( + command: gosmeeScript.path, + publicUrl: 'https://gosmee.example.test/shutdown', + payload: { + 'action': 'created', + 'issue': issue, + 'comment': comments.last, + 'repository': {'full_name': 'owner/sample'}, + }, + exitOnSigterm: false, + ); + + // And a Gitea API fixture that can manage webhooks and post issue comments. + final giteaServer = await FakeGiteaApiServer.start( + issueListResponsesByRepo: { + 'owner/sample': const >[], + }, + commentResponsesByRepo: { + 'owner/sample': {21: comments}, + }, + postCommentResponsesByRepo: { + 'owner/sample': { + 21: { + 'id': 803, + 'body': 'posted', + 'html_url': 'https://gitea.example.test/comment/803', + }, + }, + }, + viewerLogin: 'tea-octocat', + ); + addTearDown(giteaServer.close); + + // And a responder that emits a planning reply. + final responderScript = File(p.join(sandbox.path, 'responder.sh')); + await writeResponderScript(responderScript, { + 'decision': 'reply', + 'mode': 'planning', + 'markdown': + 'Plan:\n- ensure webhook cleanup when shutdown races forwarding', + 'summary': 'posted shutdown cleanup advice', + }); + + // And an orchestrator-style config that uses tea/gosmee. + final configFile = File(p.join(sandbox.path, 'agent-orchestrator.yaml')); + await configFile.writeAsString(''' +defaults: + issueAssistant: + enabled: true + mentionTriggers: ["@helper"] + responders: + - id: primary + command: ${responderScript.path} + timeout: 1m +projects: + sample: + issueTracker: + provider: gitea + eventSource: + pollInterval: 5m + type: tea/gosmee + repo: owner/sample + path: ${repoDir.path} +'''); + await File(p.join(sandbox.path, '.env')).writeAsString(''' +CWS_GITEA_HOST=${giteaServer.baseUrl} +CWS_GITEA_TOKEN=test-token +'''); + final app = await IssueAssistantApp.open( + config: await AppConfig.load(configFile.path), + databasePath: p.join(sandbox.path, 'state.sqlite3'), + gosmeeCommand: gosmeeScript.path, + responderRunner: fakeResponderRunner(), + commandExecutor: fakeExternalCommandExecutor(), + ); + + // When the long-running app starts and then closes. + final runFuture = app.run(); + for (var attempt = 0; attempt < 200; attempt += 1) { + if (giteaServer.postedBodies.isNotEmpty) { + break; + } + await Future.delayed(const Duration(milliseconds: 100)); + } + await app.close(); + await runFuture; + + // Then the app still deletes the Gitea webhook for the project. + expect(giteaServer.deletedWebhookIds, contains(81)); + }); }); } diff --git a/test/support/fake_gosmee.dart b/test/support/fake_gosmee.dart index 172a316..e0ba83d 100644 --- a/test/support/fake_gosmee.dart +++ b/test/support/fake_gosmee.dart @@ -24,6 +24,7 @@ void registerFakeGosmeeCommand({ Map? payload, File? gosmeeLog, String webhookHeader = 'X-Gitea-Event', + bool exitOnSigterm = true, }) { fakeCommandHandlers[command] = (arguments, {timeout}) async { if (gosmeeLog != null) { @@ -44,7 +45,7 @@ void registerFakeGosmeeCommand({ if (gosmeeLog != null) { await _appendLine(gosmeeLog, '${arguments.join(' ')}\n'); } - final process = FakeStartedExternalCommand(); + final process = FakeStartedExternalCommand(exitOnSigterm: exitOnSigterm); if (arguments.length >= 3 && arguments[0] == 'client' && arguments[1] == publicUrl) { diff --git a/test/support/script_utils.dart b/test/support/script_utils.dart index c2e7223..593c329 100644 --- a/test/support/script_utils.dart +++ b/test/support/script_utils.dart @@ -61,6 +61,15 @@ ExternalCommandExecutor fakeExternalCommandExecutor() { } class FakeStartedExternalCommand implements StartedExternalCommand { + FakeStartedExternalCommand({ + this.exitOnSigterm = true, + this.exitOnSigkill = true, + this.sigkillExitCode = 137, + }); + + final bool exitOnSigterm; + final bool exitOnSigkill; + final int sigkillExitCode; final StreamController> _stdoutController = StreamController(); final StreamController> _stderrController = StreamController(); final Completer _exitCodeCompleter = Completer(); @@ -96,7 +105,16 @@ class FakeStartedExternalCommand implements StartedExternalCommand { @override bool kill([ProcessSignal signal = ProcessSignal.sigterm]) { - complete(); + if (signal == ProcessSignal.sigkill) { + if (exitOnSigkill) { + complete(sigkillExitCode); + } + return true; + } + if (signal == ProcessSignal.sigterm && exitOnSigterm) { + complete(); + return true; + } return true; } }