Should PR comment cache entries be persisted across repository cache runs?

[ag-linden]

How are you running Renovate?

Self-hosted Renovate CLI.

Which platform are you running Renovate on?

GitHub.com.

Which version of Renovate are you using?

Current main behavior observed at 81040a45ae29e7947dd91c835530a1470d778368.

Please tell us more about your question or problem

I opened PR #44296 with one possible implementation, but the PR was closed as needing discussion first.
I would like to confirm the expected behavior before continuing.

ensureComment() currently stores a desired comment body hash in repoCache.prComments[number][topic].
When the desired content hash matches that cached value, the wrapper returns true without calling the platform ensureComment() implementation.

That avoids duplicate platform calls when the same comment is ensured more than once in a single run.
The part I wanted to clarify is whether the same comment-assurance hash should be restored from persisted repository cache across later runs.

A concrete scenario:

1. Repository cache is enabled
2. Renovate ensures a PR comment and stores the hash in repository cache
3. The remote platform comment is edited or deleted outside Renovate
4. A later Renovate run wants the same topic/content, so the desired hash is unchanged
5. The restored cache hash matches, so Renovate skips the platform call and reports success even though the remote comment may be stale or missing

My assumption in PR #44296 was that platform comments are mutable remote state, so persisted repository cache should not be treated as proof that a comment still exists or still has the expected body.
The implementation kept the existing in-run dedupe but made prComments runtime-only by dropping it on repository-cache load and excluding it from repository-cache persistence.

Before reopening or revising the PR, should Renovate:

• keep prComments as runtime-only state
• continue allowing persisted prComments to suppress platform checks across runs
• handle this some other way, such as adding a separate cache lifetime or invalidation boundary for comment assurance

Logs

No runtime logs.
This is based on the current code path and the behavior in PR #44296.

[RahulGautamSingh]

What is the reason to edit a comment made by renovate? or delete it?

FYI Renovate deletes outdated comments itself.

[ag-linden]

Thanks, that is a fair question. I may have framed the example too broadly around edit/delete.

I agree that when Renovate removes an outdated comment through ensureCommentRemoval(), the wrapper updates repoCache.prComments, so that path stays internally consistent.

The case I was trying to clarify is narrower: should a comment hash restored from repository cache be enough to skip checking platform state on a later run?

ensureComment() is used for active-state comments such as onboarding/reconfigure notices, lock file or artifact update errors, edited/blocked PR notices, and automerge failure comments. In those paths, the current desired state appears to be "this comment should exist with this body" until Renovate changes the content or calls ensureCommentRemoval().

If a maintainer deletes one of those bot comments while triaging a PR, or if some platform-side cleanup/moderation removes or edits it, a later Renovate run can still want the exact same comment body. With a persisted prComments hash, the wrapper returns true before calling the platform, so GitHub is never queried and the active comment is not restored.

So the behavior question is: should manual/platform-side deletion or editing of a Renovate comment be treated as intent to suppress that exact comment until its body changes, or should ensureComment() reassert active comments on later runs? If suppression is intended, the current behavior may be fine but probably deserves explicit test coverage. If reassertion is intended, then keeping prComments runtime-only was the adjustment I was aiming for.

[RahulGautamSingh]

I think the choice, here is between: should we make one extra API call per renovate pr(which has a comment) OR should we ask users to reset their repo cache when such a case arrives for them

The latter makes more sense to me.

[ag-linden]

Thanks, that framing helps. I agree this comes down to whether persisted prComments is intended as a performance cache only, or also as an implicit suppression state until the repo cache is reset.

If the intended behavior is “manual/platform-side removal can be handled by resetting repository cache,” then I’m fine closing the runtime-only approach. In that case, would a small regression test or comment be useful to make the behavior explicit? The part that surprised me is mostly that ensureComment() reads like active-state reconciliation, while restored prComments makes it skip reconciliation across runs.

[RahulGautamSingh]

Yes, you can add a test which confirms that when repo cache is reset, ensureComment creates a pr or looks for it before creating it