The original PR added EXCLUDE_SOURCES filtering to pipeline.available_sources()
and to the check-config.sh banner, but env.py::get_config() builds its config
dict from a hardcoded keys list that didn't include EXCLUDE_SOURCES. The
result: setting EXCLUDE_SOURCES in the environment silently no-op'd through
the Python pipeline. Only the bash hook (which reads shell env directly)
worked. The PR's unit tests didn't catch this because they construct config
dicts directly, bypassing get_config().
Changes:
- Add ('EXCLUDE_SOURCES', '') to env.py's keys list so the env var actually
propagates into config.
- Add an end-to-end regression test that goes through get_config() rather
than constructing config dicts directly.
- Document EXCLUDE_SOURCES in SKILL.md's source-list checklist so the model
invoking the skill knows to subtract excluded sources before displaying
the active-sources line. (Per AGENTS.md: engine flags without SKILL.md
prose are incomplete — the agent invoking the skill won't know the flag
exists.)
Addresses Greptile review on PR #407:
- P1: setup-keychain.sh ALL_KEYS was missing GOOGLE_GENAI_API_KEY and
XIAOHONGSHU_API_BASE relative to _load_keychain's inline list, so users
manually storing those keys would not see them in --list and the
interactive prompt would never offer to set them.
Hoist the canonical key list into lib/env.py::KEYCHAIN_KEYS, have
get_config() pass it through, and add a parity test that parses
ALL_KEYS out of setup-keychain.sh and asserts equality. Drift is now
caught at CI time instead of after a user reports a missing key.
- P2: os.environ.get("USER", "") silently returned "" under sudo, in
Docker without --env USER, or in CI runners that strip USER. The
resulting `security find-generic-password -a ""` call would never
match items stored by setup-keychain.sh, so all lookups silently
returned nothing. Fall back to pwd.getpwuid(os.getuid()).pw_name when
USER is absent.
The P2 process-listing comment ("secret visible briefly via ps because
security has no stdin path for -w") has no clean fix — the README
already documents the manual `security add-generic-password` invocation
as an alternative for users with strict secret hygiene.
Adds the macOS Keychain as the lowest-priority credential source on Darwin.
Items stored as generic passwords with service name "last30days-<KEY>" for
the current user are picked up automatically by get_config() — file env
and process env still win on collision.
No new config knob: behavior is strictly additive. On non-Darwin (or when
the `security` binary is missing) the loader is a no-op, so Linux/Windows
behavior is unchanged.
Priority (highest wins):
1. Environment variables
2. .claude/last30days.env (per-project)
3. ~/.config/last30days/.env (global)
4. macOS Keychain items prefixed last30days- (new)
Includes:
- lib/env.py: KEYCHAIN_SERVICE_PREFIX constant, _load_keychain helper
(platform-gated, shutil.which-gated, subprocess-error tolerant),
wiring into get_config before get_openai_auth so OPENAI_API_KEY can
come from Keychain too, _CONFIG_SOURCE reports "keychain" when no
file source is present.
- scripts/setup-keychain.sh: bash helper with interactive set,
--list, --delete, --replace modes. Uses `security add-generic-password`.
- tests/test_env_keychain.py: 12 tests covering platform gate,
missing-binary gate, success path, whitespace stripping, subprocess
errors swallowed, get_config precedence, and an OPENAI_AUTH wiring
regression test.
- tests/test_env_cookies.py: existing integration test mocks the new
_load_keychain hook so it stays hermetic on Darwin developer
machines that have real keychain entries.
- README.md: new "macOS Keychain (optional)" subsection under
"Bring your own keys" documenting setup-keychain.sh and the manual
`security add-generic-password` invocation.
Tested on macOS with a populated keychain and against the existing pytest
suite — CI-tracked tests (test_plugin_contract.py, test_version_consistency.py)
plus all env-touching tests pass. Pre-existing unrelated failures in
test_store.py / test_watchlist_commands.py / test_setup_openclaw.py /
test_footer_nudge_suppression.py are untouched.
1. N+1 queries in store.store_findings()
The old loop ran one SELECT per finding to check existence, then one
INSERT or UPDATE. 100 findings cost 200 serial SQLite roundtrips.
Now: one batch SELECT with WHERE source_url IN (...) builds a lookup
dict, then executemany() handles all inserts and updates. Query count
stays constant regardless of batch size. Benchmark on 500 findings:
~30ms to ~20ms; gap widens on slower storage.
2. O(n^2) source_items dedup in fusion.weighted_rrf()
Merging an item into an existing candidate ran any(existing.source ==
... for existing in candidate.source_items), linearly scanning a list
that grew with each merge. At 40 candidates with 20 source_items each,
fusion went quadratic. Now tracks (source, item_id) tuples in a
per-candidate set for O(1) lookup. The source_items list itself is
unchanged since other code iterates it.
3. Dead code removal
- providers.GeminiClient.ground_search() and .url_context_json(): zero
callers. Deleted.
- render._top_comment_excerpt(): zero callers. Deleted.
- env.is_reddit_available(): one-line wrapper around get_reddit_source.
Callers can check get_reddit_source(config) is not None directly.
Dinakar Sarbada