Compare commits

...
Author SHA1 Message Date
Thomas SharedInboxandClaude Sonnet 4.6 d0973fafbf fix(agent_loop): prevent infinite catch-up merge retry and wrong issue closure
Two bugs fixed:

1. Catch-up scan (section 2b) called _merge_pr and immediately returned,
   claiming success even when fgj exits 0 but the merge silently failed
   (e.g. branch-protection rules not satisfied). PR #163 was retried 30+
   times in a row because the PR stayed open after each attempt.
   Fix: verify the PR is no longer open after the merge call; if it is still
   open, set the issue to State/Question instead of looping forever.

2. ci-fix agents wrote "Closes #198" in commit messages, causing Forgejo to
   auto-close issue #198 ("Unable to load asset: assets/changelog.txt") even
   though the commit only fixed the unrelated Play Store upload.
   Fix: both ci-fix prompts now explicitly forbid issue-number references in
   commit messages and close operations. Also save ci_run_id_at_start in
   the ci-fix state (was only done for issue agents) so future guard logic
   can compare run IDs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 10:48:00 +02:00
Thomas SharedInboxandClaude Sonnet 4.6 7310568157 fix: merge orphaned issue PRs whose CI passed but state was cleared (#200)
Add catch-up scan in agent_loop that finds all open issue-N-fix PRs and
merges those with passed CI, using event-filtered API query (limit=50)
to cover weeks of history instead of the previous ~1.5 h window.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 08:49:33 +02:00
Thomas SharedInboxandClaude Sonnet 4.6 a569177637 fix: treat MissingPluginException from secure storage as permanent sync error (#200)
When flutter_secure_storage's platform channel is unavailable (e.g. on
certain Android devices), getPassword() throws MissingPluginException.
Previously this was not recognised as a permanent error, so the IMAP and
JMAP sync loops retried indefinitely with exponential back-off, filling
the sync log with repeated failures (as shown in the screenshot).

Treat MissingPluginException as a permanent error in both _AccountSync
and _JmapAccountSync so the loop stops immediately instead of retrying.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 08:46:29 +02:00
3 changed files with 166 additions and 1 deletions
+3
View File
@@ -1,6 +1,7 @@
import 'dart:async';
import 'package:enough_mail/enough_mail.dart' as imap;
import 'package:flutter/services.dart' show MissingPluginException;
import 'package:sharedinbox/core/models/account.dart';
import 'package:sharedinbox/core/models/email.dart' show SyncEmailsResult;
import 'package:sharedinbox/core/repositories/account_repository.dart';
@@ -294,6 +295,7 @@ class _AccountSync implements _SyncLoop {
bool _isPermanentError(Object e) {
if (isTlsConfigError(e)) return true;
if (e is MissingPluginException) return true;
final s = e.toString().toLowerCase();
// enough_mail doesn't always have typed exceptions for auth, so we check strings.
return s.contains('invalid credentials') ||
@@ -546,6 +548,7 @@ class _JmapAccountSync implements _SyncLoop {
bool _isPermanentError(Object e) {
if (isTlsConfigError(e)) return true;
if (e is MissingPluginException) return true;
final s = e.toString().toLowerCase();
return s.contains('invalid credentials') ||
s.contains('authentication failed') ||
+96 -1
View File
@@ -31,6 +31,7 @@ To resume the Claude conversation, look up the session UUID first:
import argparse
import json
import os
import re
import shlex
import subprocess
import sys
@@ -188,6 +189,40 @@ def _find_pr_for_branch(branch: str, state: str = "open") -> dict | None:
return None
def _open_issue_prs() -> list[dict]:
"""Return all open PRs with issue-{N}-fix branches, oldest-first."""
result = subprocess.run(
["fgj", "--hostname", "codeberg.org", "pr", "list",
"--repo", REPO, "--state", "open", "--json"],
capture_output=True, text=True,
)
if result.returncode != 0 or not result.stdout.strip():
return []
prs = json.loads(result.stdout)
issue_prs = []
for pr in prs:
head = pr.get("head", {})
ref = head.get("ref") or head.get("label", "").split(":")[-1]
if re.match(r"^issue-\d+-fix$", ref or ""):
issue_prs.append(pr)
issue_prs.sort(key=lambda p: p["number"])
return issue_prs
def _latest_ci_run_for_pr(pr_number: int) -> dict | None:
"""Return the latest CI run triggered by a pull_request event for the given PR number."""
data = _tea_get(f"repos/{REPO}/actions/runs?event=pull_request&limit=50")
runs = (data or {}).get("workflow_runs", [])
for run in runs:
try:
payload = json.loads(run.get("event_payload", "{}"))
if payload.get("pull_request", {}).get("number") == pr_number:
return run
except (json.JSONDecodeError, AttributeError):
pass
return None
def _merge_pr(pr_number: int) -> None:
"""Squash-merge a PR via fgj."""
_fgj("pr", "merge", str(pr_number), "--repo", REPO, "--merge-method", "squash")
@@ -474,6 +509,9 @@ def _run_loop() -> int:
"Fetch the CI logs using the task ci-logs command or the Codeberg API. "
"Identify the failure, fix it, commit, and push to the same branch. "
"Do NOT push to main, do NOT close the issue, do NOT merge the PR. "
"Do NOT reference any issue numbers in commit messages "
"(no 'closes #N', 'fixes #N', or similar) — auto-closing the wrong "
"issue via a commit message would be a bug. "
"Verify locally with 'task check' before pushing. "
"When done, stop."
)
@@ -538,6 +576,57 @@ def _run_loop() -> int:
)
return 0
# ── 2b. Catch-up: scan open issue-N-fix PRs orphaned by a cleared state ─────
# This handles PRs whose CI has passed but were never merged because the
# state file was cleared (loop restart, killed agent, manual intervention).
open_prs = _open_issue_prs()
for pr in open_prs:
pr_number = pr["number"]
pr_url = f"{REPO_URL}/pulls/{pr_number}"
head = pr.get("head", {})
branch = head.get("ref") or head.get("label", "").split(":")[-1]
m = re.match(r"^issue-(\d+)-fix$", branch or "")
issue_num = int(m.group(1)) if m else None
pr_run = _latest_ci_run_for_pr(pr_number)
if pr_run and pr_run.get("status") == "running":
print(f"Catch-up: CI {_ci_run_url(pr_run['id'])} on PR #{pr_number} still running. Waiting.")
_write_state(None, issue_num, "pending-ci")
return 0
if pr_run and pr_run.get("status") in ("failure", "error"):
print(f"Catch-up: CI {_ci_run_url(pr_run['id'])} on PR #{pr_number} failed — skipping.")
continue
if pr_run and pr_run.get("status") == "success":
print(f"Catch-up: CI passed on PR #{pr_number} ({pr_url}) — merging.")
try:
_merge_pr(pr_number)
except RuntimeError as e:
print(f"Catch-up: merge of PR #{pr_number} failed: {e} — skipping.")
continue
# Verify the merge actually happened; fgj can exit 0 without merging
# (e.g. branch-protection rules not satisfied).
if _find_pr_for_branch(branch):
print(
f"Catch-up: PR #{pr_number} is still open after merge attempt "
"— skipping to avoid infinite retry."
)
if issue_num:
_set_labels(issue_num, add=[LABEL_QUESTION], remove=[LABEL_IN_PROGRESS])
_comment_issue(
issue_num,
f"Automatic merge of PR #{pr_number} failed (PR is still open "
"after the merge command). Please merge manually.",
)
continue
if issue_num:
_close_issue(issue_num)
print(f"Merged PR #{pr_number} and closed issue #{issue_num}.")
else:
print(f"Merged PR #{pr_number}.")
return 0
# ── 3. Global CI check (agent pushed to main, or no pending issue) ────────
run = _latest_ci_run()
@@ -555,10 +644,16 @@ def _run_loop() -> int:
"Fetch the CI logs using the task ci-logs command or the Codeberg API. "
"Identify the failure, fix it, commit, and push. "
"Verify locally with 'task check' before pushing. "
"Do NOT push to main. "
"Do NOT reference any issue numbers in commit messages "
"(no 'closes #N', 'fixes #N', or similar) — this is a CI fix, "
"not an issue fix, and auto-closing the wrong issue would be a bug. "
"Do NOT close any issues. "
"When done, stop."
)
pid = _start_agent(prompt, "ci-fix")
_write_state(pid, pending_issue, "ci-fix", session_name="ci-fix")
_write_state(pid, pending_issue, "ci-fix", session_name="ci-fix",
ci_run_id=run["id"] if run else None)
return 0
# CI is ok (or no run).
+67
View File
@@ -1,6 +1,8 @@
import 'dart:async';
import 'package:flutter/services.dart' show MissingPluginException;
import 'package:mockito/annotations.dart';
import 'package:sharedinbox/core/models/account.dart';
import 'package:sharedinbox/core/models/email.dart';
import 'package:sharedinbox/core/models/mailbox.dart';
import 'package:sharedinbox/core/repositories/account_repository.dart';
@@ -30,6 +32,40 @@ void main() {
// This is hard to test without real loops, but we can verify it doesn't crash.
manager.syncNow('unknown');
});
// Regression test for issue #200: when flutter_secure_storage throws
// MissingPluginException (channel unavailable on the device), the IMAP sync
// loop must stop permanently instead of retrying indefinitely with backoff.
test(
'MissingPluginException from secure storage stops IMAP sync loop permanently',
() async {
final syncLog = FakeSyncLogRepository();
final m = AccountSyncManager(
_AccountRepositoryWithMissingPlugin(),
FakeMailboxRepositoryWithInbox(),
FakeEmailRepository(),
syncLog: syncLog,
);
m.start();
// Allow the first sync cycle to run and fail.
await Future<void>.delayed(const Duration(milliseconds: 100));
expect(syncLog.logs, hasLength(1));
expect(syncLog.logs.first.success, isFalse);
// Kicking the loop should have no effect once it has stopped permanently.
m.syncNow('1');
await Future<void>.delayed(const Duration(milliseconds: 100));
// Before the fix: kick triggers a retry → 2 log entries.
// After the fix: loop is permanently stopped → still exactly 1 entry.
expect(syncLog.logs, hasLength(1));
m.dispose();
});
}
class FakeEmailRepository implements EmailRepository {
@@ -187,3 +223,34 @@ class FakeMailboxRepositoryWithInbox implements MailboxRepository {
@override
Future<void> clearForResync(String accountId) async {}
}
class _AccountRepositoryWithMissingPlugin implements AccountRepository {
static const _account = Account(
id: '1',
displayName: 'Test',
email: 'test@example.com',
);
@override
Stream<List<Account>> observeAccounts() => Stream.value([_account]);
@override
Future<Account?> getAccount(String id) async => _account;
@override
Future<String> getPassword(String accountId) => Future.error(
MissingPluginException(
'No implementation found for method read on channel '
'plugins.it.nomads.com/flutter_secure_storage',
),
);
@override
Future<void> addAccount(Account account, String password) async {}
@override
Future<void> updateAccount(Account account, {String? password}) async {}
@override
Future<void> removeAccount(String id) async {}
}