fix: make -c/--context, -y/--yes, and --fgm flags actually work#549
Open
SOV710 wants to merge 7 commits intodi-sukharev:masterfrom
Open
fix: make -c/--context, -y/--yes, and --fgm flags actually work#549SOV710 wants to merge 7 commits intodi-sukharev:masterfrom
SOV710 wants to merge 7 commits intodi-sukharev:masterfrom
Conversation
…flict cleye's ignoreArgv passes unconsumed flags and arguments through to the internal `git commit` execa call. Although -c/--context is defined as a known cleye flag, a defensive guard is needed to strip it from extraArgs in case it leaks through, which would conflict with git's own handling. Add a sanitization step at the entry of commit() that filters -c, --context, and their values from extraArgs before they are forwarded to the git commit invocation.
Same class of bug as the -c/--context fix: these flags could leak into extraArgs and be forwarded to the internal `git commit` call, causing unexpected behavior. Extend the extraArgs sanitization to also strip -y, --yes, --fgm, and their values.
When the user answers "No" at the confirmation prompt and chooses to regenerate, the recursive call to generateCommitMessageFromGitDiff forwarded only `diff`, `extraArgs`, and `fullGitMojiSpec`. Both `context` and `skipCommitConfirmation` were silently dropped, so: - `-c/--context` was honored only on the first attempt and lost on every regeneration; - `-y/--yes` was honored only on the first attempt, forcing a manual confirmation after regeneration. Forward both fields through the recursive call so the user's flags are respected for the full lifetime of the commit() invocation.
When a staged diff exceeds MAX_REQUEST_TOKENS, generateCommitMessageByDiff routes through getCommitMsgsPromisesFromFileDiffs → getMessagesPromisesByChangesInFile → generateCommitMessageChatCompletionPrompt to produce one sub-prompt per chunk. That entire chain was threading `fullGitMojiSpec` but never `context`, so `-c/--context` was silently dropped for any diff large enough to trigger chunking, even though the simple (non-chunked) path forwarded it correctly. Add a `context` parameter to each of the three helpers and thread it through to generateCommitMessageChatCompletionPrompt so the user's context is present in every sub-prompt.
getCommitConvention gated the entire GitMoji branch on config.OCO_EMOJI, so --fgm was silently ignored unless the user had previously run `oco config set OCO_EMOJI true`. Since OCO_EMOJI defaults to false, --fgm was a no-op for most users. This violates the standard CLI convention that command-line flags should override configuration. Restructure getCommitConvention so that --fgm forces FULL_GITMOJI_SPEC regardless of OCO_EMOJI: --fgm=true → FULL_GITMOJI_SPEC --fgm=false + OCO_EMOJI=true → GITMOJI_HELP (unchanged) --fgm=false + OCO_EMOJI=false → CONVENTIONAL_COMMIT_KEYWORDS (unchanged) No other files need changes — the fgm flag was already threaded correctly through cli.ts → commit.ts → generateCommitMessageByDiff → getMainCommitPrompt → getCommitConvention.
The previous userInputCodeContext only skipped the context block when context was exactly '' or ' '. Anything else (e.g. a string of whitespace, null, undefined) would inject an empty or whitespace-only <context>…</context> tag into the system prompt. Trim the input and guard against null/undefined: - accept string | undefined | null - normalize via `(context ?? '').trim()` - skip the injection whenever the trimmed value is empty Also inline the INIT_MAIN_PROMPT IIFE into a normal function body and introduce a `content` local, removing a layer of nesting that obscured the prompt assembly. Behavior is unchanged.
Author
|
Hi @di-sukharev, could you please take a look when you have time? Thanks! |
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
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.
Summary
This PR fixes three CLI flags that were silently broken or no-ops in the current
ocorelease:-c/--context,-y/--yes, and--fgm. None of them caused a visible error — they just silently did the wrong thing, so users got plain conventional-commit output without ever realizing their flags were dropped (or, in the-ccase, got a crypticgit commithang/failure).Each bug is fixed in its own atomic commit so the history is bisectable.
What was broken & why
1.
-c/--context— completely non-functionalextraArgs = process.argv.slice(2)contained the raw['-c', 'my context']tokens, whichcommit()then forwarded verbatim to the internalgit commit -m "…" -c "my context"call. git interprets its own-cas "reuse commit message from<commit>" and fails/hangs looking for a commit named"my context". (Primary, visible bug.)But
-chad two additional latent bugs that would have broken it even after the extraArgs fix:getCommitMsgsPromisesFromFileDiffs→getMessagesPromisesByChangesInFile→generateCommitMessageChatCompletionPrompt) threadedfullGitMojiSpecbut nevercontext, so-csilently disappeared for any diff large enough to trigger chunking.diff,extraArgs, andfullGitMojiSpec, dropping bothcontextandskipCommitConfirmation. So-c(and-y) worked on attempt 1 and were lost on attempt 2+.2.
-y/--yesand--fgm— same extraArgs leak as-cThese also leaked into the
git commitinvocation. Less destructive than-c(they're not git flags with meaning), but still noise that could interact with repo hooks or aliases.3.
--fgm— no-op even after extraArgs fixgetCommitConventiongated the entire GitMoji branch onconfig.OCO_EMOJI:Since
OCO_EMOJIdefaults tofalse,--fgmwas silently ignored unless the user had previously runoco config set OCO_EMOJI true. CLI flags should override config, not be subordinate to it. Restructured so--fgmforcesFULL_GITMOJI_SPECunconditionally.Commits (in bisect order)
fix(cli): strip -c/--context from extraArgs to prevent git commit conflictstripOcoFlagswith-c/--contexthandling; feed raw argv to cleye but stripped argv togit commit.fix(cli): strip -y/--fgm from extraArgs to prevent git commit conflict-y/--yes/--fgm.fix(commit): preserve context and skip-confirm flag across regeneratecontext+skipCommitConfirmationthrough the recursive regenerate call.fix(generate): forward context through chunked large-diff prompt pathcontextthroughgetCommitMsgsPromisesFromFileDiffs/getMessagesPromisesByChangesInFile/generateCommitMessageChatCompletionPrompt.fix(prompts): make --fgm override OCO_EMOJI configgetCommitConventionso--fgmwins overOCO_EMOJI.refactor(prompts): null-safe, trim-aware user context handlinguserInputCodeContextagainst whitespace-only and null/undefined input; inline theINIT_MAIN_PROMPTIIFE for clarity. Behavior unchanged.buildout/cli.cjs.Behavior matrix —
--fgm×OCO_EMOJI--fgmOCO_EMOJIfalse(default)CONVENTIONAL_COMMIT_KEYWORDStrueGITMOJI_HELPfalseFULL_GITMOJI_SPECtrueFULL_GITMOJI_SPECTest plan
pnpm run build— cleanpnpm test— all 36 existing unit tests passoco -c "add retry logic"on a small diff → context appears in generated message, nogit commithangoco -c "…"on a large diff that triggers chunking → context still appears in each sub-promptoco --fgmwith default config (OCO_EMOJI=false) → message prefixed with an emoji from the 81-item full spec (e.g. 🎨, ⚡️)oco(no flags) withOCO_EMOJI=false→ plain conventional commit, unchangedoco(no flags) withOCO_EMOJI=true→ 10-itemGITMOJI_HELPsubset, unchangedoco -y --fgm -c "…"→ all three flags honored, none leak intogit commit