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. If unset or blank, it falls back to ${TMPDIR}/<harness>/magic-context/magic-context.log (e.g., opencode or pi).

  • New Features
    • packages/plugin: getMagicContextLogPath reads MAGIC_CONTEXT_LOG_PATH and ignores blank values.
    • Added tests for env override and fallback behavior.
    • Updated README.md, dashboard README, and docs to document the new env var and fallback path.

Written for commit 706d9cc. Summary will update on new commits.

Review in cubic

Greptile Summary

This PR adds support for overriding the magic-context log file path via the MAGIC_CONTEXT_LOG_PATH environment variable, with trimming to ignore blank/whitespace-only values and falling back to the existing per-harness temp-dir path when unset.

  • getMagicContextLogPath in data-path.ts now checks MAGIC_CONTEXT_LOG_PATH first, returning it (trimmed) if non-empty, otherwise falling back to the harness-specific path under os.tmpdir().
  • Three new tests cover the default fallback, the env-var override, and the blank-value guard; env state is cleanly saved and restored in before/afterEach.
  • All three documentation files are updated to surface the new env var, though packages/docs/src/content/docs/reference/dashboard.md omits the Pi-harness fallback subdirectory (pi/) that the other two docs include.

Confidence Score: 4/5

Safe to merge; the implementation is minimal and correct, with good test coverage of the new env-var logic.

The two-line change is straightforward, tests exercise every branch, and env state is properly saved/restored. The only rough edges are a stale JSDoc that no longer accurately describes the harness-separation guarantee and a minor omission in one docs file — neither affects runtime behaviour.

packages/plugin/src/shared/data-path.ts — the JSDoc comment about harness-separated logs needs updating to reflect that MAGIC_CONTEXT_LOG_PATH bypasses that separation; packages/docs/src/content/docs/reference/dashboard.md — missing the Pi fallback path notation.

Important Files Changed

Filename Overview
packages/plugin/src/shared/data-path.ts Adds MAGIC_CONTEXT_LOG_PATH env-var override to getMagicContextLogPath; logic is correct and trims blank values, but the JSDoc no longer accurately describes the harness-separation guarantee.
packages/plugin/src/shared/data-path.test.ts Three new tests cover the default fallback, the env-var override, and the blank-value guard; env state is correctly saved and restored in before/afterEach.
README.md Docs updated to surface MAGIC_CONTEXT_LOG_PATH with the original path shown as a fallback; wording is consistent with dashboard README.
packages/dashboard/README.md Updated log path entry to reference the new env var with fallback, consistent with root README.
packages/docs/src/content/docs/reference/dashboard.md Docs updated for the new env var, but the Pi-harness fallback subdirectory (pi/) is omitted, making it slightly inconsistent with the other two docs files.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["getMagicContextLogPath(harness)"] --> B{"MAGIC_CONTEXT_LOG_PATH set\nand non-blank?"}
    B -->|Yes| C["Return env var value\n(harness ignored)"]
    B -->|No| D["getMagicContextTempDir(harness)"]
    D --> E["path.join(os.tmpdir(), harness, 'magic-context')"]
    E --> F["Return harness-specific path\ne.g. /tmp/opencode/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["getMagicContextLogPath(harness)"] --> B{"MAGIC_CONTEXT_LOG_PATH set\nand non-blank?"}
    B -->|Yes| C["Return env var value\n(harness ignored)"]
    B -->|No| D["getMagicContextTempDir(harness)"]
    D --> E["path.join(os.tmpdir(), harness, 'magic-context')"]
    E --> F["Return harness-specific path\ne.g. /tmp/opencode/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 (1): Last reviewed commit: "Override the default log file path by en..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

## Logs

Optional **log tail** for `magic-context.log` with filtering — useful alongside Cache when correlating busts with plugin log lines. Open from the sidebar **Logs** item.
Optional **log tail** for `MAGIC_CONTEXT_LOG_PATH` (fallback: `${TMPDIR}/opencode/magic-context/magic-context.log`) with filtering — useful alongside Cache when correlating busts with plugin log lines. Open from the sidebar **Logs** item.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The two README files both mention pi/ as the Pi-harness fallback subdirectory, but this docs page omits it, leaving the description inconsistent.

Suggested change
Optional **log tail** for `MAGIC_CONTEXT_LOG_PATH` (fallback: `${TMPDIR}/opencode/magic-context/magic-context.log`) with filtering — useful alongside Cache when correlating busts with plugin log lines. Open from the sidebar **Logs** item.
Optional **log tail** for `MAGIC_CONTEXT_LOG_PATH` (fallback: `${TMPDIR}/opencode/magic-context/magic-context.log`, `pi/` for Pi) with filtering — useful alongside Cache when correlating busts with plugin log lines. Open from the sidebar **Logs** item.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@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.

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