Enable loading of lora-adapter parts in dev mode by rishi-jat · Pull Request #1098 · kitops-ml/kitops

rishi-jat · 2026-02-26T23:38:33Z

Description

This PR enables dev mode to load LoRA adapters when type: lora-adapter parts are defined in the ModelKit.

Dev mode now:

Detects lora-adapter entries in model.parts
Resolves adapter paths within the context directory
Locates the corresponding .gguf adapter file
Passes --lora <adapter_path> to the LLM harness during startup

Existing behavior remains unchanged when no LoRA adapters are present.

Linked issues

Fixes #306

Copilot

Pull request overview

This PR adds support for loading LoRA (Low-Rank Adaptation) adapters in dev mode when type: lora-adapter parts are defined in a ModelKit. Dev mode now detects lora-adapter entries, resolves their paths, locates the corresponding .gguf files, and passes them to the LLM harness via --lora flags.

Changes:

Modified LLMHarness.Start() to accept a slice of LoRA adapter paths
Added LoRA adapter detection and collection logic in runDev()
Created findLoraAdapterFile() helper function to locate .gguf adapter files

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
pkg/lib/harness/llm-harness.go	Updated Start() method signature to accept loraPaths parameter; modified command construction for both Windows and non-Windows to include --lora flags for each adapter
pkg/cmd/dev/dev.go	Added logic to detect lora-adapter type parts, resolve their paths, and pass them to the harness; added findLoraAdapterFile() helper function mirroring findModelFile()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T23:42:29Z

pkg/lib/harness/llm-harness.go

+		loraArgs := ""
+		for _, loraPath := range loraPaths {
+			loraArgs += fmt.Sprintf(" --lora %s", loraPath)
+		}
 		cmd = exec.Command("sh", "-c",
-			fmt.Sprintf("./llamafile --server --model %s --host %s --port %d --path %s --gpu AUTO --nobrowser --unsecure",
-				modelPath, harness.Host, harness.Port, uiHome),
+			fmt.Sprintf("./llamafile --server --model %s --host %s --port %d --path %s --gpu AUTO --nobrowser --unsecure%s",
+				modelPath, harness.Host, harness.Port, uiHome, loraArgs),
 		)


The non-Windows shell command construction is vulnerable to shell injection. The loraPath variables are directly interpolated into the shell command string without proper escaping or quoting. If a lora adapter path contains spaces or special shell characters (e.g., semicolons, backticks, dollar signs), it could lead to command injection or malformed commands.

Consider using the same approach as the Windows code path: build an args slice and pass it to exec.Command instead of using shell interpolation. This is safer and more consistent across platforms.

pkg/cmd/dev/dev.go

rishi-jat · 2026-03-01T14:33:00Z

/cc @gorkem

amisevsk · 2026-03-02T22:02:21Z

@rishi-jat there are some test failures on this PR

amisevsk · 2026-03-02T22:09:12Z

pkg/lib/harness/llm-harness.go

+		args := []string{
+			"--server",
+			"--model", modelPath,
+			"--host", harness.Host,
+			"--port", fmt.Sprintf("%d", harness.Port),
+			"--path", uiHome,
+			"--gpu", "AUTO",
+			"--nobrowser",
+			"--unsecure",
+		}
+		for _, loraPath := range loraPaths {
+			args = append(args, "--lora", loraPath)
+		}
+		cmd = exec.Command("./llamafile", args...)


The arguments seem to be the same between the windows and non-windows case. Why not define them once and branch on the exec.Command step?

args := []string{ "--server", "--model", modelPath, "--host", harness.Host, "--port", fmt.Sprintf("%d", harness.Port), "--path", uiHome, "--gpu", "AUTO", "--nobrowser", "--unsecure", } for _, loraPath := range loraPaths { args = append(args, "--lora", loraPath) } if runtime.GOOS == "windows" { cmd = exec.Command("./llamafile.exe", args...) } else { cmd = exec.Command("./llamafile", args...) }

Good point.

amisevsk · 2026-03-02T22:10:51Z

pkg/cmd/dev/dev.go

+	if err := filepath.WalkDir(absPath, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if strings.HasSuffix(path, ".gguf") && d.Type().IsRegular() {
+			if loraPath == "" {
+				loraPath = path
+			} else {
+				return fmt.Errorf("multiple lora adapter files found: %s and %s", loraPath, path)
+			}
+		}
+		return nil
+	}); err != nil {
+		return "", fmt.Errorf("error searching for lora adapter file in %s: %w", absPath, err)
+	} else if loraPath == "" {
+		return "", fmt.Errorf("could not find lora adapter file in %s", absPath)
+	}
+	output.Debugf("Found lora adapter path in directory %s at %s", absPath, loraPath)
+	return loraPath, nil


Since only one LoRA is supported, I think it makes sense to avoid the walkDir step entirely and error out if the path specified by the modelpart is not a regular file.

pkg/cmd/dev/dev.go

rishi-jat · 2026-03-16T00:50:49Z

@amisevsk I have made the suggested changes please have a look. Thanks!

amisevsk

Looks good to me, thanks @rishi-jat

gorkem

Thank you for the PR, this was something I was hoping to get to for a long time. PR needs to be rebased and there are a couple of issues that would be nice to address before merging.

pkg/cmd/dev/dev.go

Signed-off-by: Rishi Jat <rishijat098@gmail.com> Avoid shell interpolation when passing LoRA adapter paths Signed-off-by: Rishi Jat <rishijat098@gmail.com> refactor: require direct .gguf file for lora adapter instead of directory walk Signed-off-by: Rishi Jat <rishijat098@gmail.com>

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat · 2026-03-17T18:12:57Z

@gorkem @amisevsk Made all the suggested changes have a look.

gorkem · 2026-03-17T23:36:43Z

@rishi-jat the build fails. Please make sure that build passes cleanly

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat · 2026-03-18T12:13:38Z

@gorkem CI pass. please have a look. Thanks!

amisevsk

I'm not sure why there's mock-related code in the llm-harness.go file

pkg/lib/harness/llm-harness.go

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

amisevsk

Tests are still failing

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat · 2026-03-25T23:12:41Z

Tests are still failing

CI is now green please have a look. Thanks!

amisevsk

The current version is once again vulnerable to shell injection:

We run with --lora <LoRA-Path> and sh -c, a malicious LoRA path can easily lead to shell injection
Since we support LoRA adapters as directories (I still think it's not worth doing), it's additionally easy to obfuscate the presence of injection, as we can just store a directory path and put the malicious file in it to hide its presence from the user

E.g., if my ModelKit contains something like

/adapters/"lora.gguf &; echo "hi" > ~/hi.gguf"

and the Kitfile

model
  parts:
    - path: adapters/
      type: lora

you a) won't be able to notice an issue by looking at the Kitfile, and b) will automatically run arbitrary code when using kit dev.

It seems we're at an impasse here, as it seems sh -c is required for APE binaries used by llamafile, though.

amisevsk · 2026-03-31T15:21:05Z

According to https://github.com/jart/cosmopolitan?tab=readme-ov-file#shells, the sh -c requirement was resolved in zsh 5.9. Assuming this is primarily a problem for macOS, I've checked an up-to-date mac and the default version of zsh there is 5.9 currently, though I don't know when this was updated.

Is the sh -c requirement still actual? zsh 5.9 was released in 2022.

amisevsk · 2026-03-31T15:23:33Z

pkg/lib/harness/llm-harness.go

 		cmd = exec.Command("sh", "-c",
-			fmt.Sprintf("./llamafile --server --model %s --host %s --port %d --path %s --gpu AUTO --nobrowser --unsecure",
-				modelPath, harness.Host, harness.Port, uiHome),
+			fmt.Sprintf("./llamafile --server --model %s --host %s --port %d --path %s --gpu AUTO --nobrowser --unsecure%s",
+				modelPath, harness.Host, harness.Port, uiHome, loraArgs),


I understand reverting to using sh -c in 87f4adf, but why also undo the change of using one args slice for both windows and unix? If we want the shell-injectable form here, we can still reuse the args slice rather than duplicating the arguments in this format:

args := []string{ "--server", "--model", modelPath, "--host", harness.Host, "--port", fmt.Sprintf("%d", harness.Port), "--path", uiHome, "--gpu", "AUTO", "--nobrowser", "--unsecure", } for _, loraPath := range loraPaths { args = append(args, "--lora", loraPath) } if runtime.GOOS == "windows" { cmd = exec.Command("./llamafile.exe", args...) } else { // Note: running llamafile with sh -c is required as it is an APE binary (is this still true?) llamaCmd := "./llamafile " + strings.Join(args, " ") cmd = exec.Command("sh", "-c", llamaCmd) }

amisevsk · 2026-03-31T15:24:53Z

pkg/cmd/dev/dev.go

+	// Case 2: directory → search for .gguf
+	if stat.IsDir() {
+		entries, err := os.ReadDir(absPath)
+		if err != nil {
+			return "", fmt.Errorf("error searching for lora adapter in %s: %w", absPath, err)
+		}
+
+		var found string
+		for _, entry := range entries {
+			if entry.Type().IsRegular() && strings.HasSuffix(strings.ToLower(entry.Name()), ".gguf") {
+				path := filepath.Join(absPath, entry.Name())
+				if found != "" {
+					return "", fmt.Errorf("multiple lora adapter files found: %s and %s", found, path)
+				}
+				found = path
+			}
+		}
+		if found == "" {
+			return "", fmt.Errorf("no .gguf lora adapter found in %s", absPath)
+		}
+		output.Debugf("Found lora adapter path in directory %s at %s", absPath, found)
+		return found, nil
+	}


I know me and @gorkem are giving conflicting review here, but I'm still of the opinion that if you have a model part with type lora-adapter, we should not support directories for this part. Given that a LoRA adapter is a specific thing, what's the use case for allowing it to be a directory?

Copilot AI review requested due to automatic review settings February 26, 2026 23:38

Copilot started reviewing on behalf of rishi-jat February 26, 2026 23:38 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

amisevsk requested a review from gorkem March 2, 2026 22:02

amisevsk reviewed Mar 2, 2026

View reviewed changes

rishi-jat force-pushed the feat/dev-mode-lora-adapter branch from 0eebd5e to 5bff6cf Compare March 16, 2026 01:17

amisevsk approved these changes Mar 17, 2026

View reviewed changes

gorkem requested changes Mar 17, 2026

View reviewed changes

pkg/cmd/dev/dev.go Outdated Show resolved Hide resolved

pkg/cmd/dev/dev.go Show resolved Hide resolved

rishi-jat force-pushed the feat/dev-mode-lora-adapter branch from 5bff6cf to a2fa09b Compare March 17, 2026 18:00

rishi-jat force-pushed the feat/dev-mode-lora-adapter branch from a2fa09b to 25e9f06 Compare March 17, 2026 18:02

did maintainer suggestion

2f94a5e

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat requested a review from gorkem March 17, 2026 18:12

rishi-jat added 2 commits March 18, 2026 15:59

fix: use correct warning log API

350a0ad

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

fix: ensure dev lifecycle consistency with mock harness fallback

e43a990

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

amisevsk requested changes Mar 19, 2026

View reviewed changes

pkg/lib/harness/llm-harness.go Outdated Show resolved Hide resolved

fix

79a426d

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat requested a review from amisevsk March 20, 2026 15:33

amisevsk requested changes Mar 24, 2026

View reviewed changes

fix: restore sh -c for llamafile on Linux

87f4adf

Signed-off-by: Rishi Jat <rishijat098@gmail.com>

rishi-jat force-pushed the feat/dev-mode-lora-adapter branch from 5a78f85 to 87f4adf Compare March 25, 2026 22:59

gorkem approved these changes Mar 31, 2026

View reviewed changes

amisevsk reviewed Mar 31, 2026

View reviewed changes

amisevsk requested changes Mar 31, 2026

View reviewed changes

Conversation

rishi-jat commented Feb 26, 2026

Description

Linked issues

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rishi-jat commented Mar 1, 2026

Uh oh!

amisevsk commented Mar 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rishi-jat commented Mar 16, 2026

Uh oh!

amisevsk left a comment

Choose a reason for hiding this comment

Uh oh!

gorkem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rishi-jat commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gorkem commented Mar 17, 2026

Uh oh!

rishi-jat commented Mar 18, 2026

Uh oh!

amisevsk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amisevsk left a comment

Choose a reason for hiding this comment

Uh oh!

rishi-jat commented Mar 25, 2026

Uh oh!

amisevsk left a comment

Choose a reason for hiding this comment

Uh oh!

amisevsk commented Mar 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rishi-jat commented Mar 17, 2026 •

edited

Loading