Skip to content

refactor(sdk): remove unreachable branches and dead helpers #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
samzong merged 1 commit into from
Sep 12, 2026
Merged

refactor(sdk): remove unreachable branches and dead helpers #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 5 additions & 26 deletions go/kitup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,15 +437,9 @@ type bundleMetadata struct {
Explicit bool
}

type bundleFile struct {
Path string
Contents []byte
Mode fs.FileMode
}

type normalizedSkillBundle struct {
Files []bundleFile
ByPath map[string]bundleFile
Files []SkillFile
ByPath map[string]SkillFile
}

var skillNamePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`)
Expand Down Expand Up @@ -812,10 +806,6 @@ func RunBundledSkillInstall(opts InstallWorkflowOptions) (InstallWorkflowReport,
if info, err := file.Stat(); err == nil && info.Mode()&os.ModeCharDevice != 0 {
opts.StdinTTY = true
}
} else if in == os.Stdin {
if info, err := os.Stdin.Stat(); err == nil && info.Mode()&os.ModeCharDevice != 0 {
opts.StdinTTY = true
}
}
}
out := opts.Out
Expand Down Expand Up @@ -884,9 +874,6 @@ func RunBundledSkillInstall(opts InstallWorkflowOptions) (InstallWorkflowReport,
return InstallWorkflowReport{Selection: selection, Scope: scope, Plan: plan, Report: report, DryRun: opts.DryRun}, nil
}
renderInstallSummary(out, plan)
if len(plan.Installed)+len(plan.Updated) == 0 {
return InstallWorkflowReport{Selection: selection, Scope: scope, Plan: plan, Report: plan}, nil
}
if selection.NeedsConfirmation {
confirmed, err := promptConfirmation(reader, out)
if err != nil {
Expand Down Expand Up @@ -1861,7 +1848,7 @@ func readFSBundleFiles(fsys fs.FS, root string) ([]SkillFile, error) {
}

func normalizeSkillFiles(files []SkillFile) (normalizedSkillBundle, error) {
byPath := map[string]bundleFile{}
byPath := map[string]SkillFile{}
for _, file := range files {
normalizedPath, include, err := normalizeBundlePath(file.Path)
if err != nil {
Expand All @@ -1877,14 +1864,14 @@ func normalizeSkillFiles(files []SkillFile) (normalizedSkillBundle, error) {
if mode == 0 {
mode = defaultBundleFileMode(normalizedPath)
}
byPath[normalizedPath] = bundleFile{Path: normalizedPath, Contents: file.Contents, Mode: mode}
byPath[normalizedPath] = SkillFile{Path: normalizedPath, Contents: file.Contents, Mode: mode}
}
paths := make([]string, 0, len(byPath))
for path := range byPath {
paths = append(paths, path)
}
sort.Strings(paths)
normalized := normalizedSkillBundle{Files: make([]bundleFile, 0, len(paths)), ByPath: byPath}
normalized := normalizedSkillBundle{Files: make([]SkillFile, 0, len(paths)), ByPath: byPath}
for _, path := range paths {
normalized.Files = append(normalized.Files, byPath[path])
}
Expand Down Expand Up @@ -1917,14 +1904,6 @@ func normalizeBundlePath(value string) (string, bool, error) {
return strings.Join(parts, "/"), true, nil
}

func copySkillBundleDir(src, dest string) error {
bundle, err := readSkillBundle(DirectoryBundle(src))
if err != nil {
return err
}
return copySkillBundle(bundle, dest)
}

func skipName(name string) bool {
return name == ".git" || name == ".kitup.json" || name == ".DS_Store" || strings.HasSuffix(name, ".swp") || strings.HasSuffix(name, "~")
}
Expand Down
1 change: 0 additions & 1 deletion python/src/kitup/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ class BundleFile:
class NormalizedSkillBundle:
files: list[BundleFile]
by_path: dict[str, BundleFile]
label: str | None = None


@dataclass(frozen=True)
Expand Down
83 changes: 16 additions & 67 deletions python/src/kitup/workflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,7 @@


def split_flag_values(values: list[str]) -> list[str]:
return [
part.strip()
for value in values
for part in value.replace(",", " ").split()
if part.strip()
]


def dedupe(values: list[str]) -> list[str]:
seen: set[str] = set()
result: list[str] = []
for value in values:
if value not in seen:
seen.add(value)
result.append(value)
return result
return [part for value in values for part in value.replace(",", " ").split()]


def parse_scope_flag(
Expand Down Expand Up @@ -68,7 +53,7 @@ def agent_selector_from_flags(
}
)
return "*"
return dedupe(agents)
return list(dict.fromkeys(agents))


def parse_install_flags(flags: dict[str, object]) -> ParsedInstallFlags:
Expand Down Expand Up @@ -201,7 +186,7 @@ def run_bundled_skill_install_with_io(
output: object | None,
) -> InstallWorkflowReport:
reader = _LineReader(input)
writer = _coerce_output(output)
writer = _OutputWriter(output)
scope, scope_error = _resolve_workflow_scope(
reader=reader,
output=writer,
Expand All @@ -217,8 +202,8 @@ def run_bundled_skill_install_with_io(
return InstallWorkflowReport(
selection=scope_error,
scope=scope,
plan=empty_install_report(),
report=empty_install_report(),
plan=InstallReport(),
report=InstallReport(),
canceled=False,
dry_run=options.dry_run,
)
Expand All @@ -238,8 +223,8 @@ def run_bundled_skill_install_with_io(
return InstallWorkflowReport(
selection=selection,
scope=scope,
plan=empty_install_report(),
report=empty_install_report(),
plan=InstallReport(),
report=InstallReport(),
canceled=False,
dry_run=options.dry_run,
)
Expand All @@ -255,8 +240,8 @@ def run_bundled_skill_install_with_io(
return InstallWorkflowReport(
selection=selection,
scope=scope,
plan=empty_install_report(),
report=empty_install_report(),
plan=InstallReport(),
report=InstallReport(),
canceled=True,
dry_run=options.dry_run,
)
Expand Down Expand Up @@ -297,21 +282,12 @@ def run_bundled_skill_install_with_io(
dry_run=False,
)
_render_install_summary(writer, plan)
if not _has_install_writes(plan):
return InstallWorkflowReport(
selection=selection,
scope=scope,
plan=plan,
report=plan,
canceled=False,
dry_run=False,
)
if selection.needs_confirmation and not _prompt_confirmation(reader, writer):
return InstallWorkflowReport(
selection=selection,
scope=scope,
plan=plan,
report=empty_install_report(),
report=InstallReport(),
canceled=True,
dry_run=False,
)
Expand All @@ -327,10 +303,6 @@ def run_bundled_skill_install_with_io(
)


def empty_install_report(errors: list[object] | None = None) -> InstallReport:
return InstallReport(errors=list(errors or []))


def _resolve_workflow_scope(
*,
reader: "_LineReader",
Expand Down Expand Up @@ -478,10 +450,6 @@ def _has_visible_install_plan(report: InstallReport) -> bool:
)


def _has_install_writes(report: InstallReport) -> bool:
return len(report.installed) + len(report.updated) > 0


def _install_selection(
selected_host_ids: list[str],
detected_host_ids: list[str],
Expand Down Expand Up @@ -548,11 +516,7 @@ def _coerce_optional_text(value: object) -> str | None:


def _coerce_flag_values(value: object) -> list[str]:
if value is None:
return []
if isinstance(value, list):
return [item for item in value if isinstance(item, str)]
if isinstance(value, tuple):
if isinstance(value, (list, tuple)):
return [item for item in value if isinstance(item, str)]
if isinstance(value, str):
return [value]
Expand Down Expand Up @@ -592,35 +556,24 @@ def _iter_lines(self, source: object | None) -> Iterable[str]:
if source is None:
return []
if isinstance(source, bytes):
return self._split_text(source.decode("utf-8"))
return source.decode("utf-8").splitlines()
if isinstance(source, str):
return self._split_text(source)
return source.splitlines()
if hasattr(source, "read"):
contents = source.read()
if isinstance(contents, bytes):
return self._split_text(contents.decode("utf-8"))
return self._split_text(str(contents))
return contents.decode("utf-8").splitlines()
return str(contents).splitlines()
if isinstance(source, Iterable):
chunks: list[str] = []
for item in source:
if isinstance(item, bytes):
chunks.append(item.decode("utf-8"))
else:
chunks.append(str(item))
return self._split_text("".join(chunks))
return "".join(chunks).splitlines()
return []

@staticmethod
def _split_text(text: str) -> list[str]:
if text == "":
return []
lines = text.splitlines()
if text.endswith(("\n", "\r")):
return [line.rstrip("\r") for line in lines]
if not lines:
return [text.rstrip("\r")]
return [line.rstrip("\r") for line in lines]


class _OutputWriter:
def __init__(self, target: object | None) -> None:
Expand All @@ -630,7 +583,3 @@ def write(self, chunk: str) -> None:
if self._target is None:
return
self._target.write(chunk)


def _coerce_output(output: object | None) -> _OutputWriter:
return _OutputWriter(output)
27 changes: 7 additions & 20 deletions rust/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,6 @@ struct BundleFile {

#[derive(Clone, Debug)]
struct NormalizedSkillBundle {
files: Vec<BundleFile>,
by_path: BTreeMap<String, BundleFile>,
}

Expand Down Expand Up @@ -907,7 +906,7 @@ pub fn compute_bundle_content_hash(bundle: &SkillBundle) -> io::Result<String> {

fn content_hash(bundle: &NormalizedSkillBundle) -> String {
let mut hash = Sha256::new();
for file in &bundle.files {
for file in bundle.by_path.values() {
hash.update(file.path.as_bytes());
hash.update([0]);
hash.update(&file.contents);
Expand Down Expand Up @@ -1034,16 +1033,6 @@ pub fn run_bundled_skill_install_with_io<R: BufRead, W: Write>(
});
}
render_install_summary(output, &plan)?;
if plan.installed.len() + plan.updated.len() == 0 {
return Ok(InstallWorkflowReport {
selection,
scope: workflow_scope,
plan: plan.clone(),
report: plan,
canceled: false,
dry_run: false,
});
}
if selection.needs_confirmation && !prompt_confirmation(input, output)? {
return Ok(InstallWorkflowReport {
selection,
Expand Down Expand Up @@ -1416,7 +1405,7 @@ fn remove_managed_skill(

fn copy_skill_bundle(bundle: &NormalizedSkillBundle, dest: &Path) -> io::Result<()> {
fs::create_dir_all(dest)?;
for file in &bundle.files {
for file in bundle.by_path.values() {
let to = dest.join(PathBuf::from(
file.path.replace('/', std::path::MAIN_SEPARATOR_STR),
));
Expand All @@ -1435,7 +1424,7 @@ fn repair_skill_bundle_modes(
write: bool,
) -> io::Result<bool> {
let mut repaired = false;
for file in &bundle.files {
for file in bundle.by_path.values() {
let to = dest.join(PathBuf::from(
file.path.replace('/', std::path::MAIN_SEPARATOR_STR),
));
Expand Down Expand Up @@ -2169,8 +2158,7 @@ fn normalize_skill_files(files: Vec<SkillFile>) -> io::Result<NormalizedSkillBun
},
);
}
let files = by_path.values().cloned().collect();
Ok(NormalizedSkillBundle { files, by_path })
Ok(NormalizedSkillBundle { by_path })
}

fn default_bundle_file_mode(path: &str) -> u32 {
Expand All @@ -2192,9 +2180,8 @@ fn normalize_bundle_path(value: &str) -> io::Result<Option<String>> {
format!("invalid skill file path: {value}"),
));
}
let parts: Vec<_> = value.split('/').collect();
for part in &parts {
if part.is_empty() || *part == "." || *part == ".." {
for part in value.split('/') {
if part.is_empty() || part == "." || part == ".." {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("invalid skill file path: {value}"),
Expand All @@ -2204,7 +2191,7 @@ fn normalize_bundle_path(value: &str) -> io::Result<Option<String>> {
return Ok(None);
}
}
Ok(Some(parts.join("/")))
Ok(Some(value.to_string()))
}

#[cfg(unix)]
Expand Down
4 changes: 3 additions & 1 deletion tests/go-golden/golden_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,9 @@ func setupGiven(t *testing.T, tc goldenCase, home, workspace string) {
}
if target, ok := tc.Given["copySkillBundleTo"].(string); ok {
must(t, os.RemoveAll(expandString(target, home, workspace)))
must(t, copySkillBundleDir(caseSkillBundleDir(tc), expandString(target, home, workspace)))
bundle, err := readSkillBundle(DirectoryBundle(caseSkillBundleDir(tc)))
must(t, err)
must(t, copySkillBundle(bundle, expandString(target, home, workspace)))
}
if modes, ok := tc.Given["fileModes"].(map[string]any); ok {
for path, mode := range modes {
Expand Down
Loading
Loading