Compare commits

...
Author SHA1 Message Date
Thomas SharedInboxandClaude Sonnet 4.6 d16c38006d fix(agent_loop): harden loop against redundant agents, bad merges, wrong closures
- Add merge verification to the pending_issue PR path (section 2), matching
  the catch-up scan fix: if the PR is still open after _merge_pr returns, set
  State/Question instead of claiming success and leaving the issue closed with
  an unmerged PR.

- Replace _latest_ci_run() with _latest_main_ci_run() that filters to
  non-pull_request events on the 'main' prettyref.  The old limit=1 query
  could return a PR-branch run, causing section 3 to misread CI as failed
  and spawn a ci-fix agent when main was actually fine.

- Guard against redundant ci-fix agents: when the same main CI run ID has been
  failing since the previous ci-fix started (agent pushed to a branch, not
  main), check for any in-flight CI run before spawning another agent.

- Issue agent prompt: explicitly forbid "Closes #N" / "Fixes #N" in commit
  messages.  The loop is responsible for closing issues after CI passes;
  commit-keyword auto-close would race with or bypass that logic.

- Global ci-fix prompt: restore "push directly to main" (ci-fix agents need to
  land on main to clear the main CI run) and keep the "no issue references"
  guard added in the previous commit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 12:46:45 +02:00
Thomas SharedInboxandClaude Sonnet 4.6 148ceaa509 fix: show git hash as clickable link above stacktrace on crash screen (#201)
Previous PRs (#150, #179) added partial implementations that left duplicate
code via a rebase conflict: plain (non-linked) text above the stacktrace and a
clickable link section below it. This consolidates both into a single clickable
link above the stacktrace.

Also makes `gitHash` an injectable constructor parameter so tests can exercise
the link without needing a release build.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-24 12:46:45 +02:00
Thomas SharedInboxandClaude Sonnet 4.6 9752fc2d06 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 12:46:45 +02:00
3 changed files with 162 additions and 52 deletions
+24 -35
View File
@@ -10,12 +10,12 @@ class CrashScreen extends StatelessWidget {
super.key,
required this.exception,
required this.stackTrace,
this.gitHash = const String.fromEnvironment('GIT_HASH'),
});
final Object exception;
final StackTrace? stackTrace;
static const _gitHash = String.fromEnvironment('GIT_HASH');
final String gitHash;
Future<String> _buildReport() async {
String version = 'unknown';
@@ -25,8 +25,8 @@ class CrashScreen extends StatelessWidget {
} catch (_) {}
final platform =
'${Platform.operatingSystem} ${Platform.operatingSystemVersion}';
final gitLine = _gitHash.isNotEmpty
? 'Git Commit: [$_gitHash](https://codeberg.org/guettli/sharedinbox/commit/$_gitHash)\n'
final gitLine = gitHash.isNotEmpty
? 'Git Commit: [$gitHash](https://codeberg.org/guettli/sharedinbox/commit/$gitHash)\n'
: '';
return 'App Version: $version\n'
'$gitLine'
@@ -56,12 +56,27 @@ class CrashScreen extends StatelessWidget {
style: Theme.of(ctx).textTheme.titleMedium,
textAlign: TextAlign.center,
),
if (_gitHash.isNotEmpty) ...[
if (gitHash.isNotEmpty) ...[
const SizedBox(height: 8),
const Text(
'Git Commit: $_gitHash',
style: TextStyle(fontSize: 12, color: Colors.grey),
textAlign: TextAlign.center,
GestureDetector(
onTap: () async {
final url = Uri.parse(
'https://codeberg.org/guettli/sharedinbox/commit/$gitHash',
);
await launchUrl(
url,
mode: LaunchMode.externalApplication,
);
},
child: Text(
'Git Commit: $gitHash',
style: const TextStyle(
fontSize: 12,
color: Colors.blue,
decoration: TextDecoration.underline,
),
textAlign: TextAlign.center,
),
),
],
const SizedBox(height: 24),
@@ -106,32 +121,6 @@ class CrashScreen extends StatelessWidget {
),
),
],
if (_gitHash.isNotEmpty) ...[
const SizedBox(height: 16),
const Text(
'Git Commit:',
style: TextStyle(fontWeight: FontWeight.bold),
),
const SizedBox(height: 4),
GestureDetector(
onTap: () async {
final url = Uri.parse(
'https://codeberg.org/guettli/sharedinbox/commit/$_gitHash',
);
await launchUrl(
url,
mode: LaunchMode.externalApplication,
);
},
child: const Text(
_gitHash,
style: TextStyle(
color: Colors.blue,
decoration: TextDecoration.underline,
),
),
),
],
const SizedBox(height: 24),
FilledButton.icon(
onPressed: () async {
+94 -17
View File
@@ -8,12 +8,15 @@ Flow
a. Age > 1 h → kill it, set its issue to State/Question, exit 1
b. Age ≤ 1 h → print status, exit 0 (let it keep working)
2. No agent running → extract pending_issue from state (if any), then check CI
a. CI is running → save pending-ci state, exit 0
b. Latest CI failed → start fix-CI agent (preserving pending_issue), exit 0
c. CI ok + pending_issue → close the issue (CI passed), exit 0
d. CI ok (or no run yet) → find oldest Ready issue, start issue agent,
save state, exit 0
e. No Ready issues → print "nothing to do", exit 0
a. pending_issue + open PR → check PR branch CI, merge/fix/wait as needed
b. Catch-up: orphaned issue-N-fix PRs with passing CI → merge them
c. Main CI running → save pending-ci state, exit 0
d. Main CI failed → start fix-CI agent (pushes fix to main), exit 0
e. Main CI ok + pending_issue → close the issue, exit 0 (dead code path —
section 2a always returns first)
f. Main CI ok (or no run yet) → find oldest Ready issue, start issue agent,
save state, exit 0
g. No Ready issues → print "nothing to do", exit 0
Issue agents must NOT close the issue themselves; the loop closes it after CI passes.
@@ -142,10 +145,19 @@ def _ready_issues() -> list[dict]:
return ready
def _latest_ci_run() -> dict | None:
data = _tea_get(f"repos/{REPO}/actions/runs?limit=1")
def _latest_main_ci_run() -> dict | None:
"""Return the latest CI run on the main branch (excludes PR runs).
Using the global latest run (limit=1) is wrong: a passing or failing run
on a PR branch could mask the true state of main. We filter to non-PR
events on the 'main' prettyref so section-3 logic only reacts to main.
"""
data = _tea_get(f"repos/{REPO}/actions/runs?limit=20")
runs = (data or {}).get("workflow_runs", [])
return runs[0] if runs else None
for run in runs:
if run.get("event") != "pull_request" and run.get("prettyref") == "main":
return run
return None
def _latest_ci_run_for_branch(branch: str) -> dict | None:
@@ -509,6 +521,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."
)
@@ -547,7 +562,25 @@ def _run_loop() -> int:
# CI passed on the PR branch — squash-merge and close.
print(f"CI passed {_ci_run_url(pr_run['id'])} on branch {branch!r} — merging PR #{pr_number}.")
_merge_pr(pr_number)
try:
_merge_pr(pr_number)
except RuntimeError as e:
print(f"Merge of PR #{pr_number} failed: {e} — setting to State/Question.")
_set_labels(pending_issue, add=[LABEL_QUESTION], remove=[LABEL_IN_PROGRESS])
_comment_issue(
pending_issue,
f"Automatic merge of PR #{pr_number} failed: {e}. Please merge manually.",
)
return 0
if _find_pr_for_branch(branch):
print(f"PR #{pr_number} is still open after merge attempt — setting to State/Question.")
_set_labels(pending_issue, add=[LABEL_QUESTION], remove=[LABEL_IN_PROGRESS])
_comment_issue(
pending_issue,
f"Automatic merge of PR #{pr_number} failed (PR is still open after the "
"merge command). Please merge manually.",
)
return 0
_close_issue(pending_issue)
print(f"Merged PR #{pr_number} and closed {_issue_url(pending_issue)}.")
return 0
@@ -597,7 +630,26 @@ def _run_loop() -> int:
if pr_run and pr_run.get("status") == "success":
print(f"Catch-up: CI passed on PR #{pr_number} ({pr_url}) — merging.")
_merge_pr(pr_number)
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}.")
@@ -605,8 +657,8 @@ def _run_loop() -> int:
print(f"Merged PR #{pr_number}.")
return 0
# ── 3. Global CI check (agent pushed to main, or no pending issue) ────────
run = _latest_ci_run()
# ── 3. Global CI check (main branch only) ────────────────────────────────
run = _latest_main_ci_run()
if run and run.get("status") == "running":
print(f"CI run {_ci_run_url(run['id'])} is still running. Waiting.")
@@ -615,17 +667,39 @@ def _run_loop() -> int:
return 0
if run and run.get("status") in ("failure", "error"):
# Guard: if the same main CI run has been failing since the last ci-fix
# agent started, that agent pushed to a branch instead of main. Before
# spawning another agent, check whether any CI run is currently in
# progress (the branch run) and wait if so.
if ci_run_id_at_start is not None and run["id"] == ci_run_id_at_start:
check = _tea_get(f"repos/{REPO}/actions/runs?limit=5")
in_flight = [
r for r in (check or {}).get("workflow_runs", [])
if r.get("status") == "running"
]
if in_flight:
print(
f"Main CI still shows the same failed run {run['id']}; "
f"{_ci_run_url(in_flight[0]['id'])} is running "
"(previous ci-fix pushed to a branch). Waiting."
)
return 0
print(f"CI run {_ci_run_url(run['id'])} failed — starting fix agent.")
prompt = (
"The Codeberg CI for guettli/sharedinbox just failed. "
"The Codeberg CI for guettli/sharedinbox just failed on the main branch. "
f"The CI run ID is {run['id']}. "
"Fetch the CI logs using the task ci-logs command or the Codeberg API. "
"Identify the failure, fix it, commit, and push. "
"Identify the failure, fix it, commit, and push directly to main. "
"Verify locally with 'task check' before pushing. "
"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 an issue via a commit message 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).
@@ -684,7 +758,10 @@ Instructions:
- Implement the required change, following the existing code style.
- Write or update tests as appropriate.
- Run 'task check' locally and fix any failures before committing.
- Commit with a descriptive message referencing the issue number (e.g. "feat: ... (#{issue_number})").
- Commit with a descriptive message and include (#{issue_number}) in the title,
e.g. "feat: description (#{issue_number})".
Do NOT use "Closes #N" or "Fixes #N" keywords — the loop closes the issue
after CI passes; using those keywords would close it prematurely or wrongly.
- Create a branch named `issue-{issue_number}-fix`, push your changes there, and open a PR against main:
git checkout -b issue-{issue_number}-fix
git push -u origin issue-{issue_number}-fix
+44
View File
@@ -123,6 +123,50 @@ void main() {
},
);
testWidgets(
'CrashScreen shows git hash as clickable link above stacktrace',
(tester) async {
tester.view.physicalSize = const Size(800, 1200);
tester.view.devicePixelRatio = 1.0;
addTearDown(() => tester.view.resetPhysicalSize());
final mock = MockUrlLauncher();
UrlLauncherPlatform.instance = mock;
const exception = 'TestException: git hash test';
final stackTrace = StackTrace.current;
const testHash = 'abc1234';
await tester.pumpWidget(
CrashScreen(
exception: exception,
stackTrace: stackTrace,
gitHash: testHash,
),
);
// Git hash link should be present
final gitLinkFinder = find.textContaining('Git Commit: abc1234');
expect(gitLinkFinder, findsOneWidget);
// Link must appear above the stack trace
final stackTraceFinder = find.text('Stack Trace:');
expect(
tester.getTopLeft(gitLinkFinder).dy,
lessThan(tester.getTopLeft(stackTraceFinder).dy),
);
// Tapping the link should open the Codeberg commit URL
await tester.tap(gitLinkFinder);
await tester.pumpAndSettle();
expect(
mock.launchedUrl,
equals('https://codeberg.org/guettli/sharedinbox/commit/abc1234'),
);
},
);
testWidgets(
'CrashScreen used as root widget — buttons work without ScaffoldMessenger crash',
(tester) async {