Skip to content

fix(creds): WGER sentinel instead of unusable random token - #874

Merged
POWERFULMOVES merged 2 commits into
mainfrom
fix/wger-credential-sentinel
Mar 11, 2026
Merged

POWERFULMOVES merged 2 commits into
mainfrom
fix/wger-credential-sentinel

Conversation

@POWERFULMOVES

@POWERFULMOVES POWERFULMOVES commented Mar 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • WGER API token set to GENERATE_FROM_WGER_UI sentinel (Django REST Framework rejects random tokens)
  • Added to PLACEHOLDER_VALUES so future runs don't skip it
  • OSError handling in main() for env file write failures
  • Updated SEEDED_BRANDED_DEFAULTS.md with Django admin instructions

Addresses CodeRabbit findings #2, #6 from PR #864.

Test plan

  • python -m py_compile pmoves/tools/brand_defaults.py
  • upsert_env() produces WGER_API_TOKEN=GENERATE_FROM_WGER_UI

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@POWERFULMOVES has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 45 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b0212194-4e15-4e88-9edf-d7cbc27208af

📥 Commits

Reviewing files that changed from the base of the PR and between 12dab3d and cd98699.

📒 Files selected for processing (2)
  • pmoves/docs/operations/SEEDED_BRANDED_DEFAULTS.md
  • pmoves/tools/brand_defaults.py
📝 Walkthrough

Walkthrough

The changes update the WGER API token generation workflow from auto-generated tokens with a pm_wger_ prefix to a sentinel-based approach requiring manual UI generation via Django admin. Documentation is updated to reflect the new workflow, and error handling is strengthened with try/except protection around environment configuration calls.

Changes

Cohort / File(s) Summary
Documentation Update
pmoves/docs/operations/SEEDED_BRANDED_DEFAULTS.md
Updated guidance on WGER_API_TOKEN workflow, replacing auto-generated token references with GENERATE_FROM_WGER_UI sentinel value and clarifying manual token creation via Django admin.
Token Generation Logic
pmoves/tools/brand_defaults.py
Expanded PLACEHOLDER_VALUES to include GENERATE_FROM_WGER_UI, modified token handling to require UI-based generation, and added try/except error handling for upsert_env calls with stderr output and exit code 1 on failure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A sentinel value hops into place,
No more random tokens in this space,
Django admin's UI now leads the way,
Error handling guards each day,
Credentials flow through proper gates! 🎫

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete. It lacks the required Testing section with commands/output and only partially addresses the Required Checks section. Add a Testing section documenting the actual test commands executed and their output. Complete the Required Checks checklist by marking applicable items as completed or not applicable.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: replacing a random token with a sentinel value for WGER credentials.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/wger-credential-sentinel

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
pmoves/tools/brand_defaults.py (1)

137-141: Teach the remaining WGER consumers about this sentinel.

pmoves/scripts/integration-auth-setup.sh:145-160 and pmoves/integrations/health-wger/n8n/flows/health_wger_sync.json:28-35 still treat any non-empty WGER_API_TOKEN as usable, so freshly seeded envs will continue sending Token GENERATE_FROM_WGER_UI and only surface generic auth failures. Now that this sentinel is the canonical default, it would be worth updating those paths in the same change so operators get consistent guidance everywhere.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/tools/brand_defaults.py` around lines 137 - 141, The seeded default
WGER_API_TOKEN value "GENERATE_FROM_WGER_UI" is a sentinel and must be treated
as non-usable; update the integration-auth-setup script and the health_wger_sync
n8n flow so they check for WGER_API_TOKEN == "GENERATE_FROM_WGER_UI" (not just
non-empty) and behave like a missing token: do not export or attempt to use it,
surface a clear message instructing operators to create a token from the WGER
admin UI and paste it into WGER_API_TOKEN, and in the n8n flow short-circuit
auth steps when the value equals the sentinel to return a helpful
error/notification instead of attempting requests with "Token
GENERATE_FROM_WGER_UI".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/tools/brand_defaults.py`:
- Around line 207-211: The code currently calls mkdir() and write_text() before
the try that wraps upsert_env, so filesystem errors escape; move the
mkdir()/write_text() calls into the same try block that calls upsert_env (or
wrap the entire sequence that prepares and writes the env file — the mkdir(),
write_text(), and upsert_env calls involving variables env_path, env_gen_path,
and DEFAULTS) and keep the existing except OSError as e: print(...) and return 1
so any permission/disk failures are handled consistently.

---

Nitpick comments:
In `@pmoves/tools/brand_defaults.py`:
- Around line 137-141: The seeded default WGER_API_TOKEN value
"GENERATE_FROM_WGER_UI" is a sentinel and must be treated as non-usable; update
the integration-auth-setup script and the health_wger_sync n8n flow so they
check for WGER_API_TOKEN == "GENERATE_FROM_WGER_UI" (not just non-empty) and
behave like a missing token: do not export or attempt to use it, surface a clear
message instructing operators to create a token from the WGER admin UI and paste
it into WGER_API_TOKEN, and in the n8n flow short-circuit auth steps when the
value equals the sentinel to return a helpful error/notification instead of
attempting requests with "Token GENERATE_FROM_WGER_UI".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4f424381-7ddc-4e87-9a45-3648533ae689

📥 Commits

Reviewing files that changed from the base of the PR and between c81b243 and 12dab3d.

📒 Files selected for processing (2)
  • pmoves/docs/operations/SEEDED_BRANDED_DEFAULTS.md
  • pmoves/tools/brand_defaults.py

Comment thread pmoves/tools/brand_defaults.py
hunnibear and others added 2 commits March 11, 2026 18:11
Django REST Framework tokens must be created via the admin UI — random
tokens are rejected. Replace auto-generated pm_wger_ token with
GENERATE_FROM_WGER_UI sentinel. Add sentinel to PLACEHOLDER_VALUES so
future runs treat it as needing replacement. Wrap main() env file I/O
in try/except OSError for graceful failure.

Addresses CodeRabbit findings #2, #6 from PR #864.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace auto-generation claim with step-by-step Django admin instructions
for creating the WGER API token manually.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@POWERFULMOVES
POWERFULMOVES force-pushed the fix/wger-credential-sentinel branch from 12dab3d to cd98699 Compare March 11, 2026 23:11
@POWERFULMOVES
POWERFULMOVES merged commit 41b1110 into main Mar 11, 2026
6 checks passed
@POWERFULMOVES
POWERFULMOVES deleted the fix/wger-credential-sentinel branch March 12, 2026 00:01
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.

2 participants