Skip to content

fix(circadian): let Flow cards continue before retries finish - #72

Closed
Tiwas wants to merge 7 commits into
mainfrom
clg-background-retries
Closed

Tiwas wants to merge 7 commits into
mainfrom
clg-background-retries

Conversation

@Tiwas

@Tiwas Tiwas commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

Why

Homey stops an app Flow card after 60 s, and the Flow then stops at that card. Since v1.10.25 the Circadian Light Group and Collection cards waited for the first pass, verification and two retry passes. A light that does not answer costs up to 10 s per write (homey-api DEFAULT_TIMEOUT) in every pass. On Lars's Homey on 2026-10-08, "All on" ran about 53 s from the first light to the Collection finishing, with three lights that do not answer.

What changes

  • runDeviceTasksParallel has a deferRetries option. It returns after the first parallel pass and one verification. With a verify step, it first checks unconfirmed lights once more after 1.5 s, so a light that reports late is neither listed nor written twice. Lights still unconfirmed are listed in pending, and the reduced parallel retry and final serial retry run in background. A newer command stops the retries through the operation generation, also during the final serial pass, so a superseded command reports nothing.
  • Member on/off commands (runMemberCommand) and profile updates use it. A member command stays active until its retries finish, so the scheduler still waits. Verification, alarm_config, clg_error_occurred and clg_target_changed are reported when the retries finish, with the same messages as before.
  • runWithinCardTimeBudget (50 s) wraps every Circadian action card and the onoff/clg_paused capability listeners. Work still running after that goes on in the background. An error before the budget still fails the card.
  • Operations return an outcome object. Its ok keeps each operation's old boolean, so the Collection reports group errors as before, and cards without tokens still return that boolean.
  • clg_turn_on/clg_turn_off/clg_toggle return the tokens completed ("All lights confirmed", yes/no) and status (text). They are no longer deprecated, because the device's own On/Off/Toggle cards cannot return tokens. They are titled "… and report the result" in all 11 languages. Homey shows THEN cards with tokens only in Advanced Flows, so the other Circadian cards keep their definitions without tokens; a driver test checks this. Status texts are under circadian_outcome in all locales: Norwegian is translated, the other languages use the English text.
  • Collection: runAwaitedMemberGroups merges the groups' outcomes and reports group failures when every group's retries have finished. Only the newest Collection operation reports, and a group that postponed a profile update is not a failed group. The Collection queue is released when the card's part is done. The group error message used to list undefined; it now names the groups.

Behaviour change

The v1.10.25 promise that the next card waits for all retries no longer holds. A later card for the same group supersedes the retries. A profile card such as Resume or Apply temporary state that arrives while on/off retries are running is applied when they finish. That keeps the order, and the total delay matches the old in-card retries.

Companion tools

docs/tools/clg-editor.html: the config schema did not change, so it round-trips as before and needs no update.

Reviews

  • Local high-effort review: fixed tokens hiding existing cards from standard Flows, stale reports after supersession, the settle check running after the card had returned, and duplicated code.
  • Codex review of the first commit: fixed both P2 findings, stale Collection reports and deferred updates counted as failed groups.

Tests

  • Jest: 24 suites / 407 tests pass. npm run test:package: publish-level validation passes.
  • A live test on Lars's Homey follows before the release.

Docs: docs/docs/circadian-light-group.html, PROJECT_DOCUMENTATION.md.

Homey stops an app Flow card after 60 s, and the Flow then stops at that
card. Circadian Light Group and Collection cards waited for the first pass,
verification and two retry passes, so a few lights that do not answer
(up to 10 s per write) brought "All on" close to the limit.

- runDeviceTasksParallel can defer its retries: it returns after the first
  pass and one check, and retries unconfirmed lights in the background.
  A newer command still stops them.
- Member on/off commands and profile updates use it. Verification, the
  alarm, the error trigger and the target-changed trigger are reported
  when the retries finish, with the same messages as before.
- Every Circadian action card and the onoff/clg_paused listeners return
  within 50 s; anything still running goes on in the background.
- Nine action cards return "All lights confirmed" (yes/no) and "Status"
  (text). Turn on/off/toggle cards are no longer hidden, because the
  device's own On/Off cards cannot return tokens.
- The Collection merges its groups' results, reports group failures when
  their retries finish, and names the groups in the error message.
@Tiwas

Tiwas commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-08T19:34:44.838025Z c0f91a4 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36718a39b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const names = (outcome.groups || [])
.filter(group => group.failed || group.outcome?.ok === false)
.map(group => group.name);
await this.setCapabilityValue('alarm_config', names.length > 0).catch(this.error);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve alarms when stale background work is superseded

When a collection command starts while an earlier command is still retrying, the newer command supersedes the old member operation. If the newer command records a failure before the old background promise settles, the old result has superseded: true but no failed groups, so this unconditional write clears alarm_config and hides the newer failure. Skip collection failure reporting for superseded outcomes, or guard it with the operation generation.

Useful? React with 👍 / 👎.


async reportCollectionGroupFailures(label, outcome) {
const names = (outcome.groups || [])
.filter(group => group.failed || group.outcome?.ok === false)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exclude deferred Apply-now results from group failures

When a Collection on/off command has returned after its first pass but member retries are still active, a subsequent clg_apply_now reaches each member's applyCurrentProfile, which returns ok: false, skipped: 'deferred' and schedules the apply for later. This predicate nevertheless classifies every such member group as failed, setting the Collection alarm and firing an “unresponsive members” error even though no group failed and the update is pending as designed. Deferred outcomes should not be included in failedGroups.

Useful? React with 👍 / 👎.

Tiwas added 2 commits October 8, 2026 21:09
…re returning

Review fixes:
- Homey shows THEN cards with tokens only in Advanced Flows, so the six
  existing cards (Apply now, Resume, Turn on light, Apply temporary state,
  Force red mode, Set outdoor lux) get their definitions back without
  tokens. Only the re-enabled Turn on/off/toggle cards return tokens; a
  test checks the cards used in standard Flows.
- A command that a newer one took over during the final serial retry no
  longer reports an alarm or error.
- Unconfirmed lights are checked once more after 1.5 s before the card
  returns, so the tokens do not list lights that only reported late.
- runMemberCommand and outcomeWithPendingFailed remove duplicated code.
- Tests for the settle check, supersession in each retry stage, the
  Collection queue and the capability listeners' time budget.
… updates

Codex review:
- An older Collection operation whose background retries finish after a
  newer operation has reported no longer overwrites that report (alarm
  and error trigger). Only the newest operation reports.
- A member group that postponed a profile update until its on/off
  command has finished is not counted as a failed group.
@Tiwas

Tiwas commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d54c12f928

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +124 to +126
isFailedGroup(group) {
return group.failed
|| (group.outcome?.ok === false && group.outcome.skipped !== 'deferred');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exclude superseded member outcomes from group failures

When a member group's background profile retry is superseded by that group's own scheduler or a directly invoked member card, _applyCurrentProfileImpl returns an outcome with ok: false and superseded: true; this predicate consequently raises the Collection alarm and emits an “unresponsive members” error even though a newer command intentionally took over. Fresh evidence after the earlier report-race fix is that collectionReportGen only advances for newer Collection operations, so it does not suppress this report when the superseding operation ran directly on the member; exclude superseded group outcomes from failures.

Useful? React with 👍 / 👎.

Codex review: when a member group's background profile update is taken
over by the group's own scheduler or a card run directly on the group,
the Collection no longer sets its alarm or reports unresponsive members.
@Tiwas

Tiwas commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b298bb72d6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +248 to +249
this.collectionReportGen = (this.collectionReportGen || 0) + 1;
const reportGen = this.collectionReportGen;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Invalidate older reports before starting the new fan-out

When operation B starts while operation A still has member retries in the background, collectionReportGen is not advanced until B's entire runForMemberGroups first pass finishes. If A settles during that potentially long window, its callback still sees the old generation and can clear alarm_config or emit an error from stale results even though B has already superseded it. Fresh evidence beyond the earlier report-race finding is the placement of this increment after the awaited fan-out; advance the generation when the new Collection operation begins instead.

Useful? React with 👍 / 👎.

…tarts

Codex review: an older Collection operation whose retries finished while
a newer operation was still in its first pass could still report. The
report number is now taken before the fan-out.
@Tiwas

Tiwas commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 51a1d04570

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

label: 'turn_on_all_members',
verifyFn,
isCurrent: op.isCurrent,
deferRetries: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cancel deferred turn-on retries when pausing

When a light remains pending, this returns from the turn-on card while its retry retains the current operation generation. If the next Flow card or capability change pauses the group, onFlowPause only changes clg_paused and does not invalidate that operation; a retry that has already passed turnOnMemberToTarget's initial pause check can therefore still execute onoff=true after the pause, leaving a light on while the group reports itself paused. Make pausing supersede or await the active member command before it returns.

Useful? React with 👍 / 👎.

…eleted

Codex review: a turn-on retry running in the background could turn a
light on after the group had been paused. Pausing, through the card or
the capability, now takes over the running operation, and so does
deleting the group.
@Tiwas

Tiwas commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a1d4b5589

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

this.error(`apply[${reason}] background retry failed:`, error);
return outcomeWithPendingFailed(result);
});
return outcomeFromTaskResult(result, { ok: true, background });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve immediate profile failures in the initial outcome

When a profile update has both a non-transient failure in result.failed and at least one transient failure in result.pending, the pending item creates result.background, but this unconditional ok: true hides the failure already known from the first pass. Consequently clg_apply_now and a Collection containing this group return success before the background work settles, whereas the previous implementation returned false for that non-transient failure. Derive the initial ok value from the already-final failures while leaving pending retries unresolved.

Useful? React with 👍 / 👎.

Codex review: when a profile update had a failure that is not retried
and also lights being retried in the background, the first result said
ok and hid the failure. It now counts the failures that are final.
@Tiwas

Tiwas commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: c0f91a4925

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Tiwas

Tiwas commented Oct 8, 2026

Copy link
Copy Markdown
Owner Author

Replaced by #73, which has the same content in one commit. This PR's commit messages named a review tool, which the message check rejects.

@Tiwas Tiwas closed this Oct 8, 2026
@Tiwas
Tiwas deleted the clg-background-retries branch October 8, 2026 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant