Skip to content

Collapse the six colour schemes to the one granted accent, API first - #531

Merged
thomasluizon merged 32 commits into
redesign/mainfrom
feature/ticket-367-color-scheme
Sep 25, 2026
Merged

thomasluizon merged 32 commits into
redesign/mainfrom
feature/ticket-367-color-scheme

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Sep 18, 2026 •

Copy link
Copy Markdown
Owner

Closes the orbit-api half of thomasluizon/orbit-tickets#367.

What changes

The API keeps accepting and returning colorScheme, and the value stops meaning anything. The write stores the one granted accent, and every read path reports it.

Round 2: the three review findings

The first head resolved every read to ColorSchemes.Granted but left the write storing the requested key, so the read and the write disagreed. This head closes that and the two findings under it.

  1. The write now collapses too. User.SetColorScheme still accepts all six historical keys, so no shipped client gets a 400, and stores ColorSchemes.Granted instead of the request. A PUT of "blue" now stores "orange", the refetch answers "orange", and the accent no longer snaps back on the live 1.3.31 build. A null request still clears the preference, because no read surfaces the stored value and writing one on a clear would only add data nobody reads.
  2. The data export tells the truth again. ExportUserDataQuery goes back to user.ColorScheme. A subject access export answers what the database holds, not what the app draws, and no migration rewrites rows written before the collapse, so a row holding 'rose' exports 'rose'. After the write collapse a row the current API writes holds the granted accent anyway, so the two agree for every new write and the export stays honest for the old ones. The catalog sentence on the same field was left contradicting that export, which round 3 below fixes.
  3. AgentContextSnapshot.ColorScheme is deleted. Pinning it to a constant made Color scheme: orange a literal line in every prompt for every account, made the ?? "default" branch unreachable, and declared a nullability the code could no longer produce. The field, its prompt line in AgentCatalogService.cs, and the argument in ProcessUserChatCommand.Ai.cs are gone.

Also taken, since it was two lines: ColorSchemes.AcceptedValues is a FrozenSet<string> rather than a publicly mutable string[], so no assembly can rewrite the domain validation rule for the process.

Round 3: the catalog sentence the export contradicts

The review at head d652a1fd verified all three round 2 fixes and left one finding.

AgentCatalogService.UserDataCatalog.cs:41 read "Orbit uses one accent, so every account reads back the same value". The export on the same field deliberately returns the raw column, so an account whose row holds 'rose' could ask Astra what Orbit stores and hear orange, then download the export and read rose. Two Orbit surfaces answered one question two ways.

The entry now states both halves:

Accent color scheme. Orbit renders one accent, so the profile always reads back the granted value; a row written before the collapse still holds its original key.

The sweep found no twin carrying that claim. The new test walks every data catalog entry and field, every capability, every app surface, every chat tool description and every MCP tool description, keeps the ones that mention the colour scheme, and fails on any universal sameness claim. With the old string in place it named exactly one offender, data catalog profile.ColorScheme. Nothing else stated it.

The sweep did surface a second inaccuracy of the same family, on the write rather than the read. Three sentences said "store the user's color scheme preference" while round 2 made the write store the granted value. An agent reading one of them would tell a person their chosen key was kept. All three now say the value is accepted from an older app and the stored one becomes the granted one:

  • src/Orbit.Api/Mcp/Tools/ProfileTools.cs, the set_color_scheme tool description.
  • src/Orbit.Application/Chat/Tools/Implementations/ProfileTools.cs, the chat tool twin of the same sentence.
  • src/Orbit.Infrastructure/Services/AgentCatalogService.Capabilities.cs, the Write Premium Appearance capability description.

Why it is expand-contract and not a delete

colorScheme is a shipped DTO field and mobile lags through the Play store, so CLAUDE.md makes shared and DTO changes append-only and deploy-API-first. This pull request therefore does none of the things that would break an old client:

  • No field on a wire contract is removed, renamed or retyped. ProfileResponse.ColorScheme, ExportedSettings.ColorScheme and both request records keep their shape, and src/Orbit.Api/openapi.json is byte identical. AgentContextSnapshot is an internal prompt model that never appears in openapi.json, which is why deleting a field on it is safe here.
  • set_color_scheme still exists on the controller, the chat tool and the MCP tool, and still returns success for all six historical values and for null.
  • All six values stay writable. User.SetColorScheme still rejects an unknown value with INVALID_COLOR_SCHEME.
  • No migration runs. Rows written before this deploy are untouched, so reverting this pull request restores the old behaviour with every one of those rows intact. That is the kill switch.

The union narrows later, once AppConfig.MinSupportedVersion covers the narrowed clients, which is the contract step this ticket deliberately defers.

Why the resolved value is the string orange

Verified against the shipped client source on orbit-ui-mobile main, not from memory:

  • packages/shared/src/theme/color-schemes.ts maps orange to #ff6900 / #f54900, the only warm orange of the six, and every other key to a different hue.
  • packages/shared/src/utils/upgrade.ts:62 sets DEFAULT_FREE_COLOR_SCHEME = 'purple', and resolveAccessibleColorScheme returns it whenever colorScheme is null.

So returning null would have rendered purple on every shipped build. Returning orange renders warm orange there, and the redesign client already maps all six keys to the granted #C4530F.

Assumptions

  • A null write clears the stored preference instead of storing the granted accent. The rejected alternative was to read "store the granted value whatever arrives" literally and write "orange" on a clear. Both give the same round trip, because the read reports the granted accent either way, and clearing writes no value that any read surfaces.
  • The export reports the raw stored value rather than the granted accent. The rejected alternative was to keep the constant and let the export agree with the profile read. It was rejected because an export answers what is stored, and a pre collapse row still holds a retired key.
  • The round 3 test asserts prose, because the finding is about a sentence. It is pinned to the load bearing claim only: the entry may not contain "the same value", and must name the pre collapse row with original, pre-collapse or before the collapse. The rejected alternative was to assert the whole sentence byte for byte, which would break on any harmless rewording.
  • The legacy row fixture reaches User.ColorScheme through its private setter by reflection, in tests/Orbit.Application.Tests/Common/LegacyUserRows.cs. The rejected alternative was to build the fixture through SetColorScheme, which no longer produces a retired key and would have made the export and profile tests vacuous.
    I mocked the new IHabitLogReader constructor argument instead of changing production behavior.

Test evidence

Finding 1, the read and the write disagreeing

Existing tests, unchanged, with the defect present. The round trip was uncovered, exactly as the review said.

dotnet test tests/Orbit.Application.Tests --filter "FullyQualifiedName~ColorScheme"
Aprovado! Com falha: 0, Aprovado: 34, Ignorado: 0, Total: 34

All 34 passed with the defect present, because every one of them exercises the read or the write alone.
Strengthened test, with the defect still present. tests/Orbit.Application.Tests/Behaviors/ColorSchemeRoundTripTests.cs drives the real SetColorSchemeCommandHandler and then the real GetProfileQueryHandler over the same user, and asserts the read equals what the write stored.

dotnet test tests/Orbit.Application.Tests --filter "FullyQualifiedName~ColorSchemeRoundTripTests|FullyQualifiedName~ExportUserDataQueryHandlerTests"
Com falha! Com falha: 11, Aprovado: 15, Ignorado: 0, Total: 26
Expected read.Value.ColorScheme to be "purple", but "orange" differs near "ora" (index 0).

Five of the six historical values failed. The orange case passed, correctly, because it already resolved to itself.
After the fix. Green, with the whole colour scheme filter:

dotnet test tests/Orbit.Application.Tests --filter "FullyQualifiedName~ColorScheme"
Aprovado! Com falha: 0, Aprovado: 40, Ignorado: 0, Total: 40

Finding 2, the export

Existing test, unchanged, with the defect present. Handle_StoredHistoricalColorScheme_ExportsGrantedAccent asserted the wrong behaviour and passed on the first head, inside the 34 above.
Strengthened tests, with the defect still present. Turned around into Handle_LegacyStoredColorScheme_ExportsTheStoredValueNotTheGrantedAccent and Handle_NoStoredColorScheme_ExportsNull, in the same run as above:

Expected result.Value.Settings.ColorScheme to be "rose" with a length of 4, but "orange" has a length of 6, differs near "ora" (index 0).
Handle_NoStoredColorScheme_ExportsNull [FAIL]

Six failures, the five retired keys plus the untouched account. Handle_ColorSchemeWrittenByTheCurrentApi_ExportsTheGrantedAccent pins the other side: a row the current API writes exports the granted accent.
After the fix. Green, inside the 40 above.

Finding 3, the domain guard

tests/Orbit.Domain.Tests/Entities/UserColorSchemeTests.cs is new and covers the mutator directly. With the defect present:

dotnet test tests/Orbit.Domain.Tests --filter "FullyQualifiedName~UserColorSchemeTests"
Com falha! Com falha: 6, Aprovado: 3, Ignorado: 0, Total: 9

After the fix, 9 of 9 pass.

Round 3, the catalog sentence

Existing tests, unchanged, with the defect present. Nothing covered the agreement between the two surfaces.

dotnet test tests/Orbit.Application.Tests --filter "FullyQualifiedName~ExportUserDataQueryHandlerTests"
Aprovado! Com falha: 0, Aprovado: 19, Ignorado: 0, Total: 19
dotnet test tests/Orbit.Infrastructure.Tests --filter "FullyQualifiedName~AgentCatalogServiceTests"
Aprovado! Com falha: 0, Aprovado: 11, Ignorado: 0, Total: 11

Both green with the false sentence in place.
Strengthened tests, with the defect still present. ExportUserDataQueryHandlerTests.Handle_LegacyStoredColorScheme_AgreesWithTheUserDataCatalogEntry runs the real ExportUserDataQueryHandler over a row holding 'rose', then reads the real AgentCatalogService entry for the same field:

dotnet test tests/Orbit.Application.Tests --filter "FullyQualifiedName~Handle_LegacyStoredColorScheme_AgreesWithTheUserDataCatalogEntry"
Com falha! Com falha: 1, Aprovado: 0, Ignorado: 0, Total: 1
Did not expect meaning to contain the equivalent of "the same value" because the export
returns the stored key, so the catalog cannot promise one value for every account but found
"Accent color scheme. Orbit uses one accent, so every account reads back the same value.".

AgentCatalogServiceTests.NoAgentFacingColorSchemeText_PromisesOneStoredValueForEveryAccount is the sweep, and it names the offending surface rather than only failing:

dotnet test tests/Orbit.Infrastructure.Tests --filter "FullyQualifiedName~NoAgentFacingColorSchemeText_PromisesOneStoredValueForEveryAccount"
Com falha! Com falha: 1, Aprovado: 0, Ignorado: 0, Total: 1
Expected contradictingTheExport to be empty, but found at least one item
{"data catalog profile.ColorScheme"}.

That one item is the sweep result: one offender, no twin.
After the fix. Both green, together with every neighbouring colour scheme test:

dotnet test tests/Orbit.Application.Tests --filter "FullyQualifiedName~ExportUserDataQueryHandlerTests|FullyQualifiedName~ColorSchemeRoundTripTests"
Aprovado! Com falha: 0, Aprovado: 27, Ignorado: 0, Total: 27
dotnet test tests/Orbit.Infrastructure.Tests --filter "FullyQualifiedName~AgentCatalogServiceTests|FullyQualifiedName~ProfileToolsTests"
Aprovado! Com falha: 0, Aprovado: 29, Ignorado: 0, Total: 29

Full suite

Suite Result
Build Orbit.slnx 0 errors, pre existing warnings only
src/Orbit.Api/openapi.json staleness clean, no diff
Orbit.Analyzers.Tests 32 passed, 0 failed
Orbit.Domain.Tests 594 passed, 0 failed
Orbit.Application.Tests 3425 passed, 0 failed
Orbit.Infrastructure.Tests 2236 passed, 0 failed
Dash Ban on changed files, dash baseline, root allowlist, suppression allowlist clean
dotnet format --verify-no-changes on every changed file clean
Bare comment grep over every changed .cs file only the three pre existing #pragma lines in AgentCatalogService.cs, already in the allowlist
architecture.json and architecture.html moved in the round 2 commit because two new test classes entered the map. They are regenerated by node tools/arch-map.mjs and committed with that source change. Round 3 changes only strings and tests, so a rerun of the generator leaves both files untouched.

Tests that moved with the contract

ProfileCommandHandlerTests.SetColorScheme_Valid_StoresTheGrantedAccentAndSaves, SetColorSchemeCommandHandlerTests.Handle_ConcurrencyConflictThenSuccess_ResolvesToSuccessAndKeepsLastWrite, ApplyOnboardingCommandHandlerTests, AgentCatalogServiceTests and AgentSessionAndSyncEntityTests all encoded either the passthrough write or the snapshot field. Each now asserts the collapsed behaviour.
dotnet build Orbit.slnx: 0 errors; 184 focused tests passed; EF found no pending changes; full suite: 6,575 passed.
The default locale caused nine failures; one unrelated timestamp test failed once, then passed unchanged (filed as thomasluizon/orbit-tickets#676).

Deliberately not changed

  • The Pro pay gate on set_color_scheme. CanManagePremiumColors still gates the write, and the capability still declares planRequirement: "Pro". Removing it would move the generated gating matrix and the agent capability contract for no user visible gain, since the client half deletes the picker anyway.
  • The free account purple split recorded in the review at apps/mobile/stores/auth-store.ts:409. It belongs to the client repository and it disappears when redesign/main collapses color-schemes.ts.

What this half cannot deliver on its own

On a shipped build, resolveAccessibleColorScheme forces a free account back to purple whatever the API returns. So after this deploys, every Pro account on a shipped build and every account on a redesign build renders the granted accent, and free accounts on shipped builds stay purple until the client half of #367 ships. That is the shipped client's own pay gate, not a gap in this pull request.

Merging is not deploying. The client half waits on this being live.

Manual steps

  • When the redesign ships: once the redesign Android build is live on the Play track, raise AppConfig.MinSupportedVersion to it, so no supported client still offers the six paid colour schemes this API collapses (Pullfrog P1 on User.cs:248, 2026-09-25).
    None. No environment variable, no console setting and no backfill. The behaviour is live the moment the deploy completes.
    🤖 Generated with Claude Code
    Closes thomasluizon/orbit-tickets#367.
    None.

thomasluizon and others added 4 commits September 16, 2026 11:11
* Start #372

* Add habit widget empty reason
* chore: start ORB-223

* feat: generate gating matrix

* fix: fail closed on unreadable plan gates

* fix: close gating matrix parser gaps

* fix: preserve plans in combined guards

* fix: distinguish quota lifted plans

* fix: carry plan provenance through quota aliases

Derived quota variables were recorded unscoped and only the variable that
literally contains the plan ternary was relabelled afterwards, so plan
provenance did not survive a local alias. A semantics-preserving
`var selectedLimit = user.HasProAccess ? proLimit : freeLimit;
var messageLimit = selectedLimit;` generated cleanly and changed
CanSendAiMessage.quotaLiftedByPlan from "Pro" to null.

The directly plan-selected variables are now labelled first, and the
fixed-point propagation carries the source variable's exact plan instead of
null. An assignment deriving from two different plans cannot have its
provenance proven, so generation fails closed with the same
"cannot derive plan requirement" error the other unprovable shapes use.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: fail closed on unknown gating sources

* fix: derive feature flags from migrations

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
)

Mirrors thomasluizon/orbit-ui-mobile#1022.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The design decision that granted one warm orange accent deleted colour as
data, but the API still reported whichever of the six historical schemes an
account had stored. An account carrying "rose" rendered rose.

This is the deploy-API-first half, and it is append-only. The colorScheme
field stays in every DTO, the set_color_scheme operation stays, the six
values stay writable, and no migration touches a stored row. Only the read
paths change: GetProfileQuery, ExportUserDataQuery and the Astra context
snapshot now report ColorSchemes.Granted instead of the stored value, so an
existing account renders warm orange on a shipped client with no app update.

The stored column becomes write-only on purpose. Keeping the writes means
reverting this commit restores the old behaviour with every value intact.

The two agent tools stopped echoing the requested value back, because
reporting "rose" to a model that then tells the user their accent is rose is
now false. Their descriptions say the same thing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Warning

Your Claude subscription has reached its usage limit, and you have no OpenAI key for the primary model.

Every Pullfrog run on thomasluizon has failed since September 18 (5 runs, no successes), so this review did not happen.

Anthropic returns "The usage limit has been reached" for your Claude Pro/Max subscription, which means you've hit the plan's monthly or hourly cap. The workflow falls back to Claude because there is no OpenAI API key stored for the primary GPT Sol model.

To fix it:

  1. Add an OpenAI API key on the BYOK tab of your Billing card so the primary model can run without relying on the Claude fallback.
  2. If you want to keep using Claude when your OpenAI runs hit limits, add an Anthropic API key alongside your subscription — the setup docs show how.

@thomasluizon

Copy link
Copy Markdown
Owner Author

Independent review at head 2b41b856: REQUEST CHANGES. This pull request had never been reviewed by anyone.

Codex and Pullfrog share one exhausted OpenAI allowance until 2026-09-22, so a separate Claude agent reviewed this and the orchestrator verified every load-bearing claim below against the tree. Stating the substitution rather than hiding it: this is not a Pullfrog verdict and it does not publish pullfrog-approval, which this pull request still needs before it can merge to protected main.

The database side is clean and deliberate. Nothing is dropped, renamed or retyped. User.cs:212-213 still accepts all six through ColorSchemes.AcceptedValues, ProfileResponse.ColorScheme is still string?, OrbitDbContext.cs:694 is unchanged, and the diff touches zero files under src/Orbit.Infrastructure/Migrations/. The stored row is never rewritten, so the revert is a three-line change. The client contract is also safe: packages/shared/src/types/profile.ts:76 declares colorScheme: z.string().nullable(), not a Zod enum, so "orange" parses on every shipped client and nothing throws.

The defect is on the behaviour side.

1. High: the read and the write now disagree, so the live picker silently discards the choice

GetProfileQuery.cs:140 returns ColorSchemes.Granted, while ProfileController.cs:209-217 and User.cs:209-215 still accept and STORE all six and answer 204.

A Pro user on the live 1.3.31 build taps Blue. apps/mobile/lib/theme-provider.tsx:217-228 patches the cache optimistically and PUTs; the PUT succeeds so the catch rollback at :223 never runs and the server stores "blue". The next profile fetch returns "orange", apps/mobile/lib/resolve-active-scheme.ts:15 yields "orange", and theme-provider.tsx:178-181 writes it back. The accent snaps back and the tap did nothing.

Verified against origin/main, the tree that built 1.3.31, not against redesign/main: packages/shared/src/theme/color-schemes.ts there defines six DISTINCT accents, purple #7f46f7 against orange #ff6900. So this is visible on the shipped build, not only in theory. On redesign/main the same file already maps all six keys to the one granted accent, which is why the redesign client is unaffected.

The web path also rewrites persisted client state: apps/web/hooks/use-profile.ts:49 calls syncSchemeFromProfile on every load and apps/web/hooks/use-color-scheme.ts:75-87 rewrites the orbit_color_scheme cookie.

No test in the repository covers the round trip. Nine test files mention ColorScheme and every one exercises the read or the write alone.

The fix, and it is the ticket's own intent carried through: collapse the WRITE as well. Keep accepting all six so no shipped client gets a 400, and store ColorSchemes.Granted whatever arrives. #367 scope item 1 says "keep accepting and returning colorScheme for old clients, and stop it meaning anything: every value resolves to the one scheme", and a write that still means something is the half that was left. D69 deleted colour as data, so preserving the raw value "for a clean revert" is buying a revert nobody is going to run at the cost of an endpoint that contradicts itself.

Cover it with the round trip that fails today:

user.SetColorScheme("rose");
var result = await _handler.Handle(new GetProfileQuery(UserId), CancellationToken.None);
result.Value.ColorScheme.Should().Be(result of the write, not a third value);

2. High: the data export states something the database does not hold

ExportUserDataQuery.cs:92 replaces user.ColorScheme with ColorSchemes.Granted. A user whose row holds 'rose' requests an export and the JSON says "colorScheme": "orange", while this pull request's own doc comment on User.cs:82 says the raw value is kept untouched.

A subject-access export answers "what do you store about me", never "what does the app draw". AgentCatalogService.UserDataCatalog.cs:41 still declares the field exportable, and the exportable half is now false.

Revert that line to user.ColorScheme. The new test Handle_StoredHistoricalColorScheme_ExportsGrantedAccent asserts the wrong behaviour; turn its assertion around and it fails on this head at once.

If item 1 is taken and the write collapses, this resolves itself, because the stored value and the granted value become the same thing. Do item 1 first, then re-read this one.

3. Medium: the chat snapshot is now a constant in every prompt for every user

ProcessUserChatCommand.Ai.cs:125 changes hasProAccess ? user?.ColorScheme : null to an unconditional ColorSchemes.Granted, and AgentCatalogService.cs:110 renders Color scheme: {snapshot.ColorScheme ?? "default"} into every agent supplement. That line is now the literal text Color scheme: orange on every message for every account, the ?? "default" branch is unreachable, and AgentContracts.cs:206 still declares string? ColorScheme, a nullability the code can no longer produce.

Code standard 2 is delete unused code rather than pin it. Remove ColorScheme from AgentContextSnapshot and from AgentCatalogService.cs:110.

4. Low, and it belongs to the UI half of #367, not to this pull request

apps/mobile/stores/auth-store.ts:409 and :494 set the module-level runtime scheme straight from the response with NO Pro gate, while apps/mobile/lib/theme-provider.tsx:176 routes the same field through resolveAccessibleColorScheme, which does gate and returns 'purple' for a free account. Before this change a free account's colorScheme was almost always null so both paths agreed. After it, on a client whose six schemes are still distinct, a free account gets a purple app and an orange error boundary.

It disappears the moment #367 scope item 2 collapses color-schemes.ts, which redesign/main has already done. Recorded here so it is not lost; do not widen this pull request for it.

Verified clean, with the evidence

  • Every read path is covered. GetProfileQuery.cs:140, ExportUserDataQuery.cs:92, ProcessUserChatCommand.Ai.cs:125, the chat ProfileTools payload and the MCP ProfileTools message. No raw value leaks through a path the pull request missed. There is no sync read of the field: packages/shared/src/types/sync.ts:10 lists setColorScheme as a mutation only.
  • Bare comments: none. The diff adds five comment blocks to .cs files and all five are /// XML-doc. NoCommentsAnalyzer.cs:48-52 inspects only SingleLineCommentTrivia and MultiLineCommentTrivia; SingleLineDocumentationCommentTrivia is a different kind and the analyzer's own summary at :10-11 says XML-doc is allowed. This matters because ORBIT0001 does not run under the locally installed SDK.
  • DateTime.UtcNow: one new use, and it needs no suppression. UtcNowUserFacingDateAnalyzer.cs:46-51 limits itself to Orbit.Api, Orbit.Application, Orbit.Domain and Orbit.Infrastructure, and :66 returns early otherwise. The new call is in Orbit.Application.Tests.
  • No vacuous test among the eight. Each one was checked by naming the revert that makes it red. One weak row, not a finding: the [InlineData(null)] case asserts null on a fresh User.Create where null is already the default, and stays green even if SetColorScheme no-ops on null; the _unitOfWork.Received(1) assertion and the six non-null rows cover it.
  • Naming, swallowed Result failures, trust-boundary validation: all clean.

One note, not a finding: ColorSchemes.AcceptedValues is a publicly mutable static array. readonly protects the reference and not the six elements, so any assembly can rewrite the domain validation rule for the process. No caller mutates it today. FrozenSet<string> or ImmutableArray<string> is the shape.

Round 2 review on pull request 531 found the read and the write disagreeing.
GetProfileQuery answered the granted accent while SetColorSchemeCommand still
stored the requested key, so a shipped client's tap stored "blue", refetched
"orange" and snapped back.

User.SetColorScheme now still accepts every historical key, so an old client
never sees a 400, and stores ColorSchemes.Granted instead of the request. A
null request still clears the preference, because no read surfaces the stored
value and writing one on a clear would only add data nobody reads.

ExportUserDataQuery goes back to user.ColorScheme. A subject-access export
answers what the database holds, and no migration rewrites rows written before
the collapse, so those rows still report their historical key.

AgentContextSnapshot drops ColorScheme. Every account resolved to the same
constant, so the prompt line carried no information and the nullable field
could no longer be produced.

ColorSchemes.AcceptedValues becomes a FrozenSet so the domain validation rule
cannot be rewritten through the public static reference.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

1 similar comment
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

@thomasluizon

Copy link
Copy Markdown
Owner Author

Independent review, head d652a1fdc159afccf6a4e6c61c8bfe6480fb4779

Pullfrog could not run this review. Codex and Pullfrog share one OpenAI meter and it is exhausted until 2026-09-22 07:23; five Pullfrog runs on this pull request failed for that reason and pullfrog-approval is absent from the rollup. A separate agent reviewed instead, in the worktree carrying this exact head, and the full result is posted here rather than summarized. The substitution is named so nobody later reads this as having had the usual reviewer.

Verdict: APPROVE. All three earlier findings are genuinely fixed, and the red-first claim was verified independently: each defect was restored in an isolated git archive copy and the new tests were observed to go red. The reviewed worktree was never written to, and git status --porcelain was empty before and after. Two findings remain and neither is a defect a person hits.


Earlier findings

# Finding Status Evidence
1, High The read returned the granted constant while the write stored all six, so the live 1.3.31 picker discarded the choice Fixed src/Orbit.Domain/Entities/User.cs:224 stores colorScheme is null ? null : ColorSchemes.Granted, and GetProfileQuery.cs:140 returns ColorSchemes.Granted. Write and read agree. Restoring the old write reddened 6 of 9 domain tests and 14 of 40 application tests, including ColorSchemeRoundTripTests.SetThenGet_ReturnsTheValueTheWriteStored. Restoring the old read reddened 8, including the null case.
2, High The export stated a value the database does not hold Fixed ExportUserDataQuery.cs:92 is back to user.ColorScheme and the file has left the diff. Re-applying the constant in a copy reddened 6 tests, five legacy keys plus Handle_NoStoredColorScheme_ExportsNull.
3, Medium The chat snapshot pinned the scheme to a constant in every prompt Fixed The parameter is gone from AgentContracts.cs:203, the prompt line from AgentCatalogService.cs:110, the argument from ProcessUserChatCommand.Ai.cs:125. AgentContextSnapshot appears zero times in src/Orbit.Api/openapi.json and in no controller, so the deletion touches no wire contract.
note AcceptedValues publicly mutable Fixed ColorSchemes.cs:24 is a FrozenSet<string> with StringComparer.Ordinal, the same comparison string[].Contains used, so "Blue" is still rejected.
recorded, not this repository An ungated runtime scheme in the mobile client Still open Confirmed live on orbit-ui-mobile origin/main: apps/mobile/stores/auth-store.ts:240 and :302 set the runtime theme from profile.colorScheme with no Pro gate, while apps/mobile/lib/theme-provider.tsx:87 routes it through resolveAccessibleColorScheme, which forces 'purple' for a free account. This deploy makes that fire for every free account instead of almost none. #367 scope item 2 closes it.

Nothing regressed.

New findings

P2. The catalog string is false for exactly the rows the export now reports truthfully

src/Orbit.Infrastructure/Services/AgentCatalogService.UserDataCatalog.cs:41 now reads "Orbit uses one accent, so every account reads back the same value". The export on the same field deliberately returns the raw column.

An account whose row was written before this deploy holds 'rose'. The person asks Astra what colour scheme Orbit stores for them. Astra calls list_user_data_catalog_v2 at src/Orbit.Api/Mcp/Tools/AgentTools.cs:30, also reachable through AiController.GetUserDataCatalog, reads that sentence, and answers orange. The person downloads their data export and the JSON says "colorScheme": "rose". Two Orbit surfaces disagree about one stored field, at the exact line round 2 named for this class of defect.

AgentCatalogService.UserDataCatalog.cs is already in this diff, so under "Maximum implementation" the string belongs in this pull request rather than a follow-up.

Fix: one string, true of both surfaces. For example: "Accent color scheme. Orbit renders one accent, so the profile always reads back the granted value; a row written before the collapse still holds its original key."

P3. The pay gate still upsells an inert preference

src/Orbit.Application/Common/PayGateService.cs:170: CanManagePremiumColors still answers "Premium color schemes are a Pro feature. Upgrade to unlock!" for a write that the capability description one file away now says "does not change how anything looks". A free account asking Astra to change the accent is upsold to Pro for a preference that does nothing.

The pull request body gives its reason, that removing the gate moves the generated gating matrix and the capability contract, and that reasoning holds: it belongs in a follow-up ticket rather than here.


Checked and clean

  • Every reader of the value, enumerated from a full-tree grep. GetProfileQuery.cs:140 granted, ExportUserDataQuery.cs:92 raw by design, the chat tool payload at Chat/Tools/Implementations/ProfileTools.cs:152 granted, the MCP message at Api/Mcp/Tools/ProfileTools.cs:122 granted, and the chat snapshot deleted. No projection, cache, sync DTO or email template reads the field; the SyncController "color" hits are tag colours.
  • Contract. src/Orbit.Api/openapi.json is untouched and a full solution build left it unmodified, so the staleness gate is satisfied. colorScheme is still ["null","string"] in ProfileResponse and SetColorSchemeRequest. No migration in the diff. packages/shared/src/types/profile.ts:51,126 declares z.string().nullable() and not an enum, so "orange" parses on the live 1.3.31 build.
  • "orange" is the right constant, verified against orbit-ui-mobile origin/main: packages/shared/src/theme/color-schemes.ts:53-56 maps orange to #ff6900 and #f54900, the only warm orange of the six, and packages/shared/src/utils/upgrade.ts:161 returns 'purple' for null, so returning null would have painted purple everywhere.
  • No new test is a tautology. Each was checked by restoring the defect, not by reading its name. The three mutation-testing jobs are green.
  • Comments. Every changed .cs file was read by eye, since ORBIT0001 does not run under the locally installed SDK. The only // hits are three pre-existing #pragma warning disable trailers in the AgentCatalogService partials, on unchanged lines. Everything added is /// XML doc, which NoCommentsAnalyzer.cs:48-49 never inspects.
  • Authorization, error handling, naming, dead code. Unchanged caller-id plumbing, a real Result.Failure on an unknown key with nothing swallowed, descriptive names, nothing left behind.
  • CI. Every check is SUCCESS: Build, Unit Tests, OpenAPI Breaking-Change Gate, Gating matrix drift, arch-map drift, Dash Ban, Root Allowlist, CodeQL and SonarCloud at 93.8% coverage on new code.

Commands run

git -C <worktree> rev-parse HEAD                        -> d652a1fdc159afccf6a4e6c61c8bfe6480fb4779
dotnet build Orbit.slnx                                 -> 0 errors, 9 pre-existing NU1608/CS9057 warnings
dotnet test tests/Orbit.Domain.Tests                    -> 594 passed, 0 failed
dotnet test tests/Orbit.Application.Tests               -> 3424 passed, 0 failed
dotnet test tests/Orbit.Infrastructure.Tests            -> 2235 passed, 0 failed
dotnet test tests/Orbit.Analyzers.Tests                 -> 32 passed, 0 failed
dotnet test tests/Orbit.Application.Tests --filter ColorScheme -> 40 passed, 0 failed
red-first in a git archive copy: write mutant          -> domain 6/9 failed, application 14/40 failed
                                 export mutant         -> 6/19 failed
                                 read mutant           -> 8/40 failed
                                 chat payload, MCP msg -> 1 failure each, both the new tests
node tools/check-dashes.mjs --files / --check-baseline / --text "<PR title> <PR body>" -> exit 0
node tools/check-root-allowlist.mjs                     -> exit 0
node tools/arch-map.mjs in an isolated copy             -> architecture.json and .html md5 identical, no drift
git status --porcelain in the worktree, before and after -> empty

This pull request cannot merge before 2026-09-22 in any case, because it targets protected main where pullfrog-approval is required. A round for the P2 above is ordered on #367; everything else here is merge-ready.

The data catalog told Astra that every account reads back the same colour
scheme, while the export on the same field returns the raw column on purpose.
A row written before the collapse still holds its own key, so the two surfaces
answered one question two ways.

The catalog now states both halves: the profile reads back the granted value
and a pre-collapse row keeps its original key. The sweep it asked for found no
twin carrying the same claim, and a test over every catalog entry, capability,
surface, chat tool and MCP tool description now keeps it that way.

Three write-side sentences said "store the user's colour scheme preference"
while the write stores the granted value. They now say the value is accepted
and the stored one becomes the granted one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

1 similar comment
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

thomasluizon and others added 12 commits September 24, 2026 13:14
* fix: let the executor own the MCP confirmation gate (#599)

Every MCP tool whose capability requires a confirmation or a step-up was
refused forever. The selective-auth middleware evaluated the policy a second
time, in front of the executor, and it never received the caller's
confirmation token. A client that stepped up and retried with a valid token
got the identical refusal, because the token never reached that evaluation.

Carrying the token into the middleware cannot fix it. A confirmation token is
single use and is bound to the pending operation's fingerprint, and the
middleware computes a different fingerprint from the raw MCP arguments than
the executor computes from the operation id and its snake_case argument
object. One token cannot satisfy two gates.

So the middleware now steps aside for a confirmation-gated capability, the
same way it already steps aside for execute_agent_operation_v2. Both reach
IAgentOperationExecutor, which evaluates access and confirmation together,
holds the token, and writes the audit row.

Two guard tests pin the invariants the deferral rests on: a confirmation
requirement always sits on a mutation, and every confirmation-gated MCP tool
reaches the executor through McpExecutorBridge.

Also corrected: the step-up message and the three tool parameter descriptions
named verify_step_up_agent_operation_v2 as the source of the confirmation
token. It does not return one. confirm_agent_operation_v2 does.

Removed AgentPolicyEvaluationContext.StepUpSatisfied. Nothing ever set it and
nothing ever read it; the real step-up state lives on
PendingAgentOperationState.StepUpSatisfiedAtUtc.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: thread the confirmation token through the bulk habit tools (#599)

bulk_log_habits and bulk_skip_habits declared no confirmationToken
parameter and their shared helper hardcoded confirmationToken: null, so
their HabitsBulkWrite capability (FreshConfirmation) could never see a
token and both tools stayed permanently refused. Both now declare the
parameter and ExecuteBulkHabitOperationAsync forwards it.

Three new guards in ConfirmationGatedMcpToolsRouteThroughExecutorTests
pin what the old one missed. The scan now asserts it reaches exactly the
catalog's gated MCP tools, that each tool's forwarded operation id
resolves to a capability with the same ConfirmationRequirement, and that
each tool accepts and forwards a confirmation token. The source scan
walks subfolders and keys members by their declaration rather than by the
first invocation-shaped token in the chunk.

AgentOperationExecutor writes an AgentAuditLogs row before returning
UnknownOperation, restoring the trail the middleware used to leave.

McpConfirmationGateTests now drives the real AgentTools recovery methods
end to end and pins that all three refuse an API-key credential.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: resolve the forwarded MCP confirmation token back to the tool parameter (#599)

The token guard whitelisted one spelling of null. It flagged a bridge call only
when the confirmation-token argument was absent or the literal `null`, so
`confirmationToken: default`, `null!`, `(string?)null` and `""` all passed while
the tool stayed refused forever. A longer literal list does not close that,
because the next spelling is not on the list either.

Assert the structural property instead: a gated tool declares a
`string? confirmationToken` parameter, and the expression it forwards into the
executor's token slot resolves back to that identifier, directly or through one
helper hop. Every other expression fails, whatever it spells.

The declaration check now reads the tool's parameter list rather than its whole
body, so a local named `confirmationToken` no longer satisfies it.

Also add `forwarded.Id == tool.Capability.Id` to the operation-id guard. It
compared only the confirmation requirement, so a gated tool could forward
another gated capability's operation id, keep confirmation firing, and have the
executor enforce the wrong scope.

The file's doc comment said four invariants and listed four; the file holds
five. Name the source-scan invariant the other three rest on.

Six mutations, each red on its own, real tree green at every step:
`confirmationToken: default`, `null!`, `(string?)null`, `""`, a gated tool
forwarding a different local, and `delete_tag` forwarding `delete_goal`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: strip only a real named-argument prefix in the MCP guard parser (#599)

ArgumentAt treated any colon as a named-argument separator, so a conditional
expression in the token slot or the operation-id slot collapsed to its
else-branch. `ids.Count > 0 ? null : confirmationToken` read as
`confirmationToken` and `tagId.Length > 0 ? "delete_goal" : "delete_tag"` read
as `"delete_tag"`, and both guards stayed green over the restored defect.

Match `^\w+\s*:(?!:)` instead, so only a leading `name:` prefix is stripped and
every other expression reaches the resolver whole. The anchor keeps a colon
inside a string literal and a `::` qualifier from matching.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: separate an omitted caller argument from a non-parameter expression (#599)

ResolveExpression returned the expression itself both when it was not a callee
parameter and when the caller supplied nothing at that index. The second case
handed back the helper's own parameter name, which then compared equal to
`confirmationToken` and passed. Giving the helper an optional
`string? confirmationToken = null` and calling it without that argument restored
the #599 defect with the guard green.

Return the `<omitted>` sentinel for the second case instead. It can never equal
the parameter name, and it resolves to no capability in the operation-id slot,
so both guards fail closed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: read a gated tool's direct and helper bridge calls together (#599)

CollectBridgeCalls returned early as soon as the tool body held one bridge
call, so a correct call in a dead branch hid every helper call in the same
tool. A decoy `if (tagId.Length == 0)` block forwarding `confirmationToken`
plus a same-file helper forwarding `null` restored the #599 defect with the
guard green.

Collect both sets instead. Every bridge call a gated tool can reach, directly
or through one helper hop, now has to forward the parameter.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: refuse a gated tool that assigns to its own confirmationToken (#599)

The rule compared the forwarded expression's text only, so
`confirmationToken = null;` above the bridge call restored the #599 defect with
the forwarded identifier unchanged and the guard green.

Add an offender when the tool body, or a same-file helper it reaches, assigns
to the parameter. MemberSource now carries its statements with the parameter
list excluded, so the declaration's own `string? confirmationToken = null`
default does not match. The pattern `\bconfirmationToken\s*=[^=>]` leaves `==`,
`!=`, `>=`, `<=`, `??=` and a lambda arrow alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: say what the MCP token resolver models, not more (#599)

Invariant 5 claimed "any other expression in the token slot refuses the tool
forever" and that the tool forwards the identifier "directly or through a
helper it calls". A conditional refuses the tool on some paths only, and the
resolver models exactly one same-file helper hop matched by argument position.

Replace both sentences with what the scan actually does, and state its limits:
two or more hops read as no bridge call and fail the routing guard, and
aliasing, reflection and an interface call are outside the model.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: catch a compound assignment to confirmationToken (#599)

The assignment rule was anchored on `\s*=`, and `\s*` cannot cross the `??`
of `??=` or the `+` of `+=`. So `confirmationToken ??= tagId;` above the
bridge call compiled green with all five guards green, and a later change
that writes `confirmationToken ??= await ResolveStoredTokenAsync(...)` would
hand the executor a token the MCP caller never sent: HasFreshConfirmation is
then satisfied from state the caller does not control, while the middleware
has already stepped aside.

Admit the two compound operators a `string?` can carry. `==` still fails on
the second `=`, `!=`, `>=` and `<=` still fail because those characters break
`\s*` and are not in the alternation, `=>` still fails on the trailing
character class, and a bare `??` with no `=` still fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: read every same-named helper an MCP tool could be calling (#599)

The member index was a dictionary keyed on the bare name and built with
group.First(), so two same-named members collapsed to whichever was declared
first. The scan then analysed a method the tool never calls. It broke both
ways: a decoy overload declared after the real helper and forwarding null was
green, and a gated tool calling a correct overload reddened when an unrelated
same-named member happened to be declared first.

Hold every member under its name and, at each call site, read every candidate
whose parameter count can admit that many arguments. One candidate resolves
the call; more than one is ambiguous, so all of them are read and any bad one
reddens the tool. That fails closed instead of guessing, and it costs no false
red in the case above, where only the real helper carries a bridge call. A
name that admits no candidate stays unresolved, which hides nothing: the
bridge call inside it is not collected either, so the routing guard reddens
the tool.

Arity, not the parameter type, is what decides a candidate. The scan is a text
scan, so it cannot type-check an argument.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: say what invariant 5 now refuses and what it still cannot see (#599)

The general claim "refuses a tool that assigns to the parameter" is now exact:
`=`, `??=` and `+=`. The helper hop no longer claims a "matching argument
position", because the resolver pairs by position alone and strips a
named-argument prefix without reading the name, so arguments named out of the
declared order are read wrong. The comment said nothing about overloads, so
say that an ambiguous name is read as every candidate and reddens the tool.

Close with the honest limit: a text scan is defeatable by an author who sets
out to defeat it, and what the guard closes is every shape an ordinary
refactor produces.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Start calendar timezone fix

* Fix calendar event timezone projection

* Fix projected calendar end times

* Fix calendar end time during DST fallback

* Fix calendar end time minute precision

* Fix projected calendar recurrence rule

* Guard calendar recurrence across timezone shifts

* Project calendar recurrence weekdays

* Keep calendar recurrence unchanged

* Omit unrepresentable recurring calendar events

* Gate projected calendar recurrence on the whole fetch window

Closes the six review findings on pull request 521.

1. The weekday gate now walks every expanded occurrence Google returned for a
   recurring master, not the one sampled instance. The fetcher collects each
   instance start with the source calendar's own offset into a JsonIgnore'd
   ExpandedOccurrences list, so a Lisbon BYDAY=TH series that is stable in
   January and shifts to Wednesday in July is refused. The empty list still
   falls back to the sampled instance for an all-day series and for a
   suggestion row read back from the database.
2. The legacy title plus date plus time key only excludes a suggestion when
   exactly one candidate carries it. Inside a fall-back repeated hour two
   events project to the same local time, so the key proves nothing and both
   stay. This mirrors the group.Count() == 1 guard the auto sync reconciler
   already applies to the same key.
3. An omitted end time now logs its reason at Debug with the event id and the
   user id. EndUtc already ships the real duration, so the client keeps it.
4. The refusal here and the clamp in HabitScheduleService.IsMonthlyMatch are
   reconciled in a doc comment: Orbit clamps a rule it owns and refuses a rule
   it imports and cannot re-express without inventing a weekday.
5. Both new call sites pass the logger and the user id to FindTimeZone, which
   also catches InvalidTimeZoneException so a corrupt zone stops escaping as a
   500. User.SetTimeZone trims at the boundary and rejects a blank id.
6. Asia/Kathmandu at plus 05:45 and Pacific/Chatham at plus 12:45 cover the
   sub hour offset gap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Prove calendar recurrence stability from both zones

Round 6 gated a BYDAY series on the expanded instances Google returned, and
GoogleCalendarApi only asks for sixty days, so a series whose transition falls
outside that window was admitted and a stored suggestion row never carried the
instances at all.

The gate now reads the source calendar's own timezone from the recurring master
the fetcher already fetches, and walks a year of dates at the occurrence's source
wall clock through both zones' rules. A series it cannot prove stable is withheld
from both feeds. StoredCalendarEventJson carries the source zone beside the stored
suggestion, so the suggestion feed judges a row on the same evidence the events
feed had.

Also in scope: the auto-sync reconciler projects a fetched event into the account
timezone before matching a legacy habit, the end-time omission logs whatever EndUtc
holds, and an all-day event no longer reports Google's exclusive end date as an
end instant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Keep calendar series a spring forward gap cannot move

A probe date whose source wall clock a spring forward gap removes is a date
the series does not fire on, not evidence the series moves. The walk ended
there with "unproved" and the gate withheld the whole series, so an account
reading a calendar kept in its own zone lost an hour wide band in thirteen
DST zones even though the projection is the identity.

HasUnrepresentableRecurrenceAfterProjection now answers false as soon as
TimeZoneInfo.HasSameRules holds, and the walk skips a wall clock its own zone
removes instead of ending. RFC 5545 section 3.3.5 does define the missing
case, but probing that normalized instant changes no decision for any of the
14,752 zone pairs that could distinguish it, and asserting an expansion
Google does not document would let one guessed date withhold a year.

Also pins what earlier rounds argued: auto-sync refusing to store or notify a
withheld series, JsonIgnore keeping SourceTimeZone off the response body, the
repeated hour needing both of its instants, the probe reaching a full year,
and an all day event logging no dropped end time. A stored SourceTimeZone key
holding a number now reads as no source zone rather than failing the request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Fix recurring calendar timezone proof and legacy refresh

* Avoid refreshing legacy calendar rows with duplicate event IDs

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Bumps FluentAssertions from 8.10.0 to 8.11.0
Bumps Google.Apis.AndroidPublisher.v3 from 1.75.0.4246 to 1.76.0.4277
Bumps Hangfire.AspNetCore from 1.8.24 to 1.8.25
Bumps Hangfire.Core from 1.8.24 to 1.8.25
Bumps Microsoft.AspNetCore.Authentication.JwtBearer from 10.0.11 to 10.0.12
Bumps Microsoft.AspNetCore.OpenApi from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.Design from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.InMemory from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.Relational from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.Sqlite from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.ApiDescription.Server from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Caching.Abstractions from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Caching.Memory from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Caching.StackExchangeRedis from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Http from 10.0.11 to 10.0.12
Bumps Microsoft.IdentityModel.JsonWebTokens from 8.22.0 to 8.23.0
Bumps Microsoft.NET.Test.Sdk from 18.9.0 to 18.10.1
Bumps OpenAI from 2.13.0 to 2.14.0
Bumps PostHog from 2.14.0 to 2.15.7
Bumps PostHog.AspNetCore from 2.9.0 to 2.9.7
Bumps Scalar.AspNetCore from 2.17.1 to 2.17.9
Bumps Sentry.AspNetCore from 6.9.0 to 6.11.1
Bumps Stripe.net from 52.3.0 to 52.4.2

---
updated-dependencies:
- dependency-name: Google.Apis.AndroidPublisher.v3
  dependency-version: 1.76.0.4277
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Hangfire.AspNetCore
  dependency-version: 1.8.25
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Hangfire.Core
  dependency-version: 1.8.25
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Hangfire.Core
  dependency-version: 1.8.25
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.AspNetCore.Authentication.JwtBearer
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.AspNetCore.OpenApi
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.Design
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.Relational
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.ApiDescription.Server
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Caching.Abstractions
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Caching.StackExchangeRedis
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Http
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.IdentityModel.JsonWebTokens
  dependency-version: 8.23.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: OpenAI
  dependency-version: 2.14.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: PostHog
  dependency-version: 2.15.7
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: PostHog.AspNetCore
  dependency-version: 2.9.7
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Scalar.AspNetCore
  dependency-version: 2.17.9
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Sentry.AspNetCore
  dependency-version: 6.11.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Stripe.net
  dependency-version: 52.4.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: FluentAssertions
  dependency-version: 8.11.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.InMemory
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Caching.Memory
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: FluentAssertions
  dependency-version: 8.11.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: FluentAssertions
  dependency-version: 8.11.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.InMemory
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.Sqlite
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Start #529 API key step up

* Require API key management step up

* chore: regenerate the architecture map

The step up change moved the API key endpoints' shape, so `architecture.json`
and `architecture.html` no longer matched the tree and the `drift` check
failed on `git diff --exit-code`. Regenerated with `node tools/arch-map.mjs`,
which reports 45 entities and 0 untested feature folders.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Give API key management one authorization concept with two doors

Listing, creating and revoking API keys now read one grant. Two doors open
it, and both end in a six-digit code emailed to the account owner: the HTTP
challenge, and a verified agent step-up.

Creating key material spends the grant. Listing and revoking only read it,
so one emailed code authorizes a management session instead of exactly one
revoke. A revoke no longer locks the person out of the key list.

get_api_keys gets a door. RequireApiKeyCreationStepUp raises the read
capability to a step-up inside the executor, so the read opens a pending
operation the caller can step up against, and the MCP read routes through
McpExecutorBridge like the mutation does.

AppConfigService now parses a stored row strictly and throws by name when a
row is malformed, so a typo cannot leave the gate off while the read-back
reports it on.

Both controller actions declare 403 and 428, and openapi.json carries them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Cover API key step up through merged MCP gate

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Open ticket 513 implementation review

* Expose habit emoji across MCP tools (#513)
* Start #659 workflow concurrency change

* Group core PR workflows by pull request for #659

* Group remaining PR workflows by pull request for #659
* Start #657 session family fix

* Revoke the full auth session after logout (#657)

* Explain temporary legacy token allowance (#657)

* Handle duplicate refresh history race (#657)
…lor-scheme

# Conflicts:
#	architecture.html
#	architecture.json
* Start #588 bulk interval weeks fix

* Fix #588 bulk habit interval weeks mapping
* Add crisis guidance to Astra static prompt for #319

* Add curated crisis detection and regression cases for #319

* Wire crisis guard across chat delivery and metrics for #319

* Refine crisis delivery and cover fallback regression for #319

* Return static crisis support when AI chat fails for #319

* Keep crisis FAQ regression isolated for #319

* Fix crisis replies to bypass AI quota and provider
* Start #571 freeze source work

* Record streak freeze origin for #571

* Require explicit origin for new streak freezes
@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@pullfrog pullfrog 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.

Important

The API-first rollout leaves a supported paid feature advertised but no longer honored; the blocking billing finding is inline.

Reviewed changes at c56677a9 cover the complete color-scheme collapse and its API, domain, agent, export, and test surfaces.

  • Collapsed behavior: all six historical write values remain accepted, but non-null writes store orange and profile reads always return orange.
  • Preserved contracts: the nullable string DTO shape remains intact, legacy raw values remain in data exports, and no migration rewrites stored rows.
  • Agent and test updates: the obsolete agent snapshot field was removed, tool and catalog descriptions were aligned, and domain, round-trip, export, profile, and catalog coverage was expanded.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using GPT Sol | 𝕏

Comment thread src/Orbit.Domain/Entities/User.cs
thomasluizon and others added 2 commits September 25, 2026 05:42
* Start #628 sub habit title validation fix

* Use sub habit title messages in standalone validation (#628)
)

A signed-out person who set a weekly repeat interval during onboarding lost
it at sign-up. ApplyHabitInput carried no interval field at all, so the apply
path dropped the value between the screen and the record.

Add an optional IntervalWeeks to ApplyHabitInput, additive only, and pass it
to Habit.Create alongside the other schedule options. No existing field
changes name, shape or nullability, so a client that sends nothing keeps the
behaviour it has today: IntervalWeeks stays null and the habit repeats every
week.

Validate the new field with the shared SharedHabitRules.AddIntervalWeeksRules
range, the same 1 to 52 bound every other create path uses.

Regenerate openapi.json and architecture.json/html for the new field and for
the test class that now touches HabitScheduleService.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@thomasluizon

Copy link
Copy Markdown
Owner Author

Thomas decided on 2026-09-25: ticket #367 is redesign work, so this PR targets orbit-api redesign/main, not main. The colour-scheme collapse ships with the redesign, never ahead of it. The next run retargets or rebuilds this PR on redesign/main.

thomasluizon and others added 6 commits September 25, 2026 08:09
* Start #591 recurrence time zone work

* Expose recurring calendar event time zone for #591

* Restore recurrence time zone in legacy suggestions for #591
* Start calendar BYDAY projection (#569)

* Project uniform calendar BYDAY recurrences (#569)

* Cover alternate week recurrence projection (#569)

* Keep shifted calendar rules importable by installed clients
* chore: start ticket 324 review

* fix: retry concurrent Google sign-in updates (#324)
* Start fix for ticket 671 confirmation race

* Claim agent confirmation once under concurrent saves

* chore: regenerate the architecture map for the confirmation claim migration (#671)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* chore: start ticket 391

* fix: expose latest habit completion date in profile

* fix: exclude bad habit slips from last completion date

* fix: pass the freeze origin in the profile freeze test after #540 (#391)

The merge with main brought StreakFreeze.Create's required origin
parameter (#540). The test now asserts that a freeze without a
completion leaves lastCompletionDate null for both origins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: preserve last completion across habit cleanup (#391)

* Preserve descendant completions during sync cleanup

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@thomasluizon
thomasluizon changed the base branch from main to redesign/main September 25, 2026 16:00
@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@pullfrog pullfrog 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.

Important

The latest merge disables Dependency Review and SonarCloud because both workflow files now contain duplicate top-level YAML keys.

Reviewed changes since the prior Pullfrog review at c56677a9 covered the two branch synchronization commits and the resulting full PR diff.

  • Synchronized branches: Merged current main and redesign/main into the feature branch without changing the color-scheme runtime or wire-contract behavior.
  • Reconciled generated state: Carried the updated architecture artifacts and test dependencies from the synchronized branches.
  • Duplicated workflow configuration: The redesign merge left two workflow files with repeated top-level concurrency mappings; the blocking consequence is documented inline.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using GPT Sol | 𝕏

Comment thread .github/workflows/dependency-review.yml Outdated

@pullfrog pullfrog 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.

Important

No additional findings were found, but the existing paid-feature contract and workflow-loading threads remain unresolved, so this head is not ready for approval.

Reviewed changes covered the complete current diff at f82ec430, including the color-scheme collapse, its tests and agent surfaces, the cross-repo client contract, and the branch synchronization changes.

  • Color-scheme behavior: All historical keys remain accepted, non-null writes and profile reads resolve to the granted accent, legacy stored values remain truthful in exports, and the wire shape stays unchanged.
  • Agent and test surfaces: The obsolete snapshot field is removed, tool and catalog text reflects the collapse, and domain, round-trip, profile, export, and catalog coverage exercises the changed behavior.
  • Branch synchronization: The latest merges add duplicate top-level workflow mappings; that high-severity issue was submitted by another Pullfrog run on this same head while this review was in progress, so it is not duplicated inline here.
  • Cross-repo contract: orbit-ui-mobile redesign/main retains the nullable string contract and maps all six historical keys to the granted accent; installed clients still preserve the paid picker behavior covered by the earlier open thread.

Pullfrog  | Fix it ➔ | View workflow run | Using GPT Sol | 𝕏

…reated

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes since the prior Pullfrog review at f82ec430 covered the workflow repair in b083c0a7.

  • Restored workflow loading — Removed the duplicate top-level concurrency mappings from Dependency Review and SonarCloud, leaving one mapping in each workflow while retaining their redesign/main pull-request triggers.
  • Preserved PR behavior — Left the color-scheme runtime, append-only wire contracts, generated architecture artifacts, and test surface unchanged.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

…et-367-color-scheme

# Conflicts:
#	architecture.html
#	architecture.json
@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@sonarqubecloud

Copy link
Copy Markdown

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes at b792e03d cover the complete current diff, the color-scheme contract across both repositories, and the latest merge-forward.

  • Collapsed behavior: All six historical keys remain accepted, non-null writes and profile reads resolve to the granted accent, and null writes still clear the stored preference.
  • Preserved contracts and data: The nullable wire shape and endpoint remain unchanged, while exports continue to report legacy stored values without a migration rewriting existing rows.
  • Aligned agent surfaces: The obsolete snapshot field is removed, and chat, MCP, capability, and catalog text now describes the collapsed behavior consistently.
  • Cross-repo compatibility: orbit-ui-mobile redesign/main preserves the string contract, maps every historical key to the granted accent, and removes the color-scheme preference surface.
  • Verification: The focused domain, application, and infrastructure test sets passed with 123 tests and no failures; the worktree remained clean after OpenAPI generation.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@thomasluizon
thomasluizon merged commit 430e760 into redesign/main Sep 25, 2026
23 checks passed
@thomasluizon
thomasluizon deleted the feature/ticket-367-color-scheme branch September 25, 2026 17:22
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