Skip to content

ci: delete the detectNewJavaFiles workflow (XNT-200) - #16

Merged
v-giannakopoulos merged 1 commit into
mainfrom
xnt-200-delete-detect-new-java-files
Sep 26, 2026
Merged

v-giannakopoulos merged 1 commit into
mainfrom
xnt-200-delete-detect-new-java-files

Conversation

@v-giannakopoulos

Copy link
Copy Markdown

Closes XNT-200.

The problem

detectNewJavaFiles.yml runs on ubuntu-latest-low, a label this organisation does not have. The job is never assigned a runner, never reaches a terminal state, and a non-terminal job holds its whole run non-terminal. So every pull request in this repository sits permanently one check short of green, with no error to read. It did this to PR #14, which had to be merged by hand, and it is doing it to PR #15 right now.

Unlike the other four ubuntu-latest-low workflows in this repo, this one has no guard: pull_request, no if:, so it is scheduled on every PR and then waits forever.

It has never worked here, not once

15 runs since 2026-09-07 — 14 cancelled, 1 still queued, zero successes.

The cancelled conclusion is its own trap. It reads as though someone cancelled the run; what actually happened is the concurrency group superseded a job still waiting for a runner that was never coming. Nothing in the signal says "this cannot run".

Why delete rather than disable or repoint

  • Delete — follows XNT-124, which deleted this same file from files-android. Chosen. The fork diff states the decision instead of hiding it in repository settings, and CONTRIBUTING.md can be corrected in the same commit.
  • Disable via gh workflow disable — zero fork diff and survives upstream syncs better, but leaves CONTRIBUTING.md advertising a check that silently does nothing.
  • Repoint to ubuntu-latest — rejected.

One correction to the ticket on that last option. It argued repointing would fail permanently because the repo contains .java files. It would not. The script only inspects steps.file_changes.outputs.files_added, so the 178 .java files already present are irrelevant; it would only fail on a PR that adds a new one. The reason to delete is the XNT-124 precedent and the Kotlin-first posture, not that.

CONTRIBUTING.md

CONTRIBUTING.md linked this workflow as the thing that enforces the Kotlin-only rule for new source files. That claim has been false for the entire life of the fork — the check never produced a verdict. Rather than leave a link to a deleted file, the rule now stands as a review convention and the section says so. The rule itself is unchanged.

If an upstream sync re-adds the file

This was the stated argument against deletion, so it deserves an answer rather than silence. Deletion is not self-protecting; a sync merge can reintroduce the file. Two things would surface it:

  1. Syncs land as a pull request, not a push to main. xenia-upstream-sync.yml pushes to upstream-sync/main and opens a PR via the REST API (PRs Upstream sync: 57 new commit(s) from nextcloud/notes-android #7 and Upstream sync: 8 new commit(s) from nextcloud/notes-android #13 are the precedent), so a reintroduced file appears in that PR's diff for review.
  2. The symptom announces itself on that same PR. A re-added workflow would immediately hang the sync PR's own check run, which is exactly how this was found in the first place.

Worth being precise about the limit: notice (2) depends on someone reading the sync PR's checks, and autoApproveSync.yml is itself on ubuntu-latest-low so it never auto-approves anything here — sync PRs need manual attention today, which is what makes (1) reliable. If sync auto-approval is ever fixed, this deletion loses its backstop and would want a positive guard instead. Flagging, not fixing — out of scope here.

The other four ubuntu-latest-low workflows are out of scope, and confirmed harmless

Checked rather than assumed, because deleting this file only meets the goal if nothing else blocks terminal state:

Workflow Why it does not block a PR
pr-feedback.yml schedule only, plus if: github.repository_owner == 'nextcloud'
stale.yml schedule only
autoApproveSync.yml job-level if: requires the sync label and github.actor == 'nextcloud-android-bot'
renovate-approve-merge.yml job-level if: requires renovate[bot]; no renovate config in this fork

The distinction that matters: a job-level if: is evaluated before runner assignment, so a false guard yields skipped — which is terminal. detectNewJavaFiles has no guard, which is why it queues instead. PR #15 shows both halves empirically: Auto approve sync and auto-approve-merge report skipping, while detectNewJavaFiles reports pending 0.

Caveat for whoever picks up XNT-193's per-file decision: renovate-approve-merge and autoApproveSync would hang on a PR that does match their guard. Neither actor is active in this fork today, so neither blocks anything now.

Evidence

Before — PR #15 (xnt197-conflict-marker-guard, not mine), read live:

reuse-compliance-check   pass       15s
conflict-markers         pass        5s
detectNewJavaFiles       pending     0     <- never resolves
Auto approve sync        skipping    0
auto-approve-merge       skipping    0

After — the checks on this PR, below. Because the workflow file is deleted in this branch, detectNewJavaFiles is not scheduled at all, and every check should reach a terminal state. That is the actual goal; the file being gone is only the means.

🤖 Generated with Claude Code

It runs on `ubuntu-latest-low`, a label this organisation does not have, so
the job is never assigned a runner and never reaches a terminal state. A
non-terminal job holds its whole run non-terminal, so every pull request in
this repository sits permanently one check short of green with no error to
read. It was visibly doing this to PR #14, which had to be merged by hand,
and it is doing it to PR #15 right now.

The workflow has never once produced a verdict in this fork. Fifteen runs
since 2026-09-07: fourteen `cancelled` and one still `queued`, no successes.
The `cancelled` conclusion is its own trap — it reads as though someone
cancelled the run, when in fact the concurrency group superseded a job that
was still waiting for a runner that was never coming.

Deleted rather than repointed, following XNT-124 which deleted this same
file from files-android. Disabling it was the other candidate; deletion was
chosen so the fork diff states the decision rather than hiding it in
repository settings.

CONTRIBUTING.md advertised this workflow as the thing that enforces the
Kotlin-only rule for new files. That claim was false for the entire life of
the fork, so it is corrected here rather than left pointing at a deleted
file: the rule stands, as a review convention.

One correction to the ticket, which argued against repointing to
`ubuntu-latest` on the grounds that it would fail permanently because the
repo contains `.java` files. It would not. The script only inspects
`files_added`, so the 178 `.java` files already present are irrelevant and
it would only fail on a pull request that adds a new one. The reason to
delete is XNT-124 precedent and the Kotlin-first posture, not that.

Assisted-by: ClaudeCode:claude-opus-5
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the AI assisted This PR contains AI-assisted commits label Sep 26, 2026
@v-giannakopoulos
v-giannakopoulos merged commit 5bbe409 into main Sep 26, 2026
11 checks passed
@v-giannakopoulos
v-giannakopoulos deleted the xnt-200-delete-detect-new-java-files branch September 26, 2026 17:42
@v-giannakopoulos

Copy link
Copy Markdown
Author

Correcting a misleading line in my own PR description, flagged by another session working on PR #15.

My table above is headed "Why it does not block a PR" and that column is accurate — but read quickly it gives four workflows a clean bill of health, and one of them does not deserve it.

stale.yml is broken today, not latent. Same signature as the file this PR deletes: 18 cancelled, 1 queued, zero successes ever. It is scheduled daily, queues at ~03:40, and gets cancelled roughly 24 hours later when the next day's run supersedes it. So the daily stale sweep has silently never run in this fork.

It does not change anything this PR claims. stale.yml is not a PR check, so it cannot hold a pull request non-terminal, and the terminal-state evidence for #16 stands as written. But "does not block a PR" is not the same as "works", and my table blurred the two — which is precisely the failure mode this whole line of tickets is about. So, restated honestly:

Workflow Blocks PR terminal state? Actually works?
detectNewJavaFiles.yml Yes — no guard, queues forever No — 0 successes. Deleted here.
stale.yml No — not a PR check No — 0 successes, broken today
autoApproveSync.yml No — guard evaluates false → skipped Unknown — will hang the first time its condition is true
renovate-approve-merge.yml No — guard evaluates false → skipped Unknown — same
pr-feedback.yml No — schedule + owner guard Not applicable in this fork

The audit trap worth writing down: skipped reads as healthy. A condition-guarded job on a dead runner label lands skipped and looks fine indefinitely; run history will not reveal it until the day the condition is true. Grepping for the label is the only reliable way to find these — which is how the remaining four were found, not from any signal.

Still out of scope for this PR by instruction, and I am not widening it unilaterally. Raised with the orchestrator to decide between widening XNT-200 or a follow-up ticket; stale.yml at least is an active failure rather than a latent one.

🤖 Generated with Claude Code

@v-giannakopoulos

Copy link
Copy Markdown
Author

Retracting the autoApproveSync.yml claim in my previous comment. Another session challenged it, I verified it myself, and they are right. The reversibility argument for this PR stands — but not for the reason I gave, and the real reason is better.

What I got wrong. I wrote that autoApproveSync.yml is what would auto-approve our upstream sync PRs, that its never having run is why sync PRs need manual attention, and that fixing its runner label would therefore remove the backstop this deletion relies on. All three are false, for two independent reasons:

  1. It could never apply to our sync PRs, label fixed or not. Its job condition is contains(labels.*.name, 'sync') && github.actor == 'nextcloud-android-bot', and the file header says # synced from @nextcloud/android-config. It is upstream's mechanism for their config-sync bot. Our sync PRs fail both halves: Upstream sync: 8 new commit(s) from nextcloud/notes-android #13 is author=app/xenia-upstream-sync, labels=[], Upstream sync: 57 new commit(s) from nextcloud/notes-android #7 is author=xeniacloud, labels=["AI assisted"].
  2. It has no merge step. Only hmarr/auto-approve-action. Even if both conditions matched, it approves and stops — approval merges nothing.

So there is no backstop there to remove, and fixing that label cannot endanger this deletion.

The actual reason the reversibility argument holds, which is stronger. xenia-upstream-sync.yml never auto-merges by design, and says so in two places — line 8: "this workflow deliberately NEVER merges anything automatically — it only surfaces what upstream has and lets a human decide", and in the PR body it generates: "This PR is never auto-merged — review each commit for conflicts with our branding, bundle ID, CalVer versioning, and release plumbing before merging."

That matters because my original framing made the backstop contingent on a broken workflow staying broken — which would have been a genuinely fragile thing to rely on. It isn't. Mandatory human review of sync PRs is the designed state, documented at the point it is enforced, and it does not depend on any dead runner label.

One thing I found while checking, which cuts the other way. main on this repository has no branch protection at all — GET /repos/Krateos-BV/notes-android/branches/main/protection returns 404 Branch not protected.

That matters for the label sweep, not for this PR. renovate-approve-merge.yml really does merge: gh pr merge --merge --auto with a dedicated secrets.AUTOMERGE token, deliberately not GITHUB_TOKEN. --auto waits on required status checks and nothing else — and with no branch protection there are no required checks, so it would merge as soon as it can. The dead runner label is currently the only thing preventing an auto-merging robot on an unprotected default branch. Whoever repoints these labels wakes that robot; branch protection should land first. Recorded here as evidence rather than acted on — out of scope for this PR, and it belongs with XNT-197's branch-protection work.

Nothing in this retraction changes what this PR does or the terminal-state evidence for it.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI assisted This PR contains AI-assisted commits

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant