feat(cli): add bm config command group for get/set/list/unset#1088
Open
phernandez wants to merge 2 commits into
Open
feat(cli): add bm config command group for get/set/list/unset#1088phernandez wants to merge 2 commits into
phernandez wants to merge 2 commits into
Conversation
Every user-facing setting previously required hand-editing config.json or knowing the BASIC_MEMORY_* env-var convention. `bm config list|get|set|unset` covers the scalar subset of BasicMemoryConfig (derived from the model, not a hand-maintained list), validates `set` through the model so invalid values fail with the Pydantic error, and marks BASIC_MEMORY_* env overrides. Secret-redaction rules (cloud_api_key never printed, database_url credentials masked) are factored out of the existing basic_memory_diagnostics MCP tool into a shared basic_memory/redaction.py module so both surfaces stay in sync. Closes #991 Co-authored-by: Paul Hernandez <60959+phernandez@users.noreply.github.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| assert result.exit_code == 0, result.output | ||
| assert "dbpass" not in result.output | ||
| assert "dbuser" not in result.output | ||
| assert "host.example.com" in result.output |
Member
Author
Local validationCI didn't auto-run on this branch (it was pushed by the bot's No failures. ✅ |
phernandez
added a commit
that referenced
this pull request
Jul 16, 2026
Anthropic's OIDC exchange is broken for the pull_request_target event (401 'Invalid OIDC token'; anthropics/claude-code-action#713). The workflow invoked that path whenever a PR body contained '@claude' — and bot-authored PR descriptions carry attribution text ('Generated by the @claude workflow'), so opening #1088/#1089/#1090/#1091 self-triggered the broken path and each failed its 'claude' check with a 401. A normal issue_comment run succeeded with the same secret in between, so the OAuth token is valid and needs no rotation. Remove the pull_request_target trigger and its PR-body '@claude' clause, so attribution text can never be read as a command and no event routes into the broken OIDC path. Explicit '@claude' mentions in issues, issue comments, PR reviews, and PR review comments remain the trigger. Also drop the now-dead pull_request.author_association gate clauses (reviewer identity is covered by sender.author_association) and the pull_request_target-only checkout ref. If automatic PR-open review is wanted later, it should be a dedicated workflow with a fixed prompt, not a PR-body mention. Signed-off-by: phernandez <paul@basicmachines.co>
Self-review follow-up on the generated code. config.py worked (24 tests green) but did not read like the rest of the codebase: - Introduce a frozen/slots ConfigSetting dataclass for the list rows instead of anonymous dict literals threaded through list/get output (docs/ENGINEERING_STYLE asks for dataclasses over ad-hoc dict[str, Any] shapes). JSON output is unchanged (asdict -> key/value/source). - Fully type the helpers and collections (list[ConfigSetting], frozenset[str]); redaction.py signatures are dict[str, Any] rather than bare dict. - Drop the silent 'except (OSError, json.JSONDecodeError)' fallback when reading config.json for source labelling. ConfigManager already parsed the file to load the config, so a read failure is a real inconsistency that should surface, not be swallowed into mislabelling every setting as a default (fail-fast rule). - De-duplicate secret masking: _render_value delegates to _redact_for_display instead of re-checking SECRET_FIELDS; name the display sentinels (SECRET_MASK / NOT_SET). - Collapse the two-step _unknown_key_error/_require_known_key into one, add section headers. Behavior and the 24 existing tests are unchanged; the model-derived allowlist is kept (it is the correct way to track BasicMemoryConfig). Signed-off-by: phernandez <paul@basicmachines.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #991. Generated by the
@claudeworkflow from the issue instructions; opened for review — not yet verified by a human.Summary
Adds a
bm configcommand group:bm config list— settings with effective values, env overrides markedbm config get <key>bm config set <key> <value>— round-trips throughBasicMemoryConfigso invalid values fail with the Pydantic errorbm config unset <key>— revert to defaultRedaction rules (never print
cloud_api_key, maskdatabase_urlcreds) were factored out ofbasic_memory_diagnosticsinto a sharedbasic_memory/redaction.pyso both surfaces stay in sync.Review checklist
bm config set cli_output_style plainworks end-to-end (the feat(cli): add Rich human-readable output to bm tool commands #967 motivator)projects) behaves as intendedlistandgetCloses #991
🤖 Generated with Claude Code