Skip to content

fix: CLI commands ignore runtime DATABASE_BACKEND when both features compiled#209

Closed
timetwisterfounder-tech wants to merge 2 commits intonearai:mainfrom
timetwisterfounder-tech:fix/cli-runtime-db-backend-selection
Closed

fix: CLI commands ignore runtime DATABASE_BACKEND when both features compiled#209
timetwisterfounder-tech wants to merge 2 commits intonearai:mainfrom
timetwisterfounder-tech:fix/cli-runtime-db-backend-selection

Conversation

@timetwisterfounder-tech
Copy link
Copy Markdown

Summary

  • ironclaw tool auth and ironclaw mcp CLI subcommands used compile-time #[cfg(feature = "postgres")] / #[cfg(all(feature = "libsql", not(feature = "postgres")))] gates to initialize the secrets store. When the binary is compiled with both postgres and libsql features (the default for cargo install --all-features), the postgres block always wins regardless of the runtime DATABASE_BACKEND setting
  • This causes ironclaw tool auth <tool> to fail with Connection pool error: Config: configuration property "url" is invalid: invalid connection string for all libsql users, making it impossible to configure tool credentials via the CLI
  • Both functions now use match config.database.backend { ... } with inner #[cfg] guards on each arm, matching the pattern already used in main.rs (line 385) and app.rs
  • Also adds a top-level auth section to the Telegram channel capabilities file so ironclaw tool auth telegram --dir ~/.ironclaw/channels/ works (channels were missing this, only tools had it)

Root cause

src/cli/tool.rs and src/cli/mcp.rs both had:

// Always wins when postgres feature is compiled in
#[cfg(feature = "postgres")]
{ /* postgres path */ }

// Dead code when both features are compiled
#[cfg(all(feature = "libsql", not(feature = "postgres")))]
{ /* libsql path */ }

Fixed to:

match config.database.backend {
    #[cfg(feature = "libsql")]
    DatabaseBackend::LibSql => { /* libsql path */ }
    #[cfg(feature = "postgres")]
    DatabaseBackend::Postgres => { /* postgres path */ }
    _ => bail!("No backend available"),
}

Verified

Tested end-to-end on a libsql-backed install:

  1. ironclaw tool auth telegram --dir ~/.ironclaw/channels/ — previously crashed with connection pool error, now succeeds
  2. ironclaw startup — Channel credentials injected channel=telegram credentials_injected=1
  3. Telegram polling — url_changed=true, status=200 (was url_changed=false, status=404)

Test plan

  • cargo build --all-features compiles cleanly
  • cargo build --no-default-features --features libsql compiles cleanly
  • cargo build (default postgres only) compiles cleanly
  • ironclaw tool auth <tool> --dir ~/.ironclaw/channels/ works with DATABASE_BACKEND=libsql
  • ironclaw mcp subcommands work with DATABASE_BACKEND=libsql

🤖 Generated with Claude Code

…compiled

`tool auth` and `mcp` CLI subcommands used compile-time `#[cfg]` gates
to select the database backend for secrets storage. When the binary is
compiled with both `postgres` and `libsql` features, the `#[cfg(feature
= "postgres")]` block always wins regardless of the runtime
`DATABASE_BACKEND` setting. This causes `tool auth` and `mcp auth` to
fail with a connection error for users running the libsql backend.

Switch both functions to `match config.database.backend { ... }` with
inner `#[cfg]` guards on each arm, matching the pattern already used in
`main.rs` and `app.rs`.

Also adds a top-level `auth` section to the Telegram channel
capabilities file so `ironclaw tool auth telegram` works for channels
(previously only tools had this section).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @timetwisterfounder-tech, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug affecting CLI commands, specifically ironclaw tool auth and ironclaw mcp, which failed to correctly identify and utilize the configured database backend at runtime when multiple database features were compiled. The core issue stemmed from an over-reliance on compile-time feature flags for database selection, leading to an incorrect backend being chosen. The changes refactor this logic to dynamically select the database based on the runtime configuration, ensuring that users can properly authenticate tools regardless of their chosen database backend. Additionally, it enhances Telegram channel capabilities to support CLI-based authentication.

Highlights

  • CLI Database Backend Selection Fix: Corrected an issue where ironclaw tool auth and ironclaw mcp CLI subcommands would incorrectly prioritize the Postgres database backend when both postgres and libsql features were compiled, ignoring the runtime DATABASE_BACKEND setting.
  • Impact on LibSQL Users: This fix resolves the problem where ironclaw tool auth <tool> would fail for LibSQL users with a 'Connection pool error', making it impossible to configure tool credentials via the CLI for them.
  • Refactored Database Initialization Logic: The database initialization logic in src/cli/tool.rs and src/cli/mcp.rs was refactored to use a match config.database.backend statement with inner #[cfg] guards, aligning with existing patterns in main.rs and app.rs for proper runtime backend selection.
  • Telegram Channel Authentication Support: Added a top-level auth section to the telegram.capabilities.json file, enabling ironclaw tool auth telegram to function correctly for Telegram channels.
Changelog
  • channels-src/telegram/telegram.capabilities.json
    • Added a new auth section to the Telegram channel capabilities JSON, including secret_name, display_name, instructions, setup_url, token_hint, and env_var fields.
  • src/cli/mcp.rs
    • Refactored the get_secrets_store function to use a match statement on config.database.backend for selecting the appropriate secrets store backend.
    • Moved #[cfg] attributes inside the match arms to correctly gate database-specific initialization code based on both compile-time features and runtime configuration.
    • Ensured crypto is wrapped in Arc once before the match statement and cloned for each branch.
  • src/cli/tool.rs
    • Refactored the auth_tool function's secrets store initialization to use a match statement on config.database.backend.
    • Replaced the previous #[cfg] block structure with #[cfg] attributes within the match arms for proper conditional compilation and runtime database selection.
    • Adjusted crypto handling to be Arc::new(crypto) once and then Arc::clone(&crypto) within the match arms.
Activity
  • The pull request description includes a detailed summary of changes, root cause analysis, verification steps, and a comprehensive test plan, indicating thorough preparation by the author.
  • No external comments or reviews have been posted on this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly addresses a bug where CLI commands would ignore the runtime DATABASE_BACKEND setting when both postgres and libsql features were compiled. The change from compile-time #[cfg] blocks to a runtime match on the database backend is the right approach. The addition of the auth section to the Telegram capabilities file is also a good related fix. I've left a couple of suggestions to improve maintainability by reducing code duplication.

Comment thread src/cli/tool.rs
Comment on lines 728 to +730
let crypto = SecretsCrypto::new(master_key.clone())?;

let secrets_store: Arc<dyn SecretsStore + Send + Sync> = {
#[cfg(feature = "postgres")]
{
let store = crate::history::Store::new(&config.database).await?;
store.run_migrations().await?;
Arc::new(PostgresSecretsStore::new(store.pool(), Arc::new(crypto)))
}
#[cfg(all(feature = "libsql", not(feature = "postgres")))]
{
let crypto = Arc::new(crypto);
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.

medium

These two lines can be combined into one for conciseness. This also avoids shadowing the crypto variable, which can be a source of confusion.

Suggested change
let crypto = SecretsCrypto::new(master_key.clone())?;
let secrets_store: Arc<dyn SecretsStore + Send + Sync> = {
#[cfg(feature = "postgres")]
{
let store = crate::history::Store::new(&config.database).await?;
store.run_migrations().await?;
Arc::new(PostgresSecretsStore::new(store.pool(), Arc::new(crypto)))
}
#[cfg(all(feature = "libsql", not(feature = "postgres")))]
{
let crypto = Arc::new(crypto);
let crypto = Arc::new(SecretsCrypto::new(master_key.clone())?);

@github-actions github-actions bot added scope: channel/cli TUI / CLI channel size: M 50-199 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: new First-time contributor labels Feb 21, 2026
Copy link
Copy Markdown
Collaborator

@serrrfirat serrrfirat left a comment

Choose a reason for hiding this comment

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

LGTM — Clean, well-scoped bugfix.

The core change (compile-time #[cfg] → runtime match config.database.backend) is correct and matches the established pattern in main.rs:407. The #[allow(unreachable_patterns)] fallback handles the no-features-compiled edge case properly.

The auth section addition to telegram.capabilities.json is the right approach — serde silently ignores it in the channel parser (ChannelCapabilitiesFile has no auth field, no deny_unknown_fields), and the tool auth CLI picks it up via the tool CapabilitiesFile parser.

Minor note (non-blocking): The libSQL init block (resolve path → connect → migrate) is now duplicated in main.rs, mcp.rs, and tool.rs. A shared create_secrets_store(config) -> Arc<dyn SecretsStore> helper would eliminate the duplication, but that's cleanup for another PR.

@serrrfirat
Copy link
Copy Markdown
Collaborator

resolve the conflict and lets merge @timetwisterfounder-tech

@ilblackdragon
Copy link
Copy Markdown
Member

Thanks for the work on this, @timetwisterfounder-tech! I've picked up your changes and continued them in #740.

The new PR includes your original work plus:

  • Merge with latest main (resolved 3 conflicts from reformatting and refactoring that happened since)
  • No code changes needed beyond conflict resolution — your fix was already approved

You're credited as co-author. Feel free to review the new PR!

@ilblackdragon
Copy link
Copy Markdown
Member

Closing in favor of #740 which lands your work on top of latest main. Thanks again for the well-scoped bugfix @timetwisterfounder-tech — the root cause analysis and test plan in the original description were excellent. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: new First-time contributor risk: low Changes to docs, tests, or low-risk modules scope: channel/cli TUI / CLI channel size: M 50-199 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants