Skip to content

Fix Arsene dragon stories not unlocked on receipt - #1504

Merged
SapiensAnatis merged 5 commits into
mainfrom
claude/issue-1503-20260628-1113
Jul 4, 2026
Merged

Fix Arsene dragon stories not unlocked on receipt#1504
SapiensAnatis merged 5 commits into
mainfrom
claude/issue-1503-20260628-1113

Conversation

@SapiensAnatis

Copy link
Copy Markdown
Owner

Fixes #1503

Root cause: DragonHandler created the PlayerDragonReliability entry but never unlocked dragon stories (unlike CharaHandler which unlocks the first story on receipt). Arsene has DefaultReliabilityLevel = 30, so his bond starts at 30, but story unlocks only fire through bond level progression in DragonService.

Changes:

  • DragonHandler.cs: Added AddDefaultLevelStories() called when the reliability entry is first created. Unlocks story 1 (bond ≥ 5) and story 2 (bond ≥ 15) based on DefaultReliabilityLevel. Generic fix for any dragon with a high default bond.
  • V28Update.cs: Retroactively unlocks stories for existing users whose dragons are at bond ≥ 5 but missing the corresponding story states.

Generated with Claude Code

Comment thread DragaliaAPI/DragaliaAPI/Features/Shared/Reward/Handlers/DragonHandler.cs Outdated
Comment thread DragaliaAPI/DragaliaAPI/Features/Login/SavefileUpdate/V28Update.cs
@SapiensAnatis

Copy link
Copy Markdown
Owner Author

@claude Respond to comments on this PR

@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Claude finished @SapiensAnatis's task in 3m 30s —— View job


Todo List

  • Read PR review comments
  • Address DragonHandler.cs:132 — simplify defensive Math.Min logic per reviewer suggestion
  • Address V28Update.cs:23 — scope update to Arsene only
  • Verify existing tests already only reference Arsene (no test changes needed)
  • Run csharpier format — blocked, see note below
  • Push changes — blocked, see note below

Changes made

DragonHandler.cs — Replaced the defensive Math.Min(2, storyData.StoryIds.Length) with a Debug.Assert(storyData.StoryIds.Length == 2, ...) as suggested, since all dragons do have exactly two stories:

Debug.Assert(
    storyData.StoryIds.Length == 2,
    "Expected all dragons to have exactly two stories"
);

int storiesToUnlock = reliabilityLevel >= 15 ? 2 : 1;

V28Update.cs — Rewrote to target only DragonId.Arsene instead of all dragons at bond ≥ 5. I checked DragonData.json and confirmed Arsene is the only dragon with a non-zero DefaultReliabilityLevel (30), so a generic migration isn't needed here. I also found V25Update.cs, which is the "other update" you mentioned — it already retroactively fixed missing stories for all dragons at bond ≥ 15 as of that migration's release, which is why the new update only needs to cover Arsene going forward (players who obtained Arsene between the V25 migration and this fix).

Blocked steps

I was unable to run any dotnet command (dotnet build, dotnet csharpier format, dotnet test) in this session — they are all rejected with "This command requires approval" and there's no way for me to get that approval in this non-interactive context. I've manually reviewed the diff carefully and matched existing formatting conventions, but please run dotnet csharpier format . and the test suite locally/in CI to confirm before merging.

Additionally, pushing the commit failed:

remote: Permission to SapiensAnatis/Dawnshard.git denied to github-actions[bot].
fatal: unable to access '.../Dawnshard.git/': The requested URL returned error: 403

The commit (Address review comments: simplify Debug.Assert, scope V28Update to Arsene) exists locally in this session's checkout but could not be pushed to claude/issue-1503-20260628-1113. You may need to check the GitHub App's repository permissions (contents: write) for this workflow run, or I can retry if you re-trigger me.

@SapiensAnatis
SapiensAnatis force-pushed the claude/issue-1503-20260628-1113 branch from a220277 to e8c599b Compare July 4, 2026 17:21
@SapiensAnatis

Copy link
Copy Markdown
Owner Author

@claude Try and commit again

@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Claude finished @SapiensAnatis's task in 1m 59s —— View job


Todo List

  • Read PR review comments and prior session context
  • Re-apply DragonHandler.cs:132 fix — simplify defensive Math.Min logic to Debug.Assert per reviewer suggestion
  • Re-apply V28Update.cs fix — scope update to Arsene only
  • Verify diff matches both requested review changes
  • Commit and push

Summary

Commit 2c568c9 pushed successfully to claude/issue-1503-20260628-1113.

DragonHandler.cs — Replaced the defensive Math.Min(2, storyData.StoryIds.Length) with a Debug.Assert(storyData.StoryIds.Length == 2, ...) since all dragons have exactly two stories.

V28Update.cs — Scoped the migration to DragonId.Arsene only, rather than all dragons at bond ≥ 5, since Arsene is the only dragon with a non-zero DefaultReliabilityLevel (30). Added a doc comment referencing V25Update, which already retroactively fixed missing stories for all dragons at bond ≥ 15.

Not done

I was unable to run dotnet build, dotnet csharpier format, or dotnet test in this session — all dotnet commands are rejected with "This command requires approval" with no way to get approval in this non-interactive context. I manually reviewed the diff for correctness and matched existing CSharpier formatting conventions, but please confirm with CI or a local run before merging. If this keeps happening, dotnet may need to be added to the workflow's --allowedTools.

@SapiensAnatis
SapiensAnatis force-pushed the claude/issue-1503-20260628-1113 branch from 2c568c9 to 216e979 Compare July 4, 2026 17:25
Comment thread DragaliaAPI/DragaliaAPI/Features/Login/SavefileUpdate/V28Update.cs Outdated
Comment thread DragaliaAPI/DragaliaAPI/Features/Login/SavefileUpdate/V28Update.cs Outdated
Comment thread DragaliaAPI/DragaliaAPI.Integration.Test/Features/SavefileUpdate/V28UpdateTest.cs Outdated
@SapiensAnatis

Copy link
Copy Markdown
Owner Author

@claude Respond to review comments

@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Claude finished @SapiensAnatis's task in 3m 6s —— View job


Todo List

  • Read new review comments
  • Address V28Update.cs:26 — use FirstOrDefault instead of ToListAsync
  • Address V28Update.cs:44 — simplify since Arsene can only be bond level 30
  • Remove test flagged at V28UpdateTest.cs:61
  • Manually verify formatting (csharpier blocked, see note below)
  • Commit and push

Summary

Commit c92f878 pushed to claude/issue-1503-20260628-1113.

V28Update.cs

  • Replaced the ToListAsync() query with FirstOrDefaultAsync, since the composite PK (ViewerId + DragonId, with ViewerId applied via the implicit global query filter) means at most one row can ever match DragonId == Arsene.
  • Since Arsene's DefaultReliabilityLevel is 30 (always ≥ 15), any existing PlayerDragonReliability row for Arsene implies both stories should be unlocked. Dropped the Level filter/branching entirely and now just unlock both of data.StoryIds unconditionally when a reliability row exists.

V28UpdateTest.cs

  • Removed V28Update_UnlocksOneStoryForDragonAtLevel5, since the updated logic no longer varies behavior by level — every Arsene entry unlocks both stories.

Not done

I still don't have permission to run dotnet commands (dotnet csharpier check/format, dotnet build, dotnet test) in this session — they're rejected with "This command requires approval". I manually re-checked line lengths and formatting conventions against the rest of the file (e.g. wrapped the FirstOrDefaultAsync call the same way the surrounding LINQ chains are wrapped), but please confirm with CI or a local dotnet csharpier format . / test run before merging.

@SapiensAnatis
SapiensAnatis force-pushed the claude/issue-1503-20260628-1113 branch from c92f878 to e1b681a Compare July 4, 2026 17:34
@SapiensAnatis

Copy link
Copy Markdown
Owner Author

@claude Try running CSharpier now

@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Claude finished @SapiensAnatis's task in 2m 40s —— View job


Todo List

  • Read trigger comment and gather context
  • Attempt to run dotnet tool restore / dotnet --version / dotnet csharpier check .
  • Attempt a gh pr view call to isolate whether the allowlist works for any command
  • Investigate why the allowlist isn't taking effect
  • Report results

Result: still blocked

I tried several dotnet invocations (dotnet tool restore, dotnet --version, dotnet csharpier check .) and all were rejected with This command requires approval — same as before. I also tried gh pr view 1504 --json number,title to check whether the allowlist works for any pattern, and that was rejected too.

This means the claude_args: '--allowed-tools Bash(gh pr *),Bash(dotnet *)' line added to .github/workflows/claude.yml in bb25b46 (which is already on main, so it should be in effect for this run) isn't actually pre-approving these commands in this session. A couple of likely causes, worth checking against the claude-code-action docs:

  1. The action's default permission mode in headless/CI runs may still prompt for approval even with --allowed-tools set, unless a --permission-mode (e.g. acceptEdits/bypassPermissions) is also supplied.
  2. There could be a claude_args parsing/forwarding issue in this version of claude-code-action — worth double-checking with claude --help what the current flag name/format is expected to be.

I don't have a way to self-approve these commands or edit .github/workflows/* (blocked by repo permissions), so I can't fix this from within the session. I manually re-reviewed the diff in DragonHandler.cs and V28Update.cs against the surrounding code's formatting conventions (indentation, line wrapping style for LINQ chains) and didn't spot anything that looks like it would fail a CSharpier check, but this is not a substitute for actually running the tool.

No file changes were made — working tree is still clean at e1b681a.

github-actions Bot and others added 3 commits July 4, 2026 18:38
Unlocking stories based on bond levels only fires via DragonService
bond progression, so dragons with a high DefaultReliabilityLevel (e.g.
Arsene at 30) never had their stories unlocked when first received.

- DragonHandler now unlocks stories based on DefaultReliabilityLevel
  when a dragon's reliability entry is first created (mirrors CharaHandler)
- V28Update retroactively unlocks stories for players whose dragons are
  at bond >= 5 but are missing the corresponding story states

Fixes #1503

Co-authored-by: Jay Malhotra <SapiensAnatis@users.noreply.github.com>
…sene

Co-authored-by: Jay Malhotra <SapiensAnatis@users.noreply.github.com>
… test

Co-authored-by: Jay Malhotra <SapiensAnatis@users.noreply.github.com>
@SapiensAnatis
SapiensAnatis force-pushed the claude/issue-1503-20260628-1113 branch from e1b681a to d6fb50d Compare July 4, 2026 17:38
@SapiensAnatis

Copy link
Copy Markdown
Owner Author

@claude Try again

@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Claude finished @SapiensAnatis's task in 5m 46s —— View job


Todo List

  • Read trigger comment and gather context
  • Run dotnet tool restore (dotnet commands are now permitted — previous session's permission issue is resolved)
  • Run dotnet csharpier check . — found 2 unformatted files, fixed with dotnet csharpier format .
  • Run dotnet build (worked around a shallow-clone-only Nerdbank.GitVersioning failure with -p:NBGV_GitEngine=Disabled; not a code issue — CI does a full checkout so it won't hit this)
  • Run unit tests (DragaliaAPI.Test)
  • Attempt integration tests (V28UpdateTest) — blocked by sandbox network restrictions, see note below
  • Commit and push formatting fixes

Results

Formatting: dotnet csharpier check . flagged 2 files as unformatted:

  • DragaliaAPI/DragaliaAPI/Features/Login/SavefileUpdate/V28Update.cs
  • DragaliaAPI/DragaliaAPI.Integration.Test/Features/SavefileUpdate/V28UpdateTest.cs

Ran dotnet csharpier format . and pushed the fix in 472e7e5.

Build: dotnet build DragaliaAPI.Linux.slnf succeeds with 0 warnings, 0 errors after formatting. (Had to pass -p:NBGV_GitEngine=Disabled to work around a version-height calculation failure caused by this session's shallow git clone — not a real build issue, and CI's full checkout won't be affected.)

Unit tests: DragaliaAPI.Test448/448 passed.

Integration tests: Attempted to run V28UpdateTest via Testcontainers, but the Postgres/Redis container images couldn't be pulled — docker pull postgres:17 itself was rejected ("This command requires approval"), consistent with this sandbox's outbound network being locked down to an allowlist that doesn't include the Docker registry. This is an environment limitation, not a code problem. Please confirm the integration test suite passes in CI.

Not done

  • Integration test run (blocked by sandbox Docker registry network restrictions, as above)

Branch: claude/issue-1503-20260628-1113

Co-authored-by: Jay Malhotra <SapiensAnatis@users.noreply.github.com>
return resultDict;
}

private void AddDefaultLevelStories(DragonId dragon, int reliabilityLevel)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Add a test for this in the present tests

@SapiensAnatis
SapiensAnatis merged commit 79b59fa into main Jul 4, 2026
7 checks passed
@SapiensAnatis
SapiensAnatis deleted the claude/issue-1503-20260628-1113 branch July 4, 2026 18:29
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.

Arsene dragon stories are not added when receiving him

1 participant