feat(cli): add bm config command group for get/set/list/unset#1088
Open
phernandez wants to merge 3 commits into
Open
feat(cli): add bm config command group for get/set/list/unset#1088phernandez wants to merge 3 commits into
phernandez wants to merge 3 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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>
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>
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>
Replace the bare substring check with exact hostname comparison on the parsed masked URL — the substring pattern trips CodeQL's incomplete-URL-sanitization rule and proves less. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A2bgrGfWiL4izcjj66u9R8 Signed-off-by: phernandez <paul@basicmachines.co>
phernandez
force-pushed
the
claude/issue-991-20260716-1949
branch
from
July 21, 2026 02:26
11e7ae2 to
6ee5746
Compare
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) — verified live: set →plain, get reads it back, invalid value rejected by Pydantic literal validation, unset reverts torichprojects) behaves as intended —_is_scalar_annotationexcludes dict/list/nested; covered bytest_configurable_fields_excludes_structured_typesandtest_config_set_rejects_structured_fieldlistandget— both render through the single_render_value→_redact_for_displaypath; covered by masking tests for both commandsset/unsetgo throughConfigManager.save_config→ sharedsave_basic_memory_configCloses #991
🤖 Generated with Claude Code