Menu

#342 fix(integrations): address deferred review findings from #338

closed
nobody
None
2026-06-25
2026-06-25
Anonymous
No

Originally created by: Akarsh-Hegde

Summary

Seven architectural findings from the PR [#338] review that were deferred to a follow-up:

  • login() explicit secret param — removes std::env::set_var from the Tokio worker thread (forward_oauth_env deleted). POSIX setenv is not async-signal-safe and Rust 1.93+ warns on it. Credentials are now read from .env and passed directly to meridian_oauth login functions.
  • ensure_fresh() graceful degradation — if store::save fails after Atlassian rotates the refresh token, the old code hard-failed, permanently locking out the user. Now logs a CRITICAL error and returns the in-memory access token (valid ~1h), giving the user a window to fix disk permissions before re-authing.
  • FIXME(cross-process-lock) comment on refresh_lock() in jira.rs — documents the daemon/tray race where independent per-process mutexes don't prevent a double-refresh race across processes. Tracked for a future fd-lock-based fix.
  • parse_env → dotenvy — the hand-rolled parser diverged from the daemon's dotenvy::dotenv_override() on edge cases (export prefix, quoted values, backslash continuation). Now uses dotenvy::from_path_iter so both processes agree on credential values.
  • Early secret/key validationstart_oauth_in_process now validates client_secret (jira) and app_key (trello) before spawning. Returns a clear error immediately rather than letting the user wait through the browser callback for a flow that was never going to succeed.
  • Per-provider AtomicBool in-flight guardJIRA_OAUTH_IN_FLIGHT / TRELLO_OAUTH_IN_FLIGHT prevent two concurrent flows from racing to bind the same loopback port or write the same token file. The guard is always cleared (success or failure) before the spawned task exits.
  • spawn_blocking for upsert_envupsert_env does synchronous std::fs I/O and was being called directly in an async fn. Wrapped in tokio::task::spawn_blocking.

Test plan

  • [ ] cargo clippy -- -D warnings passes on meridian and meridian-tray (verified in pre-commit hook)
  • [ ] cargo fmt --check passes (verified in pre-commit hook)
  • [ ] cargo test on the main workspace passes
  • [ ] Manual: Jira OAuth connect flow works end-to-end in a dev build
  • [ ] Manual: Double-clicking "Connect Jira" while flow is in progress returns the in-flight error message
  • [ ] Manual: Source build with empty JIRA_OAUTH_CLIENT_SECRET shows the "not configured" error immediately

🤖 Generated with Claude Code

Related

Tickets: #338
Tickets: #347
Tickets: #351

Discussion

  • Anonymous

    Anonymous - 2026-06-25

    Originally posted by: coderabbitai[bot]

    [!IMPORTANT]

    Review skipped

    Auto reviews are disabled on base/target branches other than the default branch.

    Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.


    ⚙️ Run configuration

    Configuration used: Organization UI

    Review profile: ASSERTIVE

    Plan: Pro Plus

    Run ID: 23644ae4-70ec-48f0-92f4-8f3fc6e1105b

    You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

    Use the checkbox below for a quick retry:
    - [ ] 🔍 Trigger review

    ✨ Finishing Touches
    🧪 Generate unit tests (beta) - [ ] Create PR with unit tests - [ ] Commit unit tests in branch `fix/pr-338-review-followup`

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

     
  • Anonymous

    Anonymous - 2026-06-25

    Ticket changed by: Akarsh-Hegde

    • status: open --> closed
     

Log in to post a comment.