Skip to content

Override the default log file path by environment variable MAGIC_CONTEXT_LOG_PATH#183

Open
kecsap wants to merge 1 commit into
cortexkit:masterfrom
kecsap:master
Open

Override the default log file path by environment variable MAGIC_CONTEXT_LOG_PATH#183
kecsap wants to merge 1 commit into
cortexkit:masterfrom
kecsap:master

Conversation

@kecsap

@kecsap kecsap commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Allow overriding the Magic Context log file path via MAGIC_CONTEXT_LOG_PATH. Both the plugin and dashboard trim and ignore blank values, defaulting to ${TMPDIR}/<harness>/magic-context/magic-context.log.

  • New Features

    • packages/plugin: getMagicContextLogPath reads trimmed MAGIC_CONTEXT_LOG_PATH with harness-temp fallback.
    • packages/dashboard: resolve_log_path_for mirrors the override; docs updated in README.md, dashboard README.md, and docs reference.
  • Bug Fixes

    • packages/dashboard: Tests serialize env var mutations with a OnceLock<Mutex> to avoid parallel races; cover override, blank, and fallback.
    • packages/plugin: Added tests for unset, override, and blank in getMagicContextLogPath.

Written for commit 2ae175f. Summary will update on new commits.

Review in cubic

Greptile Summary

Adds MAGIC_CONTEXT_LOG_PATH environment variable support to override the default per-harness log file path in both the TypeScript plugin and Rust dashboard; when unset or blank, both sides fall back to the existing ${TMPDIR}/<harness>/magic-context/magic-context.log layout.

  • Plugin (data-path.ts): getMagicContextLogPath now reads, trims, and honors MAGIC_CONTEXT_LOG_PATH before falling back to the harness-based temp path; three new tests cover unset, override, and blank-string cases.
  • Dashboard (log_parser.rs): resolve_log_path_for mirrors the same logic in Rust, with tests serialized via a OnceLock<Mutex<()>> guard to prevent env-mutation races.
  • Docs / READMEs: All three documentation files updated to surface MAGIC_CONTEXT_LOG_PATH and its fallback behaviour.

Confidence Score: 5/5

Safe to merge — the change is a small, well-tested additive feature with no modifications to existing logic.

Both the TypeScript and Rust implementations follow the same trim-and-fallback pattern, the env-mutation race condition raised in a prior review has been properly addressed with a shared mutex, and all three test suites cover the unset, override, and blank-whitespace cases. No existing call sites are affected.

No files require special attention.

Important Files Changed

Filename Overview
packages/plugin/src/shared/data-path.ts Adds two-line env-var override to getMagicContextLogPath; logic is clean and symmetric with the Rust implementation.
packages/dashboard/src-tauri/src/log_parser.rs resolve_log_path_for gains MAGIC_CONTEXT_LOG_PATH override; tests now serialized with OnceLock<Mutex<()>>, addressing prior race-condition concern.
packages/plugin/src/shared/data-path.test.ts Three new tests cover unset, override, and blank env var cases with proper save/restore of MAGIC_CONTEXT_LOG_PATH.
packages/plugin/src/features/magic-context/storage-schema-helpers.ts Type-only import cleanup (import { Database } → import type { Database }); no logic change.
README.md Doc table updated to show MAGIC_CONTEXT_LOG_PATH with harness-specific fallbacks.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Call getMagicContextLogPath / resolve_log_path_for] --> B{MAGIC_CONTEXT_LOG_PATH set?}
    B -- "Yes (non-blank after trim)" --> C[Return env var value as path]
    B -- "No / blank / whitespace-only" --> D{Which harness?}
    D -- opencode --> E["${TMPDIR}/opencode/magic-context/magic-context.log"]
    D -- pi --> F["${TMPDIR}/pi/magic-context/magic-context.log"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Call getMagicContextLogPath / resolve_log_path_for] --> B{MAGIC_CONTEXT_LOG_PATH set?}
    B -- "Yes (non-blank after trim)" --> C[Return env var value as path]
    B -- "No / blank / whitespace-only" --> D{Which harness?}
    D -- opencode --> E["${TMPDIR}/opencode/magic-context/magic-context.log"]
    D -- pi --> F["${TMPDIR}/pi/magic-context/magic-context.log"]
Loading

Comments Outside Diff (1)

  1. packages/plugin/src/shared/data-path.ts, line 39-46 (link)

    P2 The JSDoc still says "Pi and OpenCode write to SEPARATE logs under their respective harness subtrees" — but that guarantee is now broken when MAGIC_CONTEXT_LOG_PATH is set. Both harnesses will resolve to the same file, interleaving their session traces. The comment should be updated to reflect the override, and users should be warned about this behaviour.

Reviews (5): Last reviewed commit: "Override the default log file path by en..." | Re-trigger Greptile

Comment thread packages/docs/src/content/docs/reference/dashboard.md Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 5 files

Re-trigger cubic

@alfonso-magic-context alfonso-magic-context left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this, the MAGIC_CONTEXT_LOG_PATH override is a good idea and the plugin-side change is clean (single choke point in getMagicContextLogPath, trims and ignores blank, with tests). A couple of changes needed before merge:

  1. Please rebase onto master. The branch currently conflicts. We landed the config-location cutover (config + artifacts moved to the shared CortexKit location) since this PR was opened, which touches nearby docs/paths.

  2. The dashboard doc change overstates support. The PR updates packages/dashboard/README.md and the docs dashboard.md to say the dashboard Logs tab honors MAGIC_CONTEXT_LOG_PATH, but the dashboard's Rust log resolver (packages/dashboard/src-tauri/src/log_parser.rs, resolve_log_path_for) reads std::env::temp_dir() and does not read that env var. As written, a user who sets MAGIC_CONTEXT_LOG_PATH would have the plugin write to the custom path while the dashboard keeps reading the default tmp path, so the docs would be wrong.

    Two ways to resolve, either is fine:

    • Simplest: drop the dashboard doc edits from this PR and keep it plugin-only (the env override still works for the plugin's own log file).
    • Complete: also make resolve_log_path_for honor MAGIC_CONTEXT_LOG_PATH (read the env var first, fall back to the tmp path) so the dashboard Logs tab reads the same file the plugin writes. If you'd rather not touch Rust, say so and I'll add that part.

Everything else looks good. Once it's rebased and the dashboard claim is either dropped or backed by the Rust read, I'll merge.

Comment thread packages/dashboard/src-tauri/src/log_parser.rs
@kecsap kecsap force-pushed the master branch 2 times, most recently from 6481d38 to 31501a1 Compare June 27, 2026 10:22
ualtinok added a commit that referenced this pull request Jul 1, 2026
Let users redirect the diagnostic log away from the harness temp-dir default,
which matters in sandboxed/ephemeral setups (Docker, CI) where TMPDIR is
disposable or the plugin and dashboard need to agree on a shared path. The
override is read at the single chokepoint getMagicContextLogPath (TS) and its
Rust mirror resolve_log_path_for, so both the plugin logger and the dashboard
log tail honor it; blank/whitespace is treated as unset. Docs updated in three
places.

Reimplemented from @kecsap's PR #183 onto current master (the PR branch
predated the v0.30.2 WebSocket migration and carried a large stale revert).
Co-authored-by: kecsap <kecsap@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants