Skip to content

feat(cli): add --no-context-files / -c flag to skip AGENTS.md and CLAUDE.md context loading#21

Open
zombi3butt wants to merge 1 commit into
quangdang46:masterfrom
zombi3butt:feature/no-context-files-flag
Open

feat(cli): add --no-context-files / -c flag to skip AGENTS.md and CLAUDE.md context loading#21
zombi3butt wants to merge 1 commit into
quangdang46:masterfrom
zombi3butt:feature/no-context-files-flag

Conversation

@zombi3butt
Copy link
Copy Markdown

What

Adds a new CLI flag --no-context-files (short form: -c) to skip loading AGENTS.md and CLAUDE.md context files in the system prompt.

This addresses issue #9: #9

Changes

  • src/cli/args.rs: Added --no-context-files global CLI arg with short form -c
  • src/prompt.rs:
    • Added no_context_files: bool field to ContextInfo struct
    • load_agents_md_files_from_dir(): early return with empty context when env vars JCODE_NO_CONTEXT_FILES or JCODE_NC are set
    • Wired value into both build_system_prompt_full and build_system_prompt_split callers

Notes

  • No Rust toolchain available locally to run `cargo check`. Maintainer should verify compilation.
  • The logic is safe: the early return produces empty ContextInfo which matches default behavior when no AGENTS.md files exist.

…UDE.md context loading

- Adds CLI arg --no-context-files with short form -c
- Adds no_context_files field to ContextInfo struct
- load_agents_md_files_from_dir returns early when env vars are set
- Wire value into both prompt builder callers
@quangdang46
Copy link
Copy Markdown
Owner

@zombi3butt Thanks for your contribution.I will review and merge it soon

@quangdang46
Copy link
Copy Markdown
Owner

Hi @zombi3butt, thanks a lot for picking up issue #9 and sending this in — direction is right and the diff is small and focused — thank you for the contribution! Before merging, I have a few concerns; the first one is blocking.

Blocking

1. The --no-context-files / -c CLI flag is dead code

The new field is added to Args, but args.no_context_files is never read anywhere. The actual gating in src/prompt.rs uses only env vars:

// src/prompt.rs:608
if std::env::var("JCODE_NO_CONTEXT_FILES").is_ok() || std::env::var("JCODE_NC").is_ok() { return (None, ContextInfo::default()); }

So:

  • JCODE_NO_CONTEXT_FILES=1 jcode run "..." → works
  • jcode --no-context-files run "..."no effect, AGENTS.md is still loaded

This contradicts the PR description ("Wired value into both build_system_prompt_full and build_system_prompt_split callers") and fails issue #9's acceptance criteria ("Flag works for jcode, jcode run, jcode serve/connect.").

The minimal fix is to translate the flag → env var at startup, the same way args.trace is handled today in src/cli/startup.rs:58-60:

if args.no_context_files {
    crate::env::set_var("JCODE_NO_CONTEXT_FILES", "1");
}

Or thread the bool explicitly through build_system_prompt_full / build_system_prompt_split / load_agents_md_files_from_dir.

2. no_context_files field on ContextInfo is set but never read

// src/prompt.rs:73-75
/// Skip loading AGENTS.md and CLAUDE.md context files
pub no_context_files: bool,

ContextInfo describes what was loaded (sizes, presence flags) — it's not the right place to store user intent. Nothing downstream consumes info.no_context_files. Please drop this field (and the two assignments at lines 208 and 282) — the env-var (or threaded bool) is enough.

Should fix before merge

3. Undocumented JCODE_NC env var

Issue #9 spec is JCODE_NO_CONTEXT_FILES only. The extra JCODE_NC alias is:

  • not in the issue,
  • not in --help, README, or any code comment,
  • ambiguous to future readers ("NC" = network / no-color / no-context?).

Please drop JCODE_NC and keep only JCODE_NO_CONTEXT_FILES.

4. Missing UX log line (issue #9 step 4)

Issue #9 explicitly asks:

When skipped, log a single line at session start: Context files disabled (--no-context-files). This is critical so users don't get confused why behavior changed.

This isn't implemented. Please add it in load_agents_md_files_from_dir (or startup) when the flag/env is active.

5. No tests

Issue #9 testing checklist asks for at least:

  • With the flag set, the context-file loader returns empty even when fixture AGENTS.md is present.
  • Env-var path works.

src/prompt_tests.rs already has test_load_agents_md_files_uses_sandboxed_global_files as a great pattern to copy.

Nits

6. -c short flag collides visually with -C (--cwd)

-c (lowercase) vs -C (uppercase, for --cwd) is a foot-gun — jcode -c /tmp run "..." will pass /tmp as the next positional arg, which is surprising. Issue #9 suggested -n. I'd drop the short alias entirely (long form --no-context-files is fine, matches --no-update, --no-selfdev, --no-browser which are all long-only).

7. Title/description mention "CLAUDE.md", but jcode doesn't load CLAUDE.md

Only AGENTS.md (project) and ~/.AGENTS.md (global) are loaded by load_agents_md_files_from_dir. The "CLAUDE.md" / "JCODE.md" wording from issue #9 was aspirational. Please tighten the PR title/description to match what's actually skipped today.

8. Repeated env lookup

The same std::env::var("JCODE_NO_CONTEXT_FILES").is_ok() || std::env::var("JCODE_NC").is_ok() line is duplicated 3 times. Even if you drop JCODE_NC per #3, please extract a tiny helper (e.g. fn context_files_disabled() -> bool) so there's one source of truth.

9. No README update

Acceptance criteria asks for README documentation. A short bullet under the "Flags" / "Usage" section is enough.

10. Settings support (issue #9 step 3) is optional for v1

Adding loadContextFiles: bool = true to user/project config would be nice for CI-permanent use, but I'd be fine landing this PR without it and tracking that in a follow-up — as long as items 1–5 are addressed.


To summarize: the must-fix-before-merge items are #1 (flag is dead), #2 (dead field on ContextInfo), #3 (drop JCODE_NC), #4 (log line), and #5 (tests). The rest are nits I'd appreciate but won't block on.

Thanks again for the contribution — looking forward to the next round!

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.

3 participants