Skip to content

fix: Error occurred while exporting application skills #7727

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
shaohuzhang1 merged 1 commit into from
Nov 6, 2026
Merged

fix: Error occurred while exporting application skills #7727

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
2 changes: 1 addition & 1 deletion apps/application/serializers/application_skill.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from maxkb.conf import PROJECT_DIR
from maxkb.const import CONFIG

SKILL_SCRIPT_PATH = os.path.join(PROJECT_DIR, "apps", "application", "template", "skill", "skill_script.py")
SKILL_SCRIPT_PATH = os.path.join(PROJECT_DIR, "apps", "application", "template", "skill", "skill_script")


class InvalidSkillBaseUrl(AppApiException):

@shaohuzhang1 shaohuzhang1 Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff is syntactically valid, but it is a behavioral change, not an optimization: it removes the .py extension from SKILL_SCRIPT_PATH.

Potential issues

  1. File-not-found regression

    • If the file on disk is still skill_script.py, the new constant points to a non-existent path.
    • Any open(), subprocess, import, or path-existence check using this constant will fail.

  2. Execution behavior depends on how it is used

    • If executed as: 
      subprocess.run([sys.executable, SKILL_SCRIPT_PATH])
      an extensionless file can work, but only if the actual file exists and contains valid Python.
    • If executed directly: 
      subprocess.run([SKILL_SCRIPT_PATH])
      the file must exist, have a shebang, and be executable. This is less portable, especially on Windows.

  3. Import/package issues

    • Normal Python import machinery expects .py files/modules.
    • Extensionsless files can be missed by packaging, Docker copy rules, .gitignore, or deployment scripts.

  4. Consistency

    • If the file was renamed from skill_script.py to skill_script, all references, docs, tests, packaging rules, and deployment scripts must be updated.
    • If the file was not renamed, this diff is likely a bug.

Recommendations

  • Keep skill_script.py unless there is a clear reason to use an extensionless executable.
  • If extensionless is intentional, document it and ensure the actual file is renamed and included in packaging.
  • Use pathlib and validate the path early:
from pathlib import Path

SKILL_SCRIPT_PATH = (
    Path(PROJECT_DIR)
    / "apps"
    / "application"
    / "template"
    / "skill"
    / "skill_script.py"
)

if not SKILL_SCRIPT_PATH.is_file():
    raise RuntimeError(f"Skill script not found: {SKILL_SCRIPT_PATH}")
  • For portable execution, prefer:
subprocess.run([sys.executable, str(SKILL_SCRIPT_PATH)], check=True)

Conclusion: The only visible irregularity is the removed .py extension. It is only safe if the file was intentionally renamed to an extensionless executable and all callers/deployment steps support that. Otherwise, revert this change.

Expand Down
0 ...pplication/template/skill/skill_script.py → apps/application/template/skill/skill_script
View file
Open in desktop
File renamed without changes.
Loading