💄 Standardize YAML formatting and quotes #3
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
We failed to fetch the diff for pull request #3
You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.
WalkthroughAdds a new Dockerized Python "people" GitHub Action, many GitHub workflows, a full Axilo CLI (Node + React Ink) with agent, approval and patch utilities, examples, tests, tooling (Dockerfiles, Husky hooks, lint/config), and repo-level docs/license/config changes; removes some root-level build artifacts (Dockerfile, Makefile, AXILO.md, .dockerignore). Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as GitHub Scheduler / Manual Trigger
participant Runner as GitHub Actions Runner (container)
participant PeopleApp as people/app/main.py
participant GitHubAPI as GitHub GraphQL API
participant Git as Git (repo)
Scheduler->>Runner: start "people" workflow
Runner->>PeopleApp: launch container & run python module
PeopleApp->>GitHubAPI: GraphQL queries (issues, discussions, PRs, sponsors) [paginated]
GitHubAPI-->>PeopleApp: return paginated edges
PeopleApp->>PeopleApp: aggregate metrics, render YAML files
PeopleApp->>Git: write files, git commit, create branch, push
PeopleApp->>GitHubAPI: create Pull Request for updated docs/data
GitHubAPI-->>Runner: PR creation response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)axilo-cli/src/utils/agent/agent-loop.ts (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (16)
axilo-cli/examples/prompting_guide.md-11-13 (1)
11-13: Specify language identifiers for fenced code blocks.Fenced code blocks are missing language tags, which impacts syntax highlighting and readability. Add appropriate language identifiers (
bashfor shell commands,markdownfor content, etc.) to all code blocks.Apply this diff to add language identifiers:
-``` +```bash axilo --help -``` +``` -``` +```bash axilo "write 2-3 sentences on what you can do" -``` +``` -``` +```bash mkdir first-task && cd first-task git init axilo "Create a file poem.html that renders a poem about the nature of intelligence and programming by you, Axilo. Add some nice CSS and make it look like it's framed on a wall" -``` +``` -``` +```markdown poem.html has been added. Highlights: - Centered "picture frame" on a warm wall‑colored background using flexbox. - Double‑border with drop‑shadow to suggest a wooden frame hanging on a wall. - Poem is pre‑wrapped and nicely typeset with Georgia/serif fonts, includes title and small signature. - Responsive tweaks keep the frame readable on small screens. Open poem.html in a browser and you'll see the poem elegantly framed on the wall. -``` +``` -``` +```markdown Refactor: simplify model names across static documentation Can you update docs_site to use a better model naming convention on the site. @@ -108,7 +108,7 @@ Replace confusing model identifiers with a simplified version wherever they're u Write what you changed or tried to do to final_output.md -``` +```Also applies to: 17-19, 23-27, 33-43, 95-109
axilo-cli/src/components/vendor/ink-select/use-select-state.js-112-119 (1)
112-119: State update during render is an anti-pattern.Calling
dispatchandsetLastOptionsdirectly in the render body (outside a hook) can cause issues with React's rendering cycle, including potential infinite loops or unexpected re-renders. This should be moved into auseEffect.const [lastOptions, setLastOptions] = useState(options); - if (options !== lastOptions && !isDeepStrictEqual(options, lastOptions)) { - dispatch({ - type: "reset", - state: createDefaultState({ visibleOptionCount, defaultValue, options }), - }); - setLastOptions(options); - } + useEffect(() => { + if (options !== lastOptions && !isDeepStrictEqual(options, lastOptions)) { + dispatch({ + type: "reset", + state: createDefaultState({ visibleOptionCount, defaultValue, options }), + }); + setLastOptions(options); + } + }, [options, lastOptions, visibleOptionCount, defaultValue]);.husky/commit-msg-5-8 (1)
5-8: The commitlint detection logic may not work correctly.The check
command -v commitlintlooks for a globalcommitlintbinary, but commitlint is typically run vianpxand may not be in PATH. This check will likely always fail and trigger installation on every commit, even when commitlint is already installed as a dev dependency.Consider removing this auto-install logic and documenting that
npm installshould be run first, or check for the package innode_modulesinstead:-# Install commitlint if not present -if ! command -v commitlint >/dev/null 2>&1; then - echo "Installing commitlint..." - npm install --save-dev @commitlint/{cli,config-conventional} -fi +# Commitlint should be installed via npm installCommittable suggestion skipped: line range outside the PR's diff.
axilo-cli/examples/camerascii/run.sh-63-68 (1)
63-68: Add error handling for directory change and Axilo launch.Line 63 is missing error handling for the
cdcommand, which could cause the script to execute in the wrong directory if it fails. Additionally, theyq/jqpipeline andaxilocommand should validate success.Based on the static analysis hint, apply this diff to add proper error handling:
-cd "run_$new_run_number" +cd "run_$new_run_number" || { + echo "Error: Failed to change to run_$new_run_number directory" + exit 1 +} # Launch Axilo echo "Launching..." -description=$(yq -o=json '.' ../../task.yaml | jq -r '.description') -axilo "$description" +description=$(yq -o=json '.' ../../task.yaml 2>/dev/null | jq -r '.description' 2>/dev/null) || { + echo "Error: Failed to read task description from ../../task.yaml" + exit 1 +} +axilo "$description" || { + echo "Error: Axilo command failed" + exit 1 +}axilo-cli/.github/workflows/ci.yml-156-163 (1)
156-163: Update to the non-deprecated release action.The
actions/create-release@v1action is deprecated. Usesoftprops/action-gh-release@v2instead, which is already used in the docker job.Apply this diff:
- name: Create GitHub Release - uses: actions/create-release@v1 + uses: softprops/action-gh-release@v2 + if: startsWith(github.ref, 'refs/tags/') env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - tag_name: ${{ github.ref }} - release_name: Release ${{ github.ref }} - draft: false - prerelease: false.github/actions/people/action.yml-8-10 (1)
8-10: Token input is not passed to the Docker container.The
tokeninput is defined but not passed to the Docker container as an environment variable or argument. The container won't have access to the token.runs: using: "docker" image: "Dockerfile" + env: + INPUT_TOKEN: ${{ inputs.token }}Alternatively, if the Dockerfile's entrypoint expects arguments:
runs: using: "docker" image: "Dockerfile" + args: + - ${{ inputs.token }}.github/workflows/people.yml-87-93 (1)
87-93: Update to codecov-action@v4.The
codecov/codecov-action@v3action uses a Node.js 16 runner which is deprecated by GitHub. Update to v4 to avoid future workflow failures.Apply this diff:
- name: Upload coverage to Codecov if: github.event_name != 'workflow_dispatch' || github.event.inputs.debug_enabled != 'true' - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4.github/workflows/people.yml-39-43 (1)
39-43: Update to setup-python@v5.The
setup-python@v4action uses a Node.js 16 runner which is deprecated by GitHub. Update to v5 to avoid future workflow failures.Apply this diff:
- - name: Set up Python ${{ env.PYTHON_VERSION }} - uses: actions/setup-python@v4 + - name: Set up Python ${{ env.PYTHON_VERSION }} + uses: actions/setup-python@v5.github/workflows/people.yml-28-30 (1)
28-30: Moveenv.DOCKER_IMAGEout ofcontainer.image— env context is not supported there.GitHub Actions only supports
github,needs,strategy,matrix,vars, andinputscontexts in thecontainer.imagefield. Using${{ env.DOCKER_IMAGE }}will fail at runtime. Set the image reference directly (e.g.,image: myregistry/myimage:tag) or use a job-level variable via thevarscontext.axilo-cli/src/components/chat/terminal-message-history.tsx-33-36 (1)
33-36: Potential unsafe non-null assertion.On line 34,
batch.map(({ item }) => item!)uses a non-null assertion, but according to theBatchEntrytype definition (lines 10-13), eitheritemorgroupcan be present—not both. This meansitemcould beundefinedfor group entries, making the non-null assertion potentially unsafe.Consider filtering or providing a fallback:
const [messages, debug] = useMemo( - () => [batch.map(({ item }) => item!), process.env["DEBUG"]], + () => [batch.filter(({ item }) => item != null).map(({ item }) => item!), process.env["DEBUG"]], [batch], );Or, if group entries should be handled differently, adjust the logic to account for both entry types.
.github/actions/people/tests/test_graphql.py-55-73 (1)
55-73: Same async/await concern in error test.The
test_get_graphql_response_errortest has the same issue:asyncdecorator but noawaiton line 68-71.Ensure consistency with the actual function signature. If sync, this test will pass but won't actually test async behavior. If async, the missing
awaitmeans the coroutine is never executed and the test passes incorrectly.axilo-cli/src/components/chat/message-history.tsx-24-28 (1)
24-28: Unsafe non-null assertion on optionalitemproperty.Line 28 uses
item!butBatchEntry.itemis optional (item?: ResponseItem). When aBatchEntrycontains agroupinstead of anitem, this will produceundefinedvalues in themessagesarray, which could cause runtime errors when rendering.Filter out entries without
item:const MessageHistory: React.FC<MessageHistoryProps> = ({ batch, headerProps, }) => { - const messages = batch.map(({ item }) => item!); + const messages = batch + .filter((entry): entry is BatchEntry & { item: ResponseItem } => entry.item != null) + .map(({ item }) => item);Or if groups should also be rendered, the component needs additional logic to handle
GroupedResponseItem.axilo-cli/Dockerfile-49-60 (1)
49-60: USER directive confusion:appuseris created butnodeis used.The Dockerfile creates a non-root
appuser(lines 50-52) but then switches tonodeon line 60. Theappusersetup is effectively dead code. Additionally, placingUSER nodeafterCMDis unusual—Dockerfile instructions afterCMDwill still execute during build, but the ordering suggests intent confusion.Consider one of these approaches:
- Remove appuser entirely if you intend to use the built-in
nodeuser:-# Create a non-root user -RUN addgroup -S appuser && adduser -S appuser -G appuser -RUN chown -R appuser:appuser /app -USER appuser - # Health check HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ CMD node -e "require('http').get('http://localhost:3000/health')" # Default command CMD ["node", "dist/cli.js"] -USER node +USER node
- Use appuser consistently and remove the
USER nodeline.axilo-cli/scripts/run_in_container.sh-50-52 (1)
50-52: Fix argument handling to avoid word splitting issues.Using
"$@"inside a double-quoted string with other text causes incorrect expansion. The$@is intended to pass multiple arguments, but when embedded in a string like"cd ... && axilo ... \"$@\"", only$1is used and remaining args are dropped or mishandled.Apply this diff to fix the argument handling:
-docker exec axilo bash -c "cd \"$WORK_DIR\" && axilo --dangerously-auto-approve-everything -q \"$@\"" +CMD="$*" +docker exec axilo bash -c "cd \"$WORK_DIR\" && axilo --dangerously-auto-approve-everything -q $CMD"Alternatively, if a single command string is always expected:
-docker exec axilo bash -c "cd \"$WORK_DIR\" && axilo --dangerously-auto-approve-everything -q \"$@\"" +docker exec axilo bash -c "cd \"$WORK_DIR\" && axilo --dangerously-auto-approve-everything -q \"$1\""The first approach uses
$*which expands all positional parameters as a single string (separated by the first character of IFS). The second approach is cleaner if only a single quoted command string is expected per the usage examples.axilo-cli/src/components/chat/terminal-chat.tsx-191-208 (1)
191-208: Potential race condition in initial input processing.The effect clears
initialPromptandinitialImagePathsimmediately (lines 203-204) before confirmingagent?.run()was actually called. Ifagentisundefinedon the first run, the prompts are cleared but never processed. Whenagentbecomes available on a subsequent render,initialPromptwill already be empty.Consider deferring the clear until after confirming the agent ran:
useEffect(() => { const processInitialInputItems = async () => { if ( (!initialPrompt || initialPrompt.trim() === "") && (!initialImagePaths || initialImagePaths.length === 0) ) { return; } + if (!agent) { + return; // Wait for agent to be ready + } const inputItems = [ await createInputItem(initialPrompt || "", initialImagePaths || []), ]; // Clear them to prevent subsequent runs setInitialPrompt(""); setInitialImagePaths([]); - agent?.run(inputItems); + agent.run(inputItems); }; processInitialInputItems(); }, [agent, initialPrompt, initialImagePaths]);axilo-cli/src/components/vendor/ink-text-input.tsx-294-300 (1)
294-300: Bug: Boundary checks use the wrong variable.The boundary checks on lines 294-300 compare
cursorOffset(the old cursor position) instead ofnextCursorOffset(the computed new position). This means the clamping logic never actually takes effect.Apply this diff to fix the boundary checks:
- if (cursorOffset < 0) { + if (nextCursorOffset < 0) { nextCursorOffset = 0; } - if (cursorOffset > originalValue.length) { + if (nextCursorOffset > originalValue.length) { nextCursorOffset = originalValue.length; }
🟡 Minor comments (23)
axilo-cli/src/components/vendor/ink-select/option-map.js-1-25 (1)
1-25: Duplicate option values will cause data loss.If two options share the same
value, the second entry will overwrite the first in the Map (line 19:items.push([option.value, item])). The linked list remains intact, but Map lookups would only find the later item. Consider validating uniqueness or documenting this constraint.axilo-cli/src/components/vendor/ink-select/use-select-state.js-4-80 (1)
4-80: Add a default case to the reducer.The reducer switch statement lacks a default case. If an unknown action type is dispatched, the function will implicitly return
undefined, which could corrupt state.case "reset": { return action.state; } + default: { + return state; + } } };axilo-cli/examples/camerascii/task.yaml-2-5 (1)
2-5: Clarify the screenshot reference.The description instructs users to "Take a look at the screenshot details," but no screenshot is provided in this directory or mentioned in the context. Ensure the referenced screenshot exists at a documented location, or update the description to be self-contained.
axilo-cli/CONTRIBUTING.md-36-36 (1)
36-36: Update branch reference to match actual workflow.The documentation references "main branch," but this PR targets the "dev" branch. Update the branch name to reflect the actual branching strategy used in this repository.
Apply this diff if "dev" is the primary branch:
-4. Once approved, it will be merged into the main branch +4. Once approved, it will be merged into the dev branchaxilo-cli/src/components/vendor/ink-spinner.tsx-21-36 (1)
21-36: Fallback safely for unknown spinner typesIf
typeis anything other than"dots"or"ball",framesbecomes an empty array, andframes[frame]will beundefinedwhileframekeeps incrementing. It’s safer to always fall back to the"dots"frames:export default function Spinner({ type = "dots", }: { type?: string; }): JSX.Element { - const frames = spinnerTypes[type || "dots"] || []; + const frames = spinnerTypes[type] ?? spinnerTypes.dots; const interval = 80; const [frame, setFrame] = useState(0);This ensures a valid animation even when callers pass an unexpected
type.axilo-cli/examples/build-codex-demo/run.sh-49-60 (1)
49-60: Handle potentialcdfailure into the new run directoryIf
cd "run_$new_run_number"fails (e.g., mkdir error, permissions), the script will keep running in the previous directory and executeaxilofrom the wrong place. Mirroring the earliercd runs || exit 1makes this safer.- cd "run_$new_run_number" + cd "run_$new_run_number" || exit 1This also resolves the ShellCheck SC2164 warning.
axilo-cli/.github/workflows/ci.yml-61-61 (1)
61-61: Fix the tag detection conditional.The condition
contains(github.ref, 'refs/tags/')will match any ref containing that substring, including branches likefeature/refs/tags/test. UsestartsWith()instead for precise tag detection.Apply this diff:
- if: github.event_name == 'push' && contains(github.ref, 'refs/tags/') + if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/')Apply the same fix to line 128:
- if: github.event_name == 'push' && contains(github.ref, 'refs/tags/') + if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/')Also applies to: 128-128
axilo-cli/.github/workflows/ci.yml-149-153 (1)
149-153: Remove redundant NODE_AUTH_TOKEN environment variable.The inline
.npmrcfile (line 150) creates a literal token at runtime, which npm will use directly. TheNODE_AUTH_TOKENenvironment variable (line 153) is unused because npm prioritizes the literal token in.npmrcover environment variables. Either use the inline.npmrcwith a literal token (current approach), or remove the inline echo and rely onsetup-nodeto create.npmrcwithNODE_AUTH_TOKENsubstitution—not both.axilo-cli/src/components/help-overlay.tsx-46-46 (1)
46-46: Replace HTML entity with plain ampersand.The
&HTML entity will render literally in the terminal rather than as&. Use a plain&character instead for correct terminal display.Apply this diff:
<Text> - <Text color="cyan">/history</Text> – show command & file history + <Text color="cyan">/history</Text> – show command & file history for this session </Text> <Text> - <Text color="cyan">/clear</Text> – clear screen & context + <Text color="cyan">/clear</Text> – clear screen & context </Text>Also applies to: 50-50
axilo-cli/examples/prompt-analyzer/task.yaml-10-10 (1)
10-10: Fix typo in description."reaons" should be "reasons" in the phrase "primarily for aesthetic reaons".
Apply this diff:
- - Click "cluster" and see progress stream in a small window (primarily for aesthetic reaons) + - Click "cluster" and see progress stream in a small window (primarily for aesthetic reasons)axilo-cli/examples/impossible-pong/run.sh-63-63 (1)
63-63: Add error handling for directory change.The script changes into the newly created run directory without error handling. If the directory creation failed silently or the directory is inaccessible, the script will continue in the wrong directory and potentially cause issues.
Apply this diff to add error handling:
-cd "run_$new_run_number" +cd "run_$new_run_number" || exit 1axilo-cli/examples/prompt-analyzer/run.sh-63-63 (1)
63-63: Add error handling for directory change.The script changes into the newly created run directory without error handling. This is the same issue found in the impossible-pong example's run.sh.
Apply this diff:
-cd "run_$new_run_number" +cd "run_$new_run_number" || exit 1axilo-cli/src/cli.tsx-150-152 (1)
150-152: URL inconsistency with branding.The error message refers to "KhulnaSoft API key" but the URL points to
platform.openai.com. Ensure branding consistency.README.md-269-269 (1)
269-269: Typo: "siginificantly" → "significantly".-This project is under active development and the code will likely change pretty siginificantly. We'll update this message once that's complete! +This project is under active development and the code will likely change pretty significantly. We'll update this message once that's complete!README.md-123-123 (1)
123-123: Missing word in Node.js requirement.-| Node.js | **22 newer** (LTS recommended) | +| Node.js | **22 or newer** (LTS recommended) |axilo-cli/src/components/singlepass-cli-app.tsx-440-443 (1)
440-443: Error details are discarded; consider logging or displaying them.The caught error is not logged or shown to the user, making debugging difficult. At minimum, log the error to help diagnose API failures.
} catch (err) { setShowSpinner(false); + // eslint-disable-next-line no-console + console.error("[SinglePassApp] API error:", err); setState("error"); }README.md-20-20 (1)
20-20: Broken anchor link due to special character mismatch.The heading "Non‑interactive / CI mode" uses a non-breaking hyphen (U+2011:
‑) while the TOC link#non‑interactive--ci-modeuses a regular hyphen. This causes the link to not resolve correctly.Either use standard hyphens in the heading or update the anchor to match. For example:
-1. [Non‑interactive / CI mode](#non‑interactive--ci-mode) +1. [Non-interactive / CI mode](#non-interactive--ci-mode)And update the heading on line 156 to use a regular hyphen as well.
Committable suggestion skipped: line range outside the PR's diff.
README.md-365-365 (1)
365-365: Use markdown link format for email address.The bare URL/email should use proper markdown formatting.
-Have you discovered a vulnerability or have concerns about model output? Please e‑mail **[email protected]** and we will respond promptly. +Have you discovered a vulnerability or have concerns about model output? Please e‑mail **[[email protected]](mailto:[email protected])** and we will respond promptly.axilo-cli/src/components/singlepass-cli-app.tsx-446-478 (1)
446-478: File operation errors are silently swallowed.All
catchblocks inapplyFileOpsdiscard errors without logging. This can mask partial failures (e.g., permission denied, disk full) and leave the user thinking all changes applied successfully.Consider logging errors or collecting them to display a summary:
} catch { - /* ignore */ + // eslint-disable-next-line no-console + console.error(`[applyFileOps] Failed to delete: ${op.path}`); }Committable suggestion skipped: line range outside the PR's diff.
axilo-cli/src/components/chat/multiline-editor.tsx-240-248 (1)
240-248: Dead code inisCtrlEdetection.The condition on lines 245-248 is contradictory: if
input === "e", theninput.charCodeAt(0)is 101 (ASCII for 'e'), not 5 (Ctrl+E). This branch can never be true.const isCtrlE = (key.ctrl && (input === "e" || input === "\x05")) || - input === "\x05" || - (!key.ctrl && - input === "e" && - input.length === 1 && - input.charCodeAt(0) === 5); + input === "\x05";axilo-cli/src/components/chat/terminal-chat-response-item.tsx-245-256 (1)
245-256: GlobalsetOptionsmay cause race conditions with concurrent renders.The
markedlibrary'ssetOptionsmodifies global state. If multipleMarkdowncomponents render simultaneously with different terminal widths, they could interfere with each other. Also,size.rowsis in the dependency array but unused.Consider using
marked.parsewith inline options or creating a dedicated parser instance:const rendered = React.useMemo(() => { - // Configure marked for this specific render - setOptions({ - // @ts-expect-error missing parser, space props - renderer: new TerminalRenderer({ ...options, width: size.columns }), - }); - const parsed = parse(children, { async: false }).trim(); + const renderer = new TerminalRenderer({ ...options, width: size.columns }); + // @ts-expect-error missing parser, space props + const parsed = parse(children, { async: false, renderer }).trim(); return parsed; - // eslint-disable-next-line react-hooks/exhaustive-deps -- options is an object of primitives - }, [children, size.columns, size.rows]); + }, [children, size.columns]);.github/actions/people/app/main.py-622-635 (1)
622-635: PotentialKeyErrorif maintainer has no GitHub activity.If a login in
maintainers_loginsdoesn't appear inauthors(e.g., the maintainer hasn't commented on any discussions or PRs), line 626 will raise aKeyError.Consider adding a guard:
maintainers = [] for login in maintainers_logins: + if login not in authors: + logging.warning(f"Maintainer {login} not found in authors") + continue user = authors[login]Committable suggestion skipped: line range outside the PR's diff.
axilo-cli/src/components/chat/terminal-chat-input.tsx-104-120 (1)
104-120: Modulo operation can produce negative index with shift+tab.When
_key.shiftis true andsis 0, the calculation(s + (-1)) % (suggestions.length + 1)results in-1 % 4 = -1, which is a negative index. This will causesuggestions[-2]to beundefinedwhen accessed later.Apply this diff to handle the wrap-around correctly:
if (_key.tab) { + const len = suggestions.length + 1; setSelectedSuggestion( - (s) => (s + (_key.shift ? -1 : 1)) % (suggestions.length + 1), + (s) => (s + (_key.shift ? len - 1 : 1)) % len, );
🧹 Nitpick comments (54)
axilo-cli/examples/prompting_guide.md (3)
85-89: Fix hyphenation for compound adjective.Line 85 uses "longer form input" which should be hyphenated as "longer-form input" when modifying a noun.
Apply this diff:
-For more complex tasks requiring longer form input, you can write the instructions as a file on your local machine: +For more complex tasks requiring longer-form input, you can write the instructions as a file on your local machine:
91-91: Strengthen phrasing on feedback loop.Line 91 could be more direct by replacing "give feedback" with "provide feedback."
Apply this diff:
-If Axilo doesn't get it right on the first try, give feedback to fix when you're in interactive mode! +If Axilo doesn't get it right on the first try, provide feedback to fix when you're in interactive mode!
113-113: Remove redundant phrase.Line 113 contains a redundancy: "heavy lifting up front" can be simplified to "heavy lifting."
Apply this diff:
-Axilo can be surprisingly self-sufficient for bigger tasks where your preference might be for the agent to do some heavy lifting up front, and allow you to refine its work later. +Axilo can be surprisingly self-sufficient for bigger tasks where your preference might be for the agent to do some heavy lifting, and allow you to refine its work later..github/workflows/dco.yml (3)
2-2: Use block syntax for the event trigger to match YAML formatting conventions.For consistency with the rest of the workflow and standard GitHub Actions practices, consider using block syntax instead of the inline flow syntax:
-on: [pull_request] +on: + - pull_request
23-26: Add-rflag toreadand quote variables to fix shellcheck warnings.The
readcommand without the-rflag can mangle backslashes in input (SC2162), and the loop should quote variables for robustness:-unsigned=$(git log --format='%h %s' "$base..$head" | while read sha _; do - git show -s --format='%B' "$sha" | grep -qi '^Signed-off-by:' || echo "$sha" +unsigned=$(git log --format='%h %s' "$base..$head" | while read -r sha _; do + git show -s --format='%B' "$sha" | grep -qi '^Signed-off-by:' || echo "$sha" done)
28-50: Quote variables in shell output for consistency and to avoid word-splitting issues.Consistently quote environment variables in echo statements, even when word-splitting is unlikely (SC2086). This follows shell best practices:
if [ -n "$unsigned" ]; then echo "::error ::❌ DCO check failed." - echo "## ❌ DCO Check Failed" >> $GITHUB_STEP_SUMMARY + echo "## ❌ DCO Check Failed" >> "$GITHUB_STEP_SUMMARY" echo "" echo "Commits missing the 'Signed-off-by:' footer:" echo "$unsigned" echo "" echo "🛠 **Quick fix (make ONE signed commit):**" echo " git fetch origin" - git reset --soft origin/${GITHUB_BASE_REF} + git reset --soft "origin/${GITHUB_BASE_REF}" echo " git commit -s -m \"<your message>\"" echo " git push --force-with-lease" echo "" echo "🔧 **Fix individual commits:**" - git rebase -i origin/${GITHUB_BASE_REF} --exec \"git commit --amend -s --no-edit\" + git rebase -i "origin/${GITHUB_BASE_REF}" --exec \"git commit --amend -s --no-edit\" echo " git push --force-with-lease" echo "" echo "💡 Or edit the commit message in GitHub UI and add:" echo " Signed-off-by: Your Name <[email protected]>" exit 1 else - echo "## ✅ DCO Check Passed" >> $GITHUB_STEP_SUMMARY + echo "## ✅ DCO Check Passed" >> "$GITHUB_STEP_SUMMARY" echo "All commits are properly signed off!" >> $GITHUB_STEP_SUMMARY.prettierrc.toml (1)
1-5: Consider alphabetically sorting configuration properties for consistency.The configuration properties are in mixed order (printWidth, quoteProps, semi, tabWidth, trailingComma). Sorting them alphabetically improves scannability and consistency.
-printWidth = 80 -quoteProps = "consistent" -semi = true -tabWidth = 2 -trailingComma = "all" +printWidth = 80 +quoteProps = "consistent" +semi = true +tabWidth = 2 +trailingComma = "all"(Note: If property order affects Prettier's config parsing, verify before applying.)
axilo-cli/src/components/onboarding/onboarding-approval-mode.tsx (2)
7-8: Misplaced TODO comment.This TODO references
cli-spinnersand Node v20.9.0, but this component doesn't appear to use spinners. Consider moving this comment to the relevant file or removing if no longer applicable.
14-16: PlaceholderonChangehandler does nothing.The
onChange={() => {}}means user selections have no effect. The commented-out code suggests intended behavior withonReviewCommand, butReviewDecisionandonReviewCommandaren't defined in this component's scope.Would you like me to help implement the
onChangehandler, or should this be tracked as a follow-up issue?axilo-cli/src/components/vendor/ink-select/use-select-state.js (1)
143-147: Verifyoptionsdependency is necessary in the effect.The
useEffectincludesoptionsin its dependency array, butoptionsisn't used in the effect body—onlystate.value,state.previousValue, andonChangeare referenced. This could cause unnecessaryonChangecalls when options change. Consider removingoptionsfrom the dependencies.useEffect(() => { if (state.value && state.previousValue !== state.value) { onChange?.(state.value); } - }, [state.previousValue, state.value, options, onChange]); + }, [state.previousValue, state.value, onChange]);axilo-cli/.editorconfig (1)
7-9: Remove redundant section.Lines 7-9 duplicate the indentation rules already defined in the
[*]section (lines 3-5). Since all files inherit these settings, the JS/TS-specific section is unnecessary.Apply this diff to remove the redundancy:
root = true [*] indent_style = space indent_size = 2 - -[*.{js,ts,jsx,tsx}] -indent_style = space -indent_size = 2axilo-cli/.nvmrc (1)
1-1: Consider using a more flexible version specifier.The exact version
22.0.0prevents automatic patch updates. Using22orlts/ironwould allow nvm to use the latest patch version of Node.js 22.x, which is safer for security updates.Apply this diff:
-22.0.0 +22.github/dependabot.yml (1)
16-17: Double-check Dependabot team assignee formatDependabot’s
assigneesusually expect GitHub usernames ororg/teamslugs without the@. The value"@neopilotai/team"may not resolve correctly; consider using"neopilotai/team"or individual usernames, and confirm against the current Dependabot docs.Also applies to: 36-37
.husky/pre-push (1)
4-5: Use portable newlines with/bin/shWith
#!/usr/bin/env sh, many shells will print\nliterally forecho "\n…"instead of inserting a blank line. For more portable output, prefer eitherprintf '\n🔍 Running pre-push checks...\n'(and similar for other messages) or a separateechofor blank lines.Also applies to: 8-9, 13-14, 17-18, 21-21
axilo-cli/.dockerignore (1)
4-5: Verify interaction between.dockerignoreand your DockerfileIgnoring
package-lock.json/yarn.lockandDockerfileis fine only if youraxilo-cli/Dockerfilenever copies them explicitly. If the Dockerfile doesCOPY package-lock.json .(or similar), these patterns will cause build failures or non-deterministic installs. Consider allowing lockfiles (and Dockerfile) into the build context, or adjust the Dockerfile accordingly.Also applies to: 36-38, 40-47
axilo-cli/require-shim.js (1)
10-11: Avoid overwriting an existing globalrequireIf a runtime or test harness already defines
globalThis.require, this will silently replace it. To be a safer shim, only install it when missing:-import { createRequire } from "module"; -globalThis.require = createRequire(import.meta.url); +import { createRequire } from "module"; + +if (typeof globalThis.require === "undefined") { + globalThis.require = createRequire(import.meta.url); +}axilo-cli/src/components/chat/terminal-chat-past-rollout.tsx (1)
51-57: Prefer stable keys foritems.mapif availableUsing the array index as
keyworks but can cause unnecessary remounts ifitemsare reordered or spliced. IfResponseItemexposes a stable identifier (e.g.,item.id), consider using that instead ofkeyhere..github/workflows/issue-manager.yml (1)
30-30: Pin the action to a specific version or commit SHA.Using
@masterfor the action reference can lead to unexpected breaking changes. Consider pinning to a specific version tag or commit SHA for stability.- - uses: khulnasoft/issue-manager@master + - uses: khulnasoft/[email protected] # or specific SHA.github/workflows/label-approved.yml (1)
17-17: Pin the action to a specific version or commit SHA.Using
@masterfor the action reference can lead to unexpected breaking changes. Consider pinning to a specific version tag or commit SHA for stability.- - uses: khulnasoft/label-approved@master + - uses: khulnasoft/[email protected] # or specific SHAaxilo-cli/examples/camerascii/run.sh (1)
26-27: Add error handling for the YAML parsing pipeline.The
yqandjqpipeline could fail if../task.yamlis missing or malformed. Consider adding error handling to provide a clear failure message.Apply this diff to add error handling:
# Grab task name for logging -task_name=$(yq -o=json '.' ../task.yaml | jq -r '.name') +task_name=$(yq -o=json '.' ../task.yaml 2>/dev/null | jq -r '.name' 2>/dev/null) || { + echo "Error: Failed to read task name from ../task.yaml" + exit 1 +} echo "Checking for runs for task: $task_name"axilo-cli/.eslintrc.cjs (1)
42-43: Address the FIXME comment about explicit function return types.The commented-out rule
@typescript-eslint/explicit-function-return-typewould improve type safety. Consider enabling it if the codebase is ready for explicit return type annotations.Do you want me to open an issue to track enabling this rule and updating the codebase to comply?
axilo-cli/src/components/chat/use-message-grouping.ts (1)
46-75: Consider refactoring the duplicated grouping logic.Lines 46-72 contain repetitive code for detecting and grouping function_call items at different positions in
lastFew. The logic can be simplified to reduce duplication.Consider this refactored approach:
const batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }> = []; // Find the first function_call in the last three items const firstFuncCallIdx = lastFew.findIndex(item => item?.type === 'function_call'); if (firstFuncCallIdx !== -1) { const toolCall = parseToolCall(lastFew[firstFuncCallIdx]); batch.push({ group: { label: toolCall?.autoApproval?.group || "Running command", items: lastFew.slice(firstFuncCallIdx), }, }); // Add any items before the function_call individually for (let i = 0; i < firstFuncCallIdx; i++) { batch.push({ item: lastFew[i] }); } } else { lastFew.forEach((item) => batch.push({ item })); }axilo-cli/src/components/history-overlay.tsx (1)
169-173: Consider reformatting the nested ternary for readability.The nested ternary with
Array.isArraychecks is functional but could be clearer with explicit formatting or early returns.- const cmdArray: Array<string> | undefined = Array.isArray(argsObj?.["cmd"]) - ? (argsObj!["cmd"] as Array<string>) - : Array.isArray(argsObj?.["command"]) - ? (argsObj!["command"] as Array<string>) - : undefined; + const cmdArray: Array<string> | undefined = + Array.isArray(argsObj?.["cmd"]) + ? (argsObj!["cmd"] as Array<string>) + : Array.isArray(argsObj?.["command"]) + ? (argsObj!["command"] as Array<string>) + : undefined;axilo-cli/examples/prompt-analyzer/template/cluster_prompts.py (3)
1-1: Script is not executable but has shebang.The static analysis indicates the shebang is present but the file is not executable. Either remove the shebang or make the file executable.
chmod +x axilo-cli/examples/prompt-analyzer/template/cluster_prompts.py
165-165: Addstrict=Truetozip()to catch length mismatches.If the embedding API unexpectedly returns fewer vectors than requested, silent truncation would corrupt the cache mapping. Using
strict=Truesurfaces this issue immediately.- cache.update(dict(zip(texts_to_embed, new_embeddings))) + cache.update(dict(zip(texts_to_embed, new_embeddings, strict=True)))
448-448: Remove unusedscattervariable.The
scattervariable is assigned but never used. Remove the assignment or use_as a placeholder if the return value is intentionally discarded.- scatter = plt.scatter(xy[:, 0], xy[:, 1], c=labels, cmap="tab20", s=20, alpha=0.8) + plt.scatter(xy[:, 0], xy[:, 1], c=labels, cmap="tab20", s=20, alpha=0.8)axilo-cli/scripts/build_container.sh (1)
1-3: Add error handling for robustness.The script lacks
set -ewhich means failures in thedocker buildcommand won't cause the script to exit with an error code. This can mask build failures in CI pipelines.#!/bin/bash +set -euo pipefail docker build -t axilo -f axilo-cli/Dockerfile axilo-cli.github/actions/people/requirements.txt (1)
1-15: Consider updating pinned dependencies.Most dependencies are pinned to versions from mid-2023. While no other security issues were flagged, consider updating to receive bug fixes and improvements. Using a range (like
pyyaml>=5.3.1,<6.0.0) for more packages could ease maintenance..github/workflows/people.yml (1)
38-49: Consider removing redundant Python setup.The workflow runs in a container (
${{ env.DOCKER_IMAGE }}) that likely already has Python 3.11 installed, but then runssetup-python@v4andpip installagain. This is redundant if the container image includes all necessary dependencies. Consider either:
- Removing the container and using the Python setup actions directly on the runner, or
- Removing the Python setup steps and ensuring the container has all dependencies pre-installed
axilo-cli/examples/prompt-analyzer/template/README.md (1)
53-53: Standardize Markdown emphasis style.The emphasis uses underscores
_(none)_but should use asterisks*(none)*for consistency with Markdown best practices.Apply this diff:
-| `--cache` | _(none)_ | embedding cache path (JSON). Speeds up repeated runs – new texts are appended automatically. | +| `--cache` | *(none)* | embedding cache path (JSON). Speeds up repeated runs – new texts are appended automatically. |axilo-cli/src/cli_singlepass.tsx (2)
39-39: Clarify the purpose of the empty default export.The empty default export
export default {};appears unnecessary since the module already exports a named functionrunSinglePass. Unless this serves a specific purpose (e.g., module compatibility or avoiding default export side effects), consider removing it.-export default {};
17-23: Minor: Non-standard hyphen character in comments.The comment uses non-breaking hyphens (‑) instead of regular hyphens (-) in "long‑running", "render‑options", and "Ctrl+C". While this doesn't affect functionality, standard ASCII hyphens are more conventional in code comments for consistency.
axilo-cli/src/components/chat/message-history.tsx (1)
14-22: Unused props in component signature.
groupCounts,items,userMsgCount,confirmationPrompt, andloadingare defined inMessageHistoryPropsbut not destructured or used in the component. This suggests either incomplete implementation or props that should be removed.Either implement usage for these props or remove them from the interface:
type MessageHistoryProps = { batch: Array<BatchEntry>; - groupCounts: Record<string, number>; - items: Array<ResponseItem>; - userMsgCount: number; - confirmationPrompt: React.ReactNode; - loading: boolean; headerProps: TerminalHeaderProps; };axilo-cli/src/components/chat/terminal-chat-command-review.tsx (2)
98-102: Inconsistent deny message for 'n' shortcut.Line 101 uses
"Don't do that, keep going though"butDEFAULT_DENY_MESSAGEon line 12-13 is"Don't do that, but keep trying to fix the problem". This creates inconsistent messaging between pressing 'n' directly vs. submitting empty input.Use the constant for consistency:
} else if (input === "n") { onReviewCommand( ReviewDecision.NO_CONTINUE, - "Don't do that, keep going though", + DEFAULT_DENY_MESSAGE, );
34-51: Props inspection via type assertion is pragmatic but fragile.The approach of inspecting
confirmationPrompt.props.commandForDisplayworks but couples this component tightly to the internal structure ofTerminalChatToolCallCommand. If that component's props change, this logic breaks silently.Consider passing
commandForDisplaydirectly as a prop toTerminalChatCommandReviewfor explicit dependency:export function TerminalChatCommandReview({ confirmationPrompt, commandForDisplay, // Add explicit prop onReviewCommand, }: { confirmationPrompt: React.ReactNode; commandForDisplay?: string; onReviewCommand: (decision: ReviewDecision, customMessage?: string) => void; }): React.ReactElement { const showAlwaysApprove = React.useMemo(() => { if (!commandForDisplay) return true; const baseCmd = commandForDisplay.split("\n")[0]?.trim().split(/\s+/)[0] ?? ""; return baseCmd !== "apply_patch"; }, [commandForDisplay]);axilo-cli/examples/impossible-pong/template/index.html (2)
141-142: Consider clamping ball position after wall bounce.The current wall bounce logic reverses velocity but doesn't constrain the ball's position. With high velocities (especially in "insane" mode with acceleration), the ball could get stuck outside the canvas boundaries.
Apply this diff to clamp the ball position:
// Wall bounce - if (ball.y < 0 || ball.y > canvas.height) ball.vy *= -1; + if (ball.y - ballRadius < 0 || ball.y + ballRadius > canvas.height) { + ball.vy *= -1; + ball.y = Math.max(ballRadius, Math.min(canvas.height - ballRadius, ball.y)); + }
144-161: Consider preventing rapid repeated paddle collisions.The collision detection works correctly, but if the ball gets stuck inside a paddle hitbox (due to high velocity or timing), it could trigger multiple bounces per frame.
Consider adding a cooldown or checking that ball.vx has the correct sign before bouncing:
// Paddle collision let paddle = ball.x < canvas.width / 2 ? player : ai; if ( ball.x - ballRadius < paddle.x + paddleWidth && ball.x + ballRadius > paddle.x && ball.y > paddle.y && - ball.y < paddle.y + paddleHeight + ball.y < paddle.y + paddleHeight && + ((paddle === player && ball.vx < 0) || (paddle === ai && ball.vx > 0)) ) { ball.vx *= -1;This ensures the ball is moving toward the paddle before bouncing.
axilo-cli/scripts/init_firewall.sh (2)
27-28: Consider adding more allowed domains or simplifying the loop.The loop currently processes only one domain (
api.openai.com). The backslash continuation and loop structure suggest this was designed for multiple domains.Either add additional required domains:
for domain in \ - "api.openai.com"; do + "api.openai.com" \ + "other-required-domain.com"; doOr simplify to a direct assignment if only one domain is needed:
-for domain in \ - "api.openai.com"; do +domain="api.openai.com" echo "Resolving $domain..." ips=$(dig +short A "$domain") ... -done
90-96: Clarify comment to match the actual domain being verified.The comment mentions "KhulnaSoft API" while the code checks
api.openai.com. This inconsistency could cause confusion.Apply this diff for clarity:
-# Verify KhulnaSoft API access +# Verify KhulnaSoft API (api.openai.com) access if ! curl --connect-timeout 5 https://api.openai.com >/dev/null 2>&1; thenAlternatively, if these are distinct services, use consistent terminology.
axilo-cli/src/components/chat/terminal-chat-input-thinking.tsx (1)
7-65: Consider removing or re-enabling the commented thinking texts.The
thinkingTextsarray contains 50+ creative messages that are commented out, leaving only"Thinking"active. Commented code adds maintenance burden without providing functionality.Options:
- Remove the commented code if the feature was intentionally disabled
- Re-enable the messages if the variety adds value to the UX
- Move to a feature flag if you want to toggle this behavior
Do you want me to open an issue to track this decision?
axilo-cli/src/cli.tsx (1)
254-259: Ternary formatting could be clearer.The nested ternary is slightly misaligned. Consider aligning the
?and:operators consistently or usingif/elsefor readability.const approvalPolicy: ApprovalPolicy = cli.flags.fullAuto || cli.flags.approvalMode === "full-auto" ? AutoApprovalMode.FULL_AUTO - : cli.flags.autoEdit - ? AutoApprovalMode.AUTO_EDIT - : AutoApprovalMode.SUGGEST; + : cli.flags.autoEdit || cli.flags.approvalMode === "auto-edit" + ? AutoApprovalMode.AUTO_EDIT + : AutoApprovalMode.SUGGEST;Note: The
approvalMode === "auto-edit"case from the flag description isn't handled—if that's intentional, ignore this; otherwise, add support for it.axilo-cli/src/components/typeahead-overlay.tsx (1)
74-94: Sort stability concern when no query is present.Returning
0from a comparator doesn't guarantee stable sort across all JavaScript engines. To truly preserve the originalinitialItemsorder whenq.length === 0, you need to compare by original index.- const ranked = filtered.sort((a, b) => { + const ranked = [...filtered].sort((a, b) => { if (a.value === currentValue) { return -1; } if (b.value === currentValue) { return 1; } // Preserve original order when no query is present so we keep any caller // defined prioritisation (e.g. recommended models). if (q.length === 0) { - return 0; + // Use original index for stable ordering + return items.indexOf(a) - items.indexOf(b); } const ia = a.label.toLowerCase().indexOf(q); const ib = b.label.toLowerCase().indexOf(q); if (ia !== ib) { return ia - ib; } return a.label.localeCompare(b.label); });axilo-cli/src/components/chat/terminal-chat.tsx (1)
155-175: Dead code in timer cleanup else-branch.At line 165-167,
handleis alwaysnullin the else-branch because it's only assigned within the if-block. TheclearInterval(null)call is harmless but unnecessary.useEffect(() => { let handle: ReturnType<typeof setInterval> | null = null; // Only tick the "thinking…" timer when the agent is actually processing // a request *and* the user is not being asked to review a command. if (loading && confirmationPrompt == null) { setThinkingSeconds(0); handle = setInterval(() => { setThinkingSeconds((s) => s + 1); }, 1000); } else { - if (handle) { - clearInterval(handle); - } setThinkingSeconds(0); } return () => { if (handle) { clearInterval(handle); } }; }, [loading, confirmationPrompt]);axilo-cli/src/hooks/use-confirmation.ts (1)
50-55: Consider wrappingsubmitConfirmationinuseCallback.Unlike
requestConfirmation,submitConfirmationis not wrapped inuseCallback, meaning it's recreated on every render. This could cause unnecessary re-renders in components that use it as a dependency.- // Called whenever user picks Yes / No - const submitConfirmation = (result: ConfirmationResult) => { + // Called whenever user picks Yes / No + const submitConfirmation = useCallback((result: ConfirmationResult) => { if (current) { current.resolve(result); advanceQueue(); } - }; + }, [current, advanceQueue]);.github/actions/people/tests/integration/test_integration.py (2)
5-8: Unused imports.
MagicMockandGithubare imported but never used in this file.from pathlib import Path -from unittest.mock import patch, MagicMock +from unittest.mock import patch import pytest -from github import Github from app.main import (
221-222: Consider explicit encoding when reading file.For portability, specify the encoding explicitly when opening files.
- with open(output_file) as f: + with open(output_file, encoding="utf-8") as f: data = json.load(f)axilo-cli/src/components/vendor/ink-text-input.tsx (2)
64-66: Prefer strict equality (!==) over loose equality (!=).For consistency and to avoid potential type coercion issues, use strict inequality.
- if (lastMatch != 0) { + if (lastMatch !== 0) { lastMatch += 1; }
238-260: Consider reusingfindPrevWordJumpto reduce duplication.The logic for Meta+backspace/delete (lines 239-255) duplicates the regex-based word boundary search already implemented in
findPrevWordJump. Consolidating would improve maintainability.} else if (key.meta && (key.backspace || key.delete)) { - const regex = /[\s,.;!?]+/g; - let lastMatch = 0; - let currentMatch: RegExpExecArray | null; - - const stringToCursorOffset = originalValue - .slice(0, cursorOffset) - .replace(/[\s,.;!?]+$/, ""); - - // Loop through all matches - while ((currentMatch = regex.exec(stringToCursorOffset)) !== null) { - lastMatch = currentMatch.index; - } - - // Include the last match unless it is the first character - if (lastMatch != 0) { - lastMatch += 1; - } - - nextValue = - stringToCursorOffset.slice(0, lastMatch) + - originalValue.slice(cursorOffset, originalValue.length); - nextCursorOffset = lastMatch; + const jumpTo = findPrevWordJump(originalValue, cursorOffset); + nextValue = + originalValue.slice(0, jumpTo) + + originalValue.slice(cursorOffset); + nextCursorOffset = jumpTo; }axilo-cli/src/components/singlepass-cli-app.tsx (1)
33-59:localStorageis unavailable in Node.js — consider removing the dead code path.In a CLI/Node.js environment,
localStorageis never defined, so lines 35-40 and 63-68 are effectively dead code. The file-based fallback is always used. Removing the localStorage branches would simplify the code.axilo-cli/src/components/chat/multiline-editor.tsx (2)
363-364: Accessing private properties via type assertion is fragile.The code accesses
scrollRowandscrollColusingas any, which bypasses type safety and will break silently ifTextBuffer's internals change.Consider exposing these as part of
TextBuffer's public API (e.g.,getScrollPosition()) to make the contract explicit and maintainable.
27-34: Consider isolating test-only patches to a separate module.The
EventEmitter.prototypepatches are well-documented but pollute the global prototype. Consider moving them to a dedicated test setup file (e.g.,vitest.setup.ts) to keep production code clean and make the test-only nature explicit.axilo-cli/src/app.tsx (1)
37-37: Unnecessary function initializer for constant value.
useState(() => false)uses lazy initialization, which is only beneficial for expensive computations. For a simple boolean,useState(false)is cleaner and equivalent.- const [accepted, setAccepted] = useState(() => false); + const [accepted, setAccepted] = useState(false);.github/actions/people/app/main.py (1)
503-523: Dead code: commented-out issue expert aggregation.The
get_expertsfunction has extensive commented-out code for issue experts. If this migration is permanent, consider removing the dead code to improve readability. If it's temporary, add a TODO with context.axilo-cli/src/components/chat/terminal-chat-response-item.tsx (1)
127-142: Consider extracting content type mapping for clarity.The nested ternary chain works but is slightly hard to read. This is a minor style suggestion.
+const getContentText = (c: { type: string; text?: string; refusal?: string; filename?: string }): string => { + switch (c.type) { + case "output_text": + case "input_text": + return c.text ?? ""; + case "refusal": + return c.refusal ?? ""; + case "input_image": + return "<Image>"; + case "input_file": + return c.filename ?? ""; + default: + return ""; + } +};
| "from openai import KhulnaSoft\n", | ||
| "import os\n", | ||
| "\n", | ||
| "client = KhulnaSoft(api_key=os.environ.get(\"KHULNASOFT_API_KEY\", \"<your KhulnaSoft API key if not set as env var>\"))\n", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenAI Python client import OpenAI class December 2025
💡 Result:
Yes — the modern OpenAI Python client (v1+) uses the OpenAI class. Example:
- Install: pip install openai
- Import and use:
from openai import OpenAI
client = OpenAI()
resp = client.chat.completions.create(model="gpt-4o", messages=[{"role":"user","content":"Hello"}])
Sources: [1], [2]
Sources
[1] openai/openai-python (README & examples). GitHub.
[2] openai-quickstart-python (example quickstart). GitHub.
Fix the OpenAI import statement.
Line 154 shows from openai import KhulnaSoft which is incorrect. The correct import should be from openai import OpenAI, and line 157 should instantiate OpenAI() not KhulnaSoft(). Additionally, the environment variable name should be OPENAI_API_KEY not KHULNASOFT_API_KEY.
The corrected code cell should be:
from openai import OpenAI
import os
client = OpenAI(api_key=os.environ.get("OPENAI_API_KEY", "<your OpenAI API key if not set as env var>"))🤖 Prompt for AI Agents
In axilo-cli/examples/prompt-analyzer/template/Clustering.ipynb around lines 154
to 157, the import and client instantiation are wrong: replace "from openai
import KhulnaSoft" with "from openai import OpenAI", instantiate OpenAI()
instead of KhulnaSoft(), and use the environment variable OPENAI_API_KEY
(including updating the default placeholder string) when passing api_key via
os.environ.get.
|
|
||
| ## Reporting a Vulnerability | ||
|
|
||
| If you discover a security vulnerability in Axilo CLI, please report it to [email protected]. Please do not report security vulnerabilities through public GitHub issues. |
There was a problem hiding this comment.
Replace placeholder email address.
The email [email protected] is a placeholder and will prevent users from reporting security vulnerabilities. Replace it with a valid security contact email before merging.
🤖 Prompt for AI Agents
In axilo-cli/SECURITY.md around line 9, the security contact email is a
placeholder ([email protected]); replace it with the project's valid security
contact address (e.g., [email protected] or the official security team's
mailbox), update the line so the document points to the real reporting email,
and commit the change before merging.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: KhulnaSoft bot <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: KhulnaSoft bot <[email protected]>
Signed-off-by: KhulnaSoft bot <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (6)
axilo-cli/src/utils/agent/agent-loop.ts (1)
490-496: Model prefix check may match unintended models.
this.model.startsWith("o")will match any model starting with "o" (e.g., hypothetical "other-model"). Consider using a more explicit check.- if (this.model.startsWith("o")) { + if (/^o[34]-/.test(this.model)) { reasoning = { effort: "high" }; if (this.model === "o3-mini" || this.model === "o4-mini") {axilo-cli/src/utils/agent/apply-patch.ts (1)
79-79:DiffErrorshould callsuperwith the message.The
DiffErrorclass extendsErrorbut relies on implicit super call behavior. For proper error handling and stack traces, explicitly pass the message.-export class DiffError extends Error {} +export class DiffError extends Error { + constructor(message: string) { + super(message); + this.name = 'DiffError'; + } +}axilo-cli/src/utils/model-utils.ts (2)
14-38: Model cache is never invalidated.The
modelsPromiseis set once and never cleared. For long-running CLI sessions, this could result in stale model lists if models are added/removed from the account. Consider adding a TTL or cache invalidation mechanism.let modelsPromise: Promise<Array<string>> | null = null; +let modelsCacheTime: number = 0; +const MODELS_CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes async function fetchModels(): Promise<Array<string>> { // ...existing code... } export async function getAvailableModels(): Promise<Array<string>> { - if (!modelsPromise) { + const now = Date.now(); + if (!modelsPromise || (now - modelsCacheTime > MODELS_CACHE_TTL_MS)) { modelsPromise = fetchModels(); + modelsCacheTime = now; } return modelsPromise; }
35-37: Silent failure on fetch error may hide configuration issues.When
fetchModelscatches an error, it returns an empty array without logging. This could mask issues like invalid API keys or network problems that the user should know about.Consider logging the error when logging is enabled:
} catch (err) { + if (isLoggingEnabled()) { + log(`Failed to fetch models: ${err instanceof Error ? err.message : String(err)}`); + } return []; }Note: You'll need to import
logandisLoggingEnabledfrom./agent/log.js.axilo-cli/src/utils/config.ts (2)
150-158: Non-cryptographic hash for key fingerprint is weak.The djb2-style hash produces only ~8 hex characters and is not collision-resistant. While the comment acknowledges this, consider using a proper cryptographic hash if this fingerprint is used for any security-sensitive logging or auditing.
+import { createHash } from 'crypto'; + private getKeyFingerprint(): string { - // Simple hash function - in production, consider using a proper cryptographic hash - let hash = 0; - for (let i = 0; i < this.apiKey.length; i++) { - const char = this.apiKey.charCodeAt(i); - hash = ((hash << 5) - hash) + char; - hash = hash & hash; // Convert to 32bit integer - } - return `fp_${Math.abs(hash).toString(16).substring(0, 8)}`; + if (!this.apiKey) return 'fp_none'; + const hash = createHash('sha256').update(this.apiKey).digest('hex'); + return `fp_${hash.substring(0, 8)}`; }
224-259: Symlink traversal during directory tree walk should be protected withfs.realpathSync().The function uses
dirname()for traversal, which doesn't follow symlinks, butexistsSync()does. IfstartDiror any intermediate directory is a symlink, it could traverse outside the intended hierarchy. Wrap the initialresolvePath()withfs.realpathSync()to resolve symlinks before traversal, ensuring the walk stays within the real filesystem structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.husky/commit-msg(1 hunks)axilo-cli/Dockerfile(1 hunks)axilo-cli/src/cli.tsx(1 hunks)axilo-cli/src/utils/agent/agent-loop.ts(1 hunks)axilo-cli/src/utils/agent/apply-patch.ts(1 hunks)axilo-cli/src/utils/config.ts(1 hunks)axilo-cli/src/utils/config.ts.bak(1 hunks)axilo-cli/src/utils/model-utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- axilo-cli/src/cli.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- .husky/commit-msg
🧰 Additional context used
🧬 Code graph analysis (2)
axilo-cli/src/utils/model-utils.ts (1)
axilo-cli/src/utils/config.ts (1)
apiKeyManager(167-167)
axilo-cli/src/utils/agent/agent-loop.ts (1)
axilo-cli/src/utils/config.ts (4)
AppConfig(208-214)KHULNASOFT_TIMEOUT_MS(34-35)apiKeyManager(167-167)KHULNASOFT_BASE_URL(36-36)
🔇 Additional comments (5)
axilo-cli/Dockerfile (1)
67-68: The review comment references code that does not exist in the actual Dockerfile. Lines 67-68 in the current Dockerfile are an ENTRYPOINT directive and a comment, not the tarball copy operations mentioned. The actual Dockerfile correctly copies dist directories directly from the builder stage (lines 43-44) and runs the CLI vianode /app/packages/cli/dist/index.js(line 69), which is the proper approach for this monorepo setup. No tarball generation occurs in the build process—pnpm buildproduces standard dist directories that are directly copied and executed. This review comment does not apply to the current code.Likely an incorrect or invalid review comment.
axilo-cli/src/utils/agent/apply-patch.ts (1)
25-56: LGTM onassemble_changesfunction.The logic correctly handles ADD, UPDATE, and DELETE cases, and properly skips unchanged files.
axilo-cli/src/utils/model-utils.ts (1)
60-91: Well-designed graceful degradation pattern.The timeout race with fallback to
trueensures startup isn't blocked by slow network or API issues. The logic correctly handles undefined/empty models and uses RECOMMENDED_MODELS as a fast path.axilo-cli/src/utils/config.ts (1)
313-459: Config loading and bootstrap logic is well-structured.The
loadConfigfunction handles JSON/YAML fallback gracefully, merges project docs with instructions, and bootstraps first-run defaults without blocking on errors. The separation of concerns is clean.axilo-cli/src/utils/agent/agent-loop.ts (1)
480-482: The code referenced in this review comment does not exist in the repository.The file
axilo-cli/src/utils/agent/agent-loop.tscannot be found. Additionally, the typesResponseInputItemandResponseItem, the functionstageItem, and the variableturnInputdo not appear anywhere in the codebase. The repository uses a monorepo structure underpackages/rather than anaxilo-cli/directory.This review comment appears to reference a different codebase or an outdated version of the code.
Likely an incorrect or invalid review comment.
| RUN addgroup -S appuser && adduser -S appuser -G appuser | ||
| RUN chown -R appuser:appuser /app | ||
| USER appuser |
There was a problem hiding this comment.
Contradictory USER statements; clarify intended runtime user.
The Dockerfile switches users multiple times:
- Line 50–52: Create
appuserand setUSER appuser - Line 60:
USER node(overrides appuser) - Line 72:
USER root(for firewall sudoers setup) - Line 76:
USER node(final runtime user)
This is confusing. Clarify which user should run the application at runtime. If the app should run as appuser, then:
- Avoid switching to
nodeon line 60 - After firewall setup (line 75), switch back to
appuserinstead ofnode
If node is the intended user, consider consolidating setup to use node throughout (except where root is explicitly needed for sudoers/firewall configuration).
-USER appuser
-...
-CMD ["node", "dist/cli.js"]
-USER node
+USER appuser
+...
+CMD ["node", "dist/cli.js"](or adjust consistently depending on your intended runtime user)
Also applies to: 60-60, 72-72, 76-76
🤖 Prompt for AI Agents
axilo-cli/Dockerfile lines 50-52, 60, 72, 76: the Dockerfile currently switches
users multiple times (creates appuser and sets USER appuser, then switches to
node, back to root, then node), which is contradictory—choose a single intended
runtime user and make the USER usage consistent; if runtime should be appuser,
remove the USER node at line 60, ensure any steps needing node are run as root
or appuser, and after the root-only firewall/sudoers setup (around line 72)
switch back to USER appuser instead of node (also update chown to match appuser
if necessary); if runtime should be node, create/use node throughout (omit
creating appuser or change chown to node) and only switch to root for
firewall/sudoers tasks then back to USER node; in short, consolidate the user
switches to a single runtime user and update chown/ownership and the USER lines
accordingly so there are no conflicting USER declarations.
| CMD ["node", "dist/cli.js"] | ||
| USER node | ||
|
|
||
| # Install global packages | ||
| ENV NPM_CONFIG_PREFIX=/usr/local/share/npm-global | ||
| ENV PATH=$PATH:/usr/local/share/npm-global/bin | ||
|
|
||
| # Install axilo | ||
| COPY --from=builder /app/dist/axilo.tgz ./axilo.tgz | ||
| RUN npm install -g axilo.tgz | ||
|
|
||
| # Copy and set up firewall script | ||
| COPY scripts/init_firewall.sh /usr/local/bin/ | ||
| USER root | ||
| RUN chmod +x /usr/local/bin/init_firewall.sh && \ | ||
| echo "node ALL=(root) NOPASSWD: /usr/local/bin/init_firewall.sh" > /etc/sudoers.d/node-firewall && \ | ||
| chmod 0440 /etc/sudoers.d/node-firewall | ||
| USER node No newline at end of file |
There was a problem hiding this comment.
Move all setup instructions before CMD; instructions after CMD are semantically confusing.
In Docker, instructions placed after CMD execute during the build phase but violate conventional Dockerfile structure. All setup (ENV, COPY, RUN, USER switches) should precede the CMD instruction, which should be the final or near-final line. Reorganize the Dockerfile to consolidate lines 60–76 before line 59 (CMD).
Additionally, the multiple USER switches (appuser → node → root → node) are contradictory. Clarify the intended runtime user.
Suggested restructure (pseudocode):
- Lines 49–52: Create non-root user (appuser) ✓
- Before line 59 (CMD):
- Set up NPM_CONFIG_PREFIX and PATH for global installs
- Install global packages (axilo)
- Copy firewall script and set permissions (as root)
- Return to intended runtime user
- Line 59: CMD ["node", "dist/cli.js"]
(no instructions after CMD)
Do you want me to propose a complete restructure?
🤖 Prompt for AI Agents
In axilo-cli/Dockerfile around lines 59 to 76, several build and permission
setup steps (ENV, PATH, npm global install, COPY of axilo.tgz, firewall script
copy/chmod and sudoers entry) appear after the CMD which should be the final
runtime instruction; also the multiple USER switches (appuser → node → root →
node) are inconsistent. Move all ENV, PATH, npm install, COPY and RUN (firewall
script + sudoers) steps to before the CMD, set NPM_CONFIG_PREFIX and update PATH
prior to running npm install, perform installations and file permission changes
as root (or escalate only for those commands), then switch to the intended
non-root runtime user once at the end before emitting CMD
["node","dist/cli.js"]; consolidate and remove unnecessary USER toggles and
document which non-root user is intended for runtime.
| customDenyMessage?: string; | ||
| }; | ||
|
|
||
| const alreadyProcessedResponses = new Set(); |
There was a problem hiding this comment.
Module-level Set may leak memory across sessions.
alreadyProcessedResponses is a module-scoped Set that accumulates processed response IDs but is never cleared. Over long-running CLI sessions or repeated invocations, this could grow unbounded and cause memory pressure.
Consider either:
- Moving the set into
AgentLoopas an instance property and clearing it interminate(), or - Using a bounded LRU cache if cross-instance deduplication is truly needed.
🤖 Prompt for AI Agents
In axilo-cli/src/utils/agent/agent-loop.ts around line 31, the module-level Set
alreadyProcessedResponses currently lives for the process lifetime and can leak
memory; move this Set into the AgentLoop class as an instance property (e.g.,
this.alreadyProcessedResponses) so it is scoped per AgentLoop instance, replace
all uses of the module variable with the instance property, and clear it in the
AgentLoop.terminate() method (or reinitialize it) to release memory;
alternatively, if deduplication must span instances, replace the Set with a
bounded LRU cache implementation to cap growth.
| // If we have *not* seen any function_call IDs yet there is nothing that | ||
| // needs to be satisfied in a follow‑up request. In that case we clear | ||
| // the stored lastResponseId so a subsequent run starts a clean turn. | ||
| if (this.pendingAborts.size === 0) { | ||
| try { | ||
| this.onLastResponseId(""); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| } | ||
|
|
||
| // NOTE: We intentionally do *not* clear `lastResponseId` here. If the | ||
| // stream produced a `function_call` before the user cancelled, KhulnaSoft now | ||
| // expects a corresponding `function_call_output` that must reference that | ||
| // very same response ID. We therefore keep the ID around so the | ||
| // follow‑up request can still satisfy the contract. | ||
|
|
||
| // If we have *not* seen any function_call IDs yet there is nothing that | ||
| // needs to be satisfied in a follow‑up request. In that case we clear | ||
| // the stored lastResponseId so a subsequent run starts a clean turn. | ||
| if (this.pendingAborts.size === 0) { | ||
| try { | ||
| this.onLastResponseId(""); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| } | ||
|
|
||
| // NOTE: We intentionally do *not* clear `lastResponseId` here. If the | ||
| // stream produced a `function_call` before the user cancelled, KhulnaSoft now | ||
| // expects a corresponding `function_call_output` that must reference that | ||
| // very same response ID. We therefore keep the ID around so the | ||
| // follow‑up request can still satisfy the contract. |
There was a problem hiding this comment.
Duplicated code block for clearing lastResponseId.
Lines 126-135 and 143-152 contain identical logic for checking pendingAborts.size === 0 and calling this.onLastResponseId(""). The associated comment blocks (lines 137-141 and 154-158) are also duplicated. This appears to be an accidental copy-paste.
Apply this diff to remove the duplicate:
// If we have *not* seen any function_call IDs yet there is nothing that
// needs to be satisfied in a follow‑up request. In that case we clear
// the stored lastResponseId so a subsequent run starts a clean turn.
if (this.pendingAborts.size === 0) {
try {
this.onLastResponseId("");
} catch {
/* ignore */
}
}
// NOTE: We intentionally do *not* clear `lastResponseId` here. If the
// stream produced a `function_call` before the user cancelled, KhulnaSoft now
// expects a corresponding `function_call_output` that must reference that
// very same response ID. We therefore keep the ID around so the
// follow‑up request can still satisfy the contract.
-
- // If we have *not* seen any function_call IDs yet there is nothing that
- // needs to be satisfied in a follow‑up request. In that case we clear
- // the stored lastResponseId so a subsequent run starts a clean turn.
- if (this.pendingAborts.size === 0) {
- try {
- this.onLastResponseId("");
- } catch {
- /* ignore */
- }
- }
-
- // NOTE: We intentionally do *not* clear `lastResponseId` here. If the
- // stream produced a `function_call` before the user cancelled, KhulnaSoft now
- // expects a corresponding `function_call_output` that must reference that
- // very same response ID. We therefore keep the ID around so the
- // follow‑up request can still satisfy the contract.
this.onLoading(false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If we have *not* seen any function_call IDs yet there is nothing that | |
| // needs to be satisfied in a follow‑up request. In that case we clear | |
| // the stored lastResponseId so a subsequent run starts a clean turn. | |
| if (this.pendingAborts.size === 0) { | |
| try { | |
| this.onLastResponseId(""); | |
| } catch { | |
| /* ignore */ | |
| } | |
| } | |
| // NOTE: We intentionally do *not* clear `lastResponseId` here. If the | |
| // stream produced a `function_call` before the user cancelled, KhulnaSoft now | |
| // expects a corresponding `function_call_output` that must reference that | |
| // very same response ID. We therefore keep the ID around so the | |
| // follow‑up request can still satisfy the contract. | |
| // If we have *not* seen any function_call IDs yet there is nothing that | |
| // needs to be satisfied in a follow‑up request. In that case we clear | |
| // the stored lastResponseId so a subsequent run starts a clean turn. | |
| if (this.pendingAborts.size === 0) { | |
| try { | |
| this.onLastResponseId(""); | |
| } catch { | |
| /* ignore */ | |
| } | |
| } | |
| // NOTE: We intentionally do *not* clear `lastResponseId` here. If the | |
| // stream produced a `function_call` before the user cancelled, KhulnaSoft now | |
| // expects a corresponding `function_call_output` that must reference that | |
| // very same response ID. We therefore keep the ID around so the | |
| // follow‑up request can still satisfy the contract. | |
| // If we have *not* seen any function_call IDs yet there is nothing that | |
| // needs to be satisfied in a follow‑up request. In that case we clear | |
| // the stored lastResponseId so a subsequent run starts a clean turn. | |
| if (this.pendingAborts.size === 0) { | |
| try { | |
| this.onLastResponseId(""); | |
| } catch { | |
| /* ignore */ | |
| } | |
| } | |
| // NOTE: We intentionally do *not* clear `lastResponseId` here. If the | |
| // stream produced a `function_call` before the user cancelled, KhulnaSoft now | |
| // expects a corresponding `function_call_output` that must reference that | |
| // very same response ID. We therefore keep the ID around so the | |
| // follow‑up request can still satisfy the contract. | |
| this.onLoading(false); |
🤖 Prompt for AI Agents
In axilo-cli/src/utils/agent/agent-loop.ts around lines 126 to 158 there is an
accidental duplicate of the same logic and comment that clears lastResponseId
when pendingAborts.size === 0; remove the second copy (the repeated if-block and
its duplicated NOTE comment — i.e. the duplicated lines starting at the second
if-check through the second NOTE comment) so only one instance of the check,
call to this.onLastResponseId(""), and explanatory NOTE remains.
| setSessionId(this.sessionId); | ||
| setCurrentModel(this.model); | ||
|
|
||
| this.hardAbort = new AbortController(); |
There was a problem hiding this comment.
Reassignment of readonly property will throw at runtime.
hardAbort is declared as private readonly hardAbort = new AbortController(); on line 96, but line 264 attempts to reassign it in the constructor. This will cause a TypeError at runtime because you cannot assign to a readonly property.
Remove the redundant assignment since hardAbort is already initialized at declaration:
- this.hardAbort = new AbortController();
-
this.hardAbort.signal.addEventListener(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.hardAbort = new AbortController(); | |
| this.hardAbort.signal.addEventListener( |
🤖 Prompt for AI Agents
In axilo-cli/src/utils/agent/agent-loop.ts around line 264, the constructor
reassigns the private readonly field hardAbort (this.hardAbort = new
AbortController()), which will throw at runtime; remove that redundant
assignment from the constructor so the field remains initialized only at
declaration (or if you need to recreate it, change the field declaration to
non-readonly and update usages accordingly).
| function write_file(p: string, content: string): void { | ||
| if (path.isAbsolute(p)) { | ||
| throw new DiffError("We do not support absolute paths."); | ||
| } | ||
| const parent = path.dirname(p); | ||
| if (parent !== ".") { | ||
| fs.mkdirSync(parent, { recursive: true }); | ||
| } | ||
| fs.writeFileSync(p, content, "utf8"); | ||
| } |
There was a problem hiding this comment.
write_file lacks path traversal protection present in remove_file.
The remove_file function properly validates that the resolved path stays within process.cwd(), but write_file only checks for absolute paths. A relative path like ../../etc/malicious would bypass this check and write outside the intended directory.
Apply consistent security checks as done in remove_file:
function write_file(p: string, content: string): void {
if (path.isAbsolute(p)) {
throw new DiffError("We do not support absolute paths.");
}
+
+ // Resolve the path to prevent directory traversal
+ const resolvedPath = path.resolve(p);
+ const currentDir = process.cwd();
+
+ // Ensure the resolved path is within the current working directory
+ if (!resolvedPath.startsWith(currentDir + path.sep) && resolvedPath !== currentDir) {
+ throw new DiffError("Path traversal detected");
+ }
+
const parent = path.dirname(p);
if (parent !== ".") {
fs.mkdirSync(parent, { recursive: true });
}
- fs.writeFileSync(p, content, "utf8");
+ fs.writeFileSync(resolvedPath, content, "utf8");
}🤖 Prompt for AI Agents
In axilo-cli/src/utils/agent/apply-patch.ts around lines 600 to 609, write_file
currently only rejects absolute paths and thus allows path traversal via "../"
to write outside the repository; update write_file to mirror remove_file's
safety: resolve the target path against process.cwd(), verify the resolved path
starts with process.cwd() (or is equal), and throw a DiffError if it does not,
then create parent directories and write the file using the resolved path; keep
the original error message semantics and UTF-8 write.
|
|
||
| // Ensure the resolved path is within the current working directory | ||
| if (!resolvedPath.startsWith(currentDir)) { | ||
| throw new Error('Path traversal detected'); | ||
| } |
There was a problem hiding this comment.
Path traversal check has edge case with startsWith.
The check !resolvedPath.startsWith(currentDir) can have false positives. For example, if currentDir is /home/user and resolvedPath is /home/username/file, it would incorrectly pass. Append a path separator to the comparison.
- if (!resolvedPath.startsWith(currentDir)) {
+ if (!resolvedPath.startsWith(currentDir + path.sep) && resolvedPath !== currentDir) {
throw new Error('Path traversal detected');
}🤖 Prompt for AI Agents
In axilo-cli/src/utils/agent/apply-patch.ts around lines 620 to 624, the path
traversal check uses startsWith(currentDir) which can false-positive for
directories like /home/user vs /home/username; update the check to compare using
a canonicalized directory path with a trailing path separator (e.g., normalize
and resolve currentDir, ensure it ends with path.sep) and then verify
resolvedPath startsWith that directory-with-separator (or alternatively use
path.relative and confirm it does not begin with '..' or escape the directory).
| private checkRateLimit(): void { | ||
| const now = Date.now(); | ||
|
|
||
| // Reset counter if we're in a new rate limit window | ||
| if (now - this.lastUsed > this.RATE_LIMIT_WINDOW) { | ||
| this.usageCount = 0; | ||
| this.lastUsed = now; | ||
| } | ||
|
|
||
| this.usageCount++; | ||
|
|
||
| if (this.usageCount > this.RATE_LIMIT) { | ||
| const timeLeft = Math.ceil((this.lastUsed + this.RATE_LIMIT_WINDOW - now) / 1000); | ||
| throw new Error(`API rate limit exceeded. Please try again in ${timeLeft} seconds.`); | ||
| } | ||
| } | ||
|
|
||
| public getApiKey(): string | null { | ||
| if (!this.isSet) { | ||
| return null; | ||
| } | ||
|
|
||
| this.checkRateLimit(); | ||
| this.logKeyUsage('api_key_accessed'); | ||
|
|
||
| return this.apiKey; | ||
| } |
There was a problem hiding this comment.
Rate limiting on getApiKey() is misplaced and will break API calls.
The rate limit check throws an exception when exceeded, but it's applied to getApiKey() calls rather than actual API requests. This means:
- If any code calls
getApiKey()frequently (e.g., for logging, validation, or display), it will consume rate limit quota - When the limit is exceeded,
getApiKey()throws, likely crashing the calling code that may not expect an exception from a getter - The rate limit doesn't correspond to actual API usage since
getApiKey()may be called without making an API call
Either remove the rate limiting from getApiKey() (let the OpenAI SDK handle API rate limits) or make it a warning rather than an exception:
public getApiKey(): string | null {
if (!this.isSet) {
return null;
}
- this.checkRateLimit();
+ // Track usage for metrics, but don't throw - let the API handle its own limits
+ this.trackUsage();
this.logKeyUsage('api_key_accessed');
return this.apiKey;
}
+
+ private trackUsage(): void {
+ const now = Date.now();
+ if (now - this.lastUsed > this.RATE_LIMIT_WINDOW) {
+ this.usageCount = 0;
+ this.lastUsed = now;
+ }
+ this.usageCount++;
+ }🤖 Prompt for AI Agents
In axilo-cli/src/utils/config.ts around lines 94 to 120, the rate-limit check is
incorrectly applied inside getApiKey(): this causes non-API operations that
merely access the key to consume quota and throw exceptions; remove the call to
checkRateLimit() (and optional logKeyUsage rate increment) from getApiKey() so
that returning the stored key is a simple, non-throwing getter, or alternatively
replace checkRateLimit() invocation with a non-throwing warning/log (no
exception) and do not increment usageCount here; move actual rate limiting to
the code paths that perform network/API requests (or rely on the OpenAI SDK) so
only actual API calls affect quota.
| export const saveConfig = ( | ||
| config: AppConfig, | ||
| configPath = CONFIG_FILEPATH, | ||
| instructionsPath = INSTRUCTIONS_FILEPATH, | ||
| ): void => { | ||
| // If the caller passed the default JSON path *and* a YAML config already | ||
| // exists on disk, save back to that YAML file instead to preserve the | ||
| // user's chosen format. | ||
| let targetPath = configPath; | ||
| if ( | ||
| configPath === CONFIG_FILEPATH && | ||
| !existsSync(configPath) && | ||
| (existsSync(CONFIG_YAML_FILEPATH) || existsSync(CONFIG_YML_FILEPATH)) | ||
| ) { | ||
| targetPath = existsSync(CONFIG_YAML_FILEPATH) | ||
| ? CONFIG_YAML_FILEPATH | ||
| : CONFIG_YML_FILEPATH; | ||
| } | ||
|
|
||
| const dir = dirname(targetPath); | ||
| if (!existsSync(dir)) { | ||
| mkdirSync(dir, { recursive: true }); | ||
| } | ||
|
|
||
| const ext = extname(targetPath).toLowerCase(); | ||
| if (ext === ".yaml" || ext === ".yml") { | ||
| writeFileSync(targetPath, dumpYaml({ model: config.model }), "utf-8"); | ||
| } else { | ||
| writeFileSync( | ||
| targetPath, | ||
| JSON.stringify({ model: config.model }, null, 2), | ||
| "utf-8", | ||
| ); | ||
| } | ||
|
|
||
| writeFileSync(instructionsPath, config.instructions, "utf-8"); | ||
| }; |
There was a problem hiding this comment.
saveConfig only persists model, discarding other config fields.
The saveConfig function only writes { model: config.model }, ignoring approvalMode, fullAutoErrorMode, and memory fields that are part of StoredConfig. This could cause data loss if users expect round-trip persistence.
Persist all relevant configuration fields:
const ext = extname(targetPath).toLowerCase();
if (ext === ".yaml" || ext === ".yml") {
- writeFileSync(targetPath, dumpYaml({ model: config.model }), "utf-8");
+ const stored: StoredConfig = { model: config.model };
+ if (config.memory) stored.memory = config.memory;
+ if (config.fullAutoErrorMode) stored.fullAutoErrorMode = config.fullAutoErrorMode;
+ writeFileSync(targetPath, dumpYaml(stored), "utf-8");
} else {
- writeFileSync(
- targetPath,
- JSON.stringify({ model: config.model }, null, 2),
- "utf-8",
- );
+ const stored: StoredConfig = { model: config.model };
+ if (config.memory) stored.memory = config.memory;
+ if (config.fullAutoErrorMode) stored.fullAutoErrorMode = config.fullAutoErrorMode;
+ writeFileSync(targetPath, JSON.stringify(stored, null, 2), "utf-8");
}🤖 Prompt for AI Agents
In axilo-cli/src/utils/config.ts around lines 461 to 497, saveConfig currently
serializes only { model: config.model } which drops other StoredConfig fields
(approvalMode, fullAutoErrorMode, memory, etc.); change the serialization to
persist the complete stored configuration object (e.g., serialize the full
StoredConfig or spread the relevant fields) when writing YAML or JSON so all
config properties are round-tripped intact (use the same object for dumpYaml and
JSON.stringify and keep writing instructionsPath as-is).
There was a problem hiding this comment.
In axilo-cli/src/utils/config.ts around lines 461 to 497, saveConfig currently
serializes only { model: config.model } which drops other StoredConfig fields
(approvalMode, fullAutoErrorMode, memory, etc.); change the serialization to
persist the complete stored configuration object (e.g., serialize the full
StoredConfig or spread the relevant fields) when writing YAML or JSON so all
config properties are round-tripped intact (use the same object for dumpYaml and
JSON.stringify and keep writing instructionsPath as-is).
There was a problem hiding this comment.
@khulnasoft-bot, I'll provide the complete fix for the saveConfig function to persist all available StoredConfig fields:
export const saveConfig = (
config: AppConfig,
configPath = CONFIG_FILEPATH,
instructionsPath = INSTRUCTIONS_FILEPATH,
): void => {
// If the caller passed the default JSON path *and* a YAML config already
// exists on disk, save back to that YAML file instead to preserve the
// user's chosen format.
let targetPath = configPath;
if (
configPath === CONFIG_FILEPATH &&
!existsSync(configPath) &&
(existsSync(CONFIG_YAML_FILEPATH) || existsSync(CONFIG_YML_FILEPATH))
) {
targetPath = existsSync(CONFIG_YAML_FILEPATH)
? CONFIG_YAML_FILEPATH
: CONFIG_YML_FILEPATH;
}
const dir = dirname(targetPath);
if (!existsSync(dir)) {
mkdirSync(dir, { recursive: true });
}
// Build the StoredConfig object with all available fields
const stored: StoredConfig = { model: config.model };
if (config.memory !== undefined) {
stored.memory = config.memory;
}
if (config.fullAutoErrorMode !== undefined) {
stored.fullAutoErrorMode = config.fullAutoErrorMode;
}
const ext = extname(targetPath).toLowerCase();
if (ext === ".yaml" || ext === ".yml") {
writeFileSync(targetPath, dumpYaml(stored), "utf-8");
} else {
writeFileSync(targetPath, JSON.stringify(stored, null, 2), "utf-8");
}
writeFileSync(instructionsPath, config.instructions, "utf-8");
};Key changes:
- Created a
storedobject of typeStoredConfiginstead of an inline object literal - Conditionally added
memoryandfullAutoErrorModefields when they're present - Used the same
storedobject for both YAML and JSON serialization - Used
!== undefinedchecks to preserve explicitfalsevalues for boolean fields
Note: The approvalMode field from StoredConfig cannot be persisted because it doesn't exist in the AppConfig type that saveConfig receives. If approval mode persistence is needed, the function signature would need to be updated to accept it as a parameter or AppConfig would need to include it.
| // NOTE: We intentionally point the TypeScript import at the source file | ||
| // (`./auto-approval-mode.ts`) instead of the emitted `.js` bundle. This makes | ||
| // the module resolvable when the project is executed via `ts-node`, which | ||
| // resolves *source* paths rather than built artefacts. During a production | ||
| // build the TypeScript compiler will automatically rewrite the path to | ||
| // `./auto-approval-mode.js`, so the change is completely transparent for the | ||
| // compiled `dist/` output used by the published CLI. | ||
|
|
||
| import type { FullAutoErrorMode } from "./auto-approval-mode.js"; | ||
|
|
||
| import { log, isLoggingEnabled } from "./agent/log.js"; | ||
| import { AutoApprovalMode } from "./auto-approval-mode.js"; | ||
| import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; | ||
| import { load as loadYaml, dump as dumpYaml } from "js-yaml"; | ||
| import { homedir } from "os"; | ||
| import { dirname, join, extname, resolve as resolvePath } from "path"; | ||
|
|
||
| export const DEFAULT_AGENTIC_MODEL = "o4-mini"; | ||
| export const DEFAULT_FULL_CONTEXT_MODEL = "gpt-4.1"; | ||
| export const DEFAULT_APPROVAL_MODE = AutoApprovalMode.SUGGEST; | ||
| export const DEFAULT_INSTRUCTIONS = ""; | ||
|
|
||
| export const CONFIG_DIR = join(homedir(), ".axilo"); | ||
| export const CONFIG_JSON_FILEPATH = join(CONFIG_DIR, "config.json"); | ||
| export const CONFIG_YAML_FILEPATH = join(CONFIG_DIR, "config.yaml"); | ||
| export const CONFIG_YML_FILEPATH = join(CONFIG_DIR, "config.yml"); | ||
|
|
||
| // Keep the original constant name for backward compatibility, but point it at | ||
| // the default JSON path. Code that relies on this constant will continue to | ||
| // work unchanged. | ||
| export const CONFIG_FILEPATH = CONFIG_JSON_FILEPATH; | ||
| export const INSTRUCTIONS_FILEPATH = join(CONFIG_DIR, "instructions.md"); | ||
|
|
||
| export const KHULNASOFT_TIMEOUT_MS = | ||
| parseInt(process.env["KHULNASOFT_TIMEOUT_MS"] || "0", 10) || undefined; | ||
| export const KHULNASOFT_BASE_URL = process.env["KHULNASOFT_BASE_URL"] || ""; | ||
| export let KHULNASOFT_API_KEY = process.env["KHULNASOFT_API_KEY"] || ""; | ||
|
|
||
| export function setApiKey(apiKey: string): void { | ||
| KHULNASOFT_API_KEY = apiKey; | ||
| } | ||
|
|
||
| // Formatting (quiet mode-only). | ||
| export const PRETTY_PRINT = Boolean(process.env["PRETTY_PRINT"] || ""); | ||
|
|
||
| // Represents config as persisted in config.json. | ||
| export type StoredConfig = { | ||
| model?: string; | ||
| approvalMode?: AutoApprovalMode; | ||
| fullAutoErrorMode?: FullAutoErrorMode; | ||
| memory?: MemoryConfig; | ||
| }; | ||
|
|
||
| // Minimal config written on first run. An *empty* model string ensures that | ||
| // we always fall back to DEFAULT_MODEL on load, so updates to the default keep | ||
| // propagating to existing users until they explicitly set a model. | ||
| export const EMPTY_STORED_CONFIG: StoredConfig = { model: "" }; | ||
|
|
||
| // Pre‑stringified JSON variant so we don’t stringify repeatedly. | ||
| const EMPTY_CONFIG_JSON = JSON.stringify(EMPTY_STORED_CONFIG, null, 2) + "\n"; | ||
|
|
||
| export type MemoryConfig = { | ||
| enabled: boolean; | ||
| }; | ||
|
|
||
| // Represents full runtime config, including loaded instructions | ||
| export type AppConfig = { | ||
| apiKey?: string; | ||
| model: string; | ||
| instructions: string; | ||
| fullAutoErrorMode?: FullAutoErrorMode; | ||
| memory?: MemoryConfig; | ||
| }; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Project doc support (axilo.md) | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| export const PROJECT_DOC_MAX_BYTES = 32 * 1024; // 32 kB | ||
|
|
||
| const PROJECT_DOC_FILENAMES = ["axilo.md", ".axilo.md", "AXILO.md"]; | ||
|
|
||
| export function discoverProjectDocPath(startDir: string): string | null { | ||
| const cwd = resolvePath(startDir); | ||
|
|
||
| // 1) Look in the explicit CWD first: | ||
| for (const name of PROJECT_DOC_FILENAMES) { | ||
| const direct = join(cwd, name); | ||
| if (existsSync(direct)) { | ||
| return direct; | ||
| } | ||
| } | ||
|
|
||
| // 2) Fallback: walk up to the Git root and look there | ||
| let dir = cwd; | ||
| // eslint-disable-next-line no-constant-condition | ||
| while (true) { | ||
| const gitPath = join(dir, ".git"); | ||
| if (existsSync(gitPath)) { | ||
| // Once we hit the Git root, search its top‑level for the doc | ||
| for (const name of PROJECT_DOC_FILENAMES) { | ||
| const candidate = join(dir, name); | ||
| if (existsSync(candidate)) { | ||
| return candidate; | ||
| } | ||
| } | ||
| // If Git root but no doc, stop looking | ||
| return null; | ||
| } | ||
|
|
||
| const parent = dirname(dir); | ||
| if (parent === dir) { | ||
| // Reached filesystem root without finding Git | ||
| return null; | ||
| } | ||
| dir = parent; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Load the project documentation markdown (axilo.md) if present. If the file | ||
| * exceeds {@link PROJECT_DOC_MAX_BYTES} it will be truncated and a warning is | ||
| * logged. | ||
| * | ||
| * @param cwd The current working directory of the caller | ||
| * @param explicitPath If provided, skips discovery and loads the given path | ||
| */ | ||
| export function loadProjectDoc(cwd: string, explicitPath?: string): string { | ||
| let filepath: string | null = null; | ||
|
|
||
| if (explicitPath) { | ||
| filepath = resolvePath(cwd, explicitPath); | ||
| if (!existsSync(filepath)) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(`axilo: project doc not found at ${filepath}`); | ||
| filepath = null; | ||
| } | ||
| } else { | ||
| filepath = discoverProjectDocPath(cwd); | ||
| } | ||
|
|
||
| if (!filepath) { | ||
| return ""; | ||
| } | ||
|
|
||
| try { | ||
| const buf = readFileSync(filepath); | ||
| if (buf.byteLength > PROJECT_DOC_MAX_BYTES) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
| `axilo: project doc '${filepath}' exceeds ${PROJECT_DOC_MAX_BYTES} bytes – truncating.`, | ||
| ); | ||
| } | ||
| return buf.slice(0, PROJECT_DOC_MAX_BYTES).toString("utf-8"); | ||
| } catch { | ||
| return ""; | ||
| } | ||
| } | ||
|
|
||
| // (Receives params for testing) | ||
| export type LoadConfigOptions = { | ||
| /** Working directory used for project doc discovery */ | ||
| cwd?: string; | ||
| /** Disable inclusion of the project doc */ | ||
| disableProjectDoc?: boolean; | ||
| /** Explicit path to project doc (overrides discovery) */ | ||
| projectDocPath?: string; | ||
| /** Whether we are in fullcontext mode. */ | ||
| isFullContext?: boolean; | ||
| }; | ||
|
|
||
| export const loadConfig = ( | ||
| configPath: string | undefined = CONFIG_FILEPATH, | ||
| instructionsPath: string | undefined = INSTRUCTIONS_FILEPATH, | ||
| options: LoadConfigOptions = {}, | ||
| ): AppConfig => { | ||
| // Determine the actual path to load. If the provided path doesn't exist and | ||
| // the caller passed the default JSON path, automatically fall back to YAML | ||
| // variants. | ||
| let actualConfigPath = configPath; | ||
| if (!existsSync(actualConfigPath)) { | ||
| if (configPath === CONFIG_FILEPATH) { | ||
| if (existsSync(CONFIG_YAML_FILEPATH)) { | ||
| actualConfigPath = CONFIG_YAML_FILEPATH; | ||
| } else if (existsSync(CONFIG_YML_FILEPATH)) { | ||
| actualConfigPath = CONFIG_YML_FILEPATH; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let storedConfig: StoredConfig = {}; | ||
| if (existsSync(actualConfigPath)) { | ||
| const raw = readFileSync(actualConfigPath, "utf-8"); | ||
| const ext = extname(actualConfigPath).toLowerCase(); | ||
| try { | ||
| if (ext === ".yaml" || ext === ".yml") { | ||
| storedConfig = loadYaml(raw) as unknown as StoredConfig; | ||
| } else { | ||
| storedConfig = JSON.parse(raw); | ||
| } | ||
| } catch { | ||
| // If parsing fails, fall back to empty config to avoid crashing. | ||
| storedConfig = {}; | ||
| } | ||
| } | ||
|
|
||
| const instructionsFilePathResolved = | ||
| instructionsPath ?? INSTRUCTIONS_FILEPATH; | ||
| const userInstructions = existsSync(instructionsFilePathResolved) | ||
| ? readFileSync(instructionsFilePathResolved, "utf-8") | ||
| : DEFAULT_INSTRUCTIONS; | ||
|
|
||
| // Project doc ----------------------------------------------------------- | ||
| const shouldLoadProjectDoc = | ||
| !options.disableProjectDoc && | ||
| process.env["AXILO_DISABLE_PROJECT_DOC"] !== "1"; | ||
|
|
||
| let projectDoc = ""; | ||
| let projectDocPath: string | null = null; | ||
| if (shouldLoadProjectDoc) { | ||
| const cwd = options.cwd ?? process.cwd(); | ||
| projectDoc = loadProjectDoc(cwd, options.projectDocPath); | ||
| projectDocPath = options.projectDocPath | ||
| ? resolvePath(cwd, options.projectDocPath) | ||
| : discoverProjectDocPath(cwd); | ||
| if (projectDocPath) { | ||
| if (isLoggingEnabled()) { | ||
| log( | ||
| `[axilo] Loaded project doc from ${projectDocPath} (${projectDoc.length} bytes)`, | ||
| ); | ||
| } | ||
| } else { | ||
| if (isLoggingEnabled()) { | ||
| log(`[axilo] No project doc found in ${cwd}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const combinedInstructions = [userInstructions, projectDoc] | ||
| .filter((s) => s && s.trim() !== "") | ||
| .join("\n\n--- project-doc ---\n\n"); | ||
|
|
||
| // Treat empty string ("" or whitespace) as absence so we can fall back to | ||
| // the latest DEFAULT_MODEL. | ||
| const storedModel = | ||
| storedConfig.model && storedConfig.model.trim() !== "" | ||
| ? storedConfig.model.trim() | ||
| : undefined; | ||
|
|
||
| const config: AppConfig = { | ||
| model: | ||
| storedModel ?? | ||
| (options.isFullContext | ||
| ? DEFAULT_FULL_CONTEXT_MODEL | ||
| : DEFAULT_AGENTIC_MODEL), | ||
| instructions: combinedInstructions, | ||
| }; | ||
|
|
||
| // ----------------------------------------------------------------------- | ||
| // First‑run bootstrap: if the configuration file (and/or its containing | ||
| // directory) didn't exist we create them now so that users end up with a | ||
| // materialised ~/.axilo/config.json file on first execution. This mirrors | ||
| // what `saveConfig()` would do but without requiring callers to remember to | ||
| // invoke it separately. | ||
| // | ||
| // We intentionally perform this *after* we have computed the final | ||
| // `config` object so that we can just persist the resolved defaults. The | ||
| // write operations are guarded by `existsSync` checks so that subsequent | ||
| // runs that already have a config will remain read‑only here. | ||
| // ----------------------------------------------------------------------- | ||
|
|
||
| try { | ||
| if (!existsSync(actualConfigPath)) { | ||
| // Ensure the directory exists first. | ||
| const dir = dirname(actualConfigPath); | ||
| if (!existsSync(dir)) { | ||
| mkdirSync(dir, { recursive: true }); | ||
| } | ||
|
|
||
| // Persist a minimal config – we include the `model` key but leave it as | ||
| // an empty string so that `loadConfig()` treats it as "unset" and falls | ||
| // back to whatever DEFAULT_MODEL is current at runtime. This prevents | ||
| // pinning users to an old default after upgrading Axilo. | ||
| const ext = extname(actualConfigPath).toLowerCase(); | ||
| if (ext === ".yaml" || ext === ".yml") { | ||
| writeFileSync(actualConfigPath, dumpYaml(EMPTY_STORED_CONFIG), "utf-8"); | ||
| } else { | ||
| writeFileSync(actualConfigPath, EMPTY_CONFIG_JSON, "utf-8"); | ||
| } | ||
| } | ||
|
|
||
| // Always ensure the instructions file exists so users can edit it. | ||
| if (!existsSync(instructionsFilePathResolved)) { | ||
| const instrDir = dirname(instructionsFilePathResolved); | ||
| if (!existsSync(instrDir)) { | ||
| mkdirSync(instrDir, { recursive: true }); | ||
| } | ||
| writeFileSync(instructionsFilePathResolved, userInstructions, "utf-8"); | ||
| } | ||
| } catch { | ||
| // Silently ignore any errors – failure to persist the defaults shouldn't | ||
| // block the CLI from starting. A future explicit `axilo config` command | ||
| // or `saveConfig()` call can handle (re‑)writing later. | ||
| } | ||
|
|
||
| // Only include the "memory" key if it was explicitly set by the user. This | ||
| // preserves backward‑compatibility with older config files (and our test | ||
| // fixtures) that don't include a "memory" section. | ||
| if (storedConfig.memory !== undefined) { | ||
| config.memory = storedConfig.memory; | ||
| } | ||
|
|
||
| if (storedConfig.fullAutoErrorMode) { | ||
| config.fullAutoErrorMode = storedConfig.fullAutoErrorMode; | ||
| } | ||
|
|
||
| return config; | ||
| }; | ||
|
|
||
| export const saveConfig = ( | ||
| config: AppConfig, | ||
| configPath = CONFIG_FILEPATH, | ||
| instructionsPath = INSTRUCTIONS_FILEPATH, | ||
| ): void => { | ||
| // If the caller passed the default JSON path *and* a YAML config already | ||
| // exists on disk, save back to that YAML file instead to preserve the | ||
| // user's chosen format. | ||
| let targetPath = configPath; | ||
| if ( | ||
| configPath === CONFIG_FILEPATH && | ||
| !existsSync(configPath) && | ||
| (existsSync(CONFIG_YAML_FILEPATH) || existsSync(CONFIG_YML_FILEPATH)) | ||
| ) { | ||
| targetPath = existsSync(CONFIG_YAML_FILEPATH) | ||
| ? CONFIG_YAML_FILEPATH | ||
| : CONFIG_YML_FILEPATH; | ||
| } | ||
|
|
||
| const dir = dirname(targetPath); | ||
| if (!existsSync(dir)) { | ||
| mkdirSync(dir, { recursive: true }); | ||
| } | ||
|
|
||
| const ext = extname(targetPath).toLowerCase(); | ||
| if (ext === ".yaml" || ext === ".yml") { | ||
| writeFileSync(targetPath, dumpYaml({ model: config.model }), "utf-8"); | ||
| } else { | ||
| writeFileSync( | ||
| targetPath, | ||
| JSON.stringify({ model: config.model }, null, 2), | ||
| "utf-8", | ||
| ); | ||
| } | ||
|
|
||
| writeFileSync(instructionsPath, config.instructions, "utf-8"); | ||
| }; |
There was a problem hiding this comment.
Backup file should not be committed.
The file config.ts.bak appears to be a backup version of config.ts. Backup files should not be committed to the repository as they create maintenance burden and confusion. If this is intentional for migration purposes, consider documenting it or removing after the migration is complete.
Remove this file from the repository:
git rm axilo-cli/src/utils/config.ts.bakIf you need to preserve history, consider using git history rather than keeping backup files in the tree.
🤖 Prompt for AI Agents
axilo-cli/src/utils/config.ts.bak lines 1-356: this is a backup file that should
not be committed; remove it from the repository with git rm
axilo-cli/src/utils/config.ts.bak (or git rm --cached ... if you must keep it
locally), commit the removal, and add an appropriate rule (e.g. *.bak or
axilo-cli/src/utils/*.bak) to .gitignore to prevent future backups from being
committed.
2 participants
PR Type
Enhancement, Tests
Description
Implements comprehensive agent loop orchestration with streaming responses, retry logic, and function call execution management
Adds Unicode-aware text buffer with full editing capabilities (insert, delete, undo/redo, selection, clipboard)
Introduces patch parsing and application system with fuzzy matching for file operations
Implements command approval and safety assessment system with three approval modes (suggest, auto-edit, full-auto)
Adds configuration management with YAML/JSON support and project documentation discovery
Implements file content caching with LRU eviction and directory traversal utilities
Adds command execution handler with approval policy enforcement and sandbox management
Implements macOS Seatbelt sandbox policy enforcement for secure command execution
Adds raw command execution with process group management and signal handling
Implements unified diff generation with ANSI colorization and statistics
Adds terminal chat utilities for context window calculations and message deduplication
Implements agent logging infrastructure with async file persistence
Adds model availability checking and caching utilities
Implements session tracking with globally unique IDs and metadata management
Adds comprehensive test suite covering agent cancellation, termination, network errors, rate limiting, and edge cases
Includes tests for text buffer operations, patch application, external editor integration, and configuration loading
Adds regression tests for function call ID propagation, process group termination, and cancellation race conditions
Diagram Walkthrough
File Walkthrough
29 files
agent-loop.ts
Agent loop orchestration with streaming and error handlingaxilo-cli/src/utils/agent/agent-loop.ts
CLI agentic assistant
for transient errors
approval workflows
tracking to handle stray events
text-buffer.ts
Unicode-aware text buffer with editing and historyaxilo-cli/src/lib/text-buffer.ts
viewport management
deletion) with undo/redo
openInExternalEditor()forgit-style workflows
terminal UI
apply-patch.ts
Patch parsing and application with fuzzy matchingaxilo-cli/src/utils/agent/apply-patch.ts
diff format support
with fuzzy matching
filesystem
approvals.ts
Command approval and safety assessment systemaxilo-cli/src/lib/approvals.ts
patches
sandbox constraints
path restrictions
config.ts
Configuration management with project doc discoveryaxilo-cli/src/utils/config.ts
YAML/JSON support
runtime config
~/.axilo
context_files.ts
File content caching and directory traversal utilitiesaxilo-cli/src/utils/singlepass/context_files.ts
validation
glob patterns
invalidation
handle-exec-command.ts
Command execution with approval and sandbox managementaxilo-cli/src/utils/agent/handle-exec-command.ts
deriveCommandKey()environments
command classes
parsers.ts
Tool call parsing and command safety validationaxilo-cli/src/utils/parsers.ts
testing
context_limit.ts
Context size tracking and hierarchical breakdown reportingaxilo-cli/src/utils/singlepass/context_limit.ts
visualization
raw-exec.ts
Raw command execution with process group and signal managementaxilo-cli/src/utils/agent/sandbox/raw-exec.ts
(ripgrep workaround)
child processes
AbortSignal
signals
code_diff.ts
Unified diff generation with ANSI colorization and statisticsaxilo-cli/src/utils/singlepass/code_diff.ts
difflibrary with 5-line contextdeletions)
macos-seatbelt.ts
macOS Seatbelt sandbox policy enforcementaxilo-cli/src/utils/agent/sandbox/macos-seatbelt.ts
~/.pyenvand config directoryterminal-chat-utils.ts
Terminal chat utilities for deduplication and context trackingaxilo-cli/src/components/chat/terminal-chat-utils.ts
calculations and message deduplication
maxTokensForModel()returns context length estimates based on modelname
uniqueById()deduplicates response items by ID and collapsesconsecutive identical user messages
calculateContextPercentRemaining()computes remaining contextpercentage for UI display
log.ts
Agent logging system with async file persistenceaxilo-cli/src/utils/agent/log.ts
persistence
AsyncLoggerqueues and asynchronously writes log entries totimestamped files
axilo-cli-latest.logfor convenient tailingEmptyLoggerprovides no-op implementation whenDEBUGenv var is notset
parse-apply-patch.ts
Patch parsing for file operation extractionaxilo-cli/src/utils/agent/parse-apply-patch.ts
operations
*** Begin Patchand*** End Patchmarkers
counts
terminal.ts
Terminal renderer lifecycle and cleanup managementaxilo-cli/src/utils/terminal.ts
setInkRenderer()configures renderer with optional FPS debuggingonExit()ensures proper unmounting and terminal state restorationuse-message-grouping.ts
Message grouping hook for display batchingaxilo-cli/src/components/chat/use-message-grouping.ts
window
model-utils.ts
Model availability checking and caching utilitiesaxilo-cli/src/utils/model-utils.ts
getAvailableModels()fetches and caches model list from/modelsendpoint
isModelSupportedForResponses()validates model support with timeoutand fallback
preloadModels()initiates background fetch during CLI startupexec.ts
Command execution with sandbox routingaxilo-cli/src/utils/agent/exec.ts
handling
execWithSeatbelt(macOS) orrawExec(other platforms)execApplyPatch()applies patch operations with error handlinggetBaseCmd()extracts base command name for loggingcontext.ts
Task context data structures and renderingaxilo-cli/src/utils/singlepass/context.ts
TaskContextincludes prompt, file paths, directory structure, and filecontents
renderTaskContext()formats context as XML-like structure with CDATAsections
format-command.ts
Command formatting for user displayaxilo-cli/src/lib/format-command.ts
escaping
bash -lcwrapper to show actual commandshell-quotefor complex commandsuse-confirmation.ts
Confirmation prompt queue management hookaxilo-cli/src/hooks/use-confirmation.ts
requestConfirmation()andsubmitConfirmation()for async flowsave-rollout.ts
Conversation rollout persistence with debouncingaxilo-cli/src/utils/storage/save-rollout.ts
~/.axilo/sessions/items
approximate-tokens-used.ts
Token usage approximation utilityaxilo-cli/src/utils/approximate-tokens-used.ts
heuristic
session.ts
Session tracking and metadata managementaxilo-cli/src/utils/session.ts
TerminalChatSessiontype with metadata (user, version,timestamp)
file_ops.ts
File operation schema definitionsaxilo-cli/src/utils/singlepass/file_ops.ts
FileOperationtype with mutually exclusive fields for differentoperation types
EditedFilescontainer for ordered list of operationscheck-in-git.ts
Git repository detection utilityaxilo-cli/src/utils/check-in-git.ts
git rev-parse --is-inside-work-treecommandinput-utils.ts
Input item creation with image supportaxilo-cli/src/utils/input-utils.ts
ResponseInputItem.Messagewith mixed content arrayshort-path.ts
Path shortening for displayaxilo-cli/src/utils/short-path.ts
shortenPath()replaces home directory with~and truncates long pathsshortCwd()applies shortening to current working directory30 files
text-buffer-crlf.test.ts
TextBuffer CRLF normalization testaxilo-cli/tests/text-buffer-crlf.test.ts
endings correctly
insertStr()method splits on both\rand\r\nsequencesapply-patch.test.ts
Comprehensive patch application and file operation testsaxilo-cli/tests/apply-patch.test.ts
text-buffer.test.ts
Text buffer editing and cursor movement test suiteaxilo-cli/tests/text-buffer.test.ts
column preservation
text-buffer-gaps.test.ts
Text buffer feature gap documentation and future testsaxilo-cli/tests/text-buffer-gaps.test.ts
.fails()marked tests for futureimplementation
search functionality
tab length
agent-thinking-time.test.ts
Agent thinking time counter regression test suiteaxilo-cli/tests/agent-thinking-time.test.ts
.fails()to track regression until implementation is fixedagent-cancel.test.ts
Agent cancellation and output suppression testsaxilo-cli/tests/agent-cancel.test.ts
execution
cancel()is invoked mid-executionagent-network-errors.test.ts
Agent network error handling and retry testsaxilo-cli/tests/agent-network-errors.test.ts
agent-terminate.test.ts
Agent termination and state cleanup testsaxilo-cli/tests/agent-terminate.test.ts
terminate()rejects subsequentrun()callsagent-server-retry.test.ts
Agent server error retry and resilience testsaxilo-cli/tests/agent-server-retry.test.ts
multiline-input-test.ts
Multiline text editor component testsaxilo-cli/tests/multiline-input-test.ts
project-doc.test.ts
Project documentation integration testsaxilo-cli/tests/project-doc.test.ts
dummy.test.ts
Dummy test placeholderaxilo-cli/tests/dummy.test.ts
agent-function-call-id.test.ts
Function call ID propagation regression testaxilo-cli/tests/agent-function-call-id.test.ts
AgentLoopcorrectly copies function call IDsfrom KhulnaSoft responses into subsequent
function_call_outputitemsidfield
call_idparameter is properly set when submittingtool results
agent-cancel-race.test.ts
Agent cancellation race condition testaxilo-cli/tests/agent-cancel-race.test.ts
response has already started streaming
UI
agent-project-doc.test.ts
Project documentation inclusion in agent instructionsaxilo-cli/tests/agent-project-doc.test.ts
axilo.md) is properly included inagent instructions
config loading
loadConfigthroughAgentLooptoKhulnaSoft SDK
agent-cancel-prev-response.test.ts
Agent cancellation clears previous response IDaxilo-cli/tests/agent-cancel-prev-response.test.ts
cancel()properly clearsprevious_response_idstate betweenruns
agent-rate-limit-error.test.ts
Rate limit error handling testaxilo-cli/tests/agent-rate-limit-error.test.ts
AgentLoopmessage is emitted
agent-generic-network-error.test.ts
Generic network error handling testsaxilo-cli/tests/agent-generic-network-error.test.ts
AgentLoopof throwing
text-buffer-word.test.ts
TextBuffer word navigation and deletion testsaxilo-cli/tests/text-buffer-word.test.ts
TextBuffercomponentwordRight,wordLeft,deleteWordLeft,deleteWordRightoperationsboundaries
agent-cancel-early.test.ts
Early agent cancellation before function callaxilo-cli/tests/agent-cancel-early.test.ts
previous_response_idis cleared when no call IDs arecaptured
approvals.test.ts
Command auto-approval safety assessment testsaxilo-cli/src/lib/approvals.test.ts
canAutoApprove()command safety assessment functionredirects
agent-max-tokens-error.test.ts
Max tokens context length error handling testaxilo-cli/tests/agent-max-tokens-error.test.ts
AgentLoopemission
raw-exec-process-group.test.ts
Process group termination on abort regression testaxilo-cli/tests/raw-exec-process-group.test.ts
rawExec()terminates entire process group onabort
killed
agent-invalid-request-error.test.ts
Invalid request error handling testaxilo-cli/tests/agent-invalid-request-error.test.ts
AgentLoopmodel-utils-network-error.test.ts
Model utilities offline resilience testsaxilo-cli/tests/model-utils-network-error.test.ts
unavailable
occur
isModelSupportedForResponses()returns true despite failuresexternal-editor.test.ts
External editor integration testaxilo-cli/tests/external-editor.test.ts
TextBufferspawnSyncto simulate editor modificationstext-buffer-copy-paste.test.ts
TextBuffer multi-line copy/paste testsaxilo-cli/tests/text-buffer-copy-paste.test.ts
TextBuffercancel-exec.test.ts
Process execution cancellation testsaxilo-cli/tests/cancel-exec.test.ts
AbortSignalinrawExec()parse-apply-patch.test.ts
Patch parsing unit testsaxilo-cli/src/lib/parse-apply-patch.test.ts
parseApplyPatch()functionapi-key.test.ts
API key configuration testsaxilo-cli/tests/api-key.test.ts
setApiKey()updates exportedKHULNASOFT_API_KEYreference
1 files
parse-apply-patch.ts
Patch parsing for file operation extractionaxilo-cli/src/lib/parse-apply-patch.ts
src/utils/agent/parse-apply-patch.ts)1 files
typings.d.ts
TypeScript type stubs for external librariesaxilo-cli/src/typings.d.ts
definitions
shell-quoteanddifflibraries101 files
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.