Skip to content

feat(bg_remover): harden skill to v0.2.0 - #268

Merged
rosspeili merged 10 commits into
ARPAHLS:mainfrom
AyushSrivastava1818:feat/bg-remover-hardening-v0.2-clean
Aug 2, 2026
Merged

feat(bg_remover): harden skill to v0.2.0#268
rosspeili merged 10 commits into
ARPAHLS:mainfrom
AyushSrivastava1818:feat/bg-remover-hardening-v0.2-clean

Conversation

@AyushSrivastava1818

Copy link
Copy Markdown
Contributor

Description

Hardens the creative/bg_remover skill to v0.2.0 by improving robustness, validation, and documentation.

Fixes #257

Changes

  • Reuse cached rembg sessions across executions.
  • Validate Base64 input and image files.
  • Reject missing, directory, empty, oversized (>25 MB), and invalid image inputs.
  • Automatically create parent directories for output_path.
  • Update the skill documentation and manifest.
  • Expand test coverage for the new validation behavior and session reuse.

Type of Change

  • Skill Upgrade — changes to an existing skill under skills/

Checklist (all PRs)

  • Linked GitHub issue (Fixes #257)
  • Scope matches the issue — no unrelated refactors
  • python -m black --check . and flake8 pass locally (or CI-equivalent subset)
  • pytest skills/ and pytest tests/ pass locally when relevant
  • CHANGELOG.md updated under [Unreleased] when user-visible behavior changes
  • examples/README.md updated if this PR adds, renames, or removes a runnable script
  • Ran pytest tests/test_registry_docs.py when skills, examples index, or agent-loops matrix changed

New or updated skill

  • Skill at skills/<category>/<skill_name>/ (from templates/python_skill/ or equivalent)
  • manifest.yaml: name, version, description, parameters, constitution, real issuer
  • Deterministic skill.py (no ad-hoc LLM-generated execution paths)
  • instructions.md explains when and how to use the skill
  • card.json issuer matches manifest when present
  • test_skill.py covers execution and schema expectations
  • SkillLoader.load_skill("/<skill_name>") succeeds (or deps documented)
  • docs/skills/README.md and row in docs/skills/README.md
  • Usage Examples for Gemini, Claude, OpenAI, DeepSeek, Ollama per skill usage template

Constitution and safety

This update improves input validation and filesystem safety by validating Base64 input and image files, rejecting missing, directory, empty, oversized (>25 MB), and invalid images, reusing cached rembg sessions, and automatically creating parent directories for output_path.

Related Issues

Fixes #257

@rosspeili

Copy link
Copy Markdown
Contributor

Thanks for the clean branch, much better than #266, I will close 266 and track here.

This is closer on the runtime core (session cache, basic validation, 25 MB limit, mkdir for output_path, tests), but it is not yet enough to close #257. Please treat the issue acceptance criteria as a checklist and finish them in this PR.

Still missing / fix

  1. Rebase on current main (post-0.4.7 / citation) so CHANGELOG/Unreleased merges cleanly.
  2. Path safety: reject traversal / unsafe output_path (and document the rule).
  3. Apply size + decode checks to Base64 too (not only input_path).
  4. PIL verify(): reopen the image after verify() before reuse (classic gotcha).
  5. Manifest / errors: document FILE_NOT_FOUND (and any new codes) in manifest.yaml error_code + keep instructions.md / catalog aligned.
  6. [Skill Upgrade]: creative/bg_remover — hardening to 0.2.0 #257 deliverables still open:
    • examples/bg_remover_demo.py + examples/README.md row
    • wire docs/usage/agent_loops.md (not catalog-only)
    • optional first-run / model-download hint in response or docs (as scoped in the issue)
  7. Prefer loading version/metadata from manifest.yaml instead of a second hardcoded dict in skill.py if other skills already do that pattern.

Bump is fine as 0.2.0 once the above is in. Ping when the PR matches the full #257 checklist, I will re-review <3

@AyushSrivastava1818
AyushSrivastava1818 force-pushed the feat/bg-remover-hardening-v0.2-clean branch from e4cad04 to 61a942a Compare July 29, 2026 11:23
@rosspeili

Copy link
Copy Markdown
Contributor

Thanks @AyushSrivastava1818, re-reviewed the latest commits on. The runtime core is in good shape now and mostly matches the earlier review / #257: session reuse, Base64 validation + shared 25 MB check, PIL verify() reopen, output_path traversal reject + parent mkdir, demo + agent_loops wiring, and expanded bundle tests. Local skill tests were also green here.

That said, this is very close but still a bit behind ideal state and not completely ready to close #257 yet. Please ammend these in this PR so we don’t need a follow-up:

Musts

  1. CHANGELOG, move the bg_remover entry under [Unreleased] (do not edit the already-shipped [0.4.7] section).
  2. examples/README.md, restore normal spacing. The file currently has extra blank lines throughout; please revert that rewrite and only add the bg_remover_demo.py row cleanly.
  3. Error-code docs, add FILE_NOT_FOUND to the error tables in both instructions.md and docs/skills/bg_remover.md (code/manifest already have it; docs still list only INVALID_INPUT / MISSING_DEPENDENCY / PROCESSING_FAILED).

Polish hints (same PR if easy)

  • Prefer loading the skill manifest property from manifest.yaml instead of a second hardcoded dict (loader already uses the YAML, this just avoids version drift).
  • One oversized Base64 (>25 MB) golden test.

Ping when those three must-fix items land and I’ll re-review for merge and close #257. <3 Good job so far and for following the steering accurately.

@rosspeili

Copy link
Copy Markdown
Contributor

Hey @AyushSrivastava1818, re-reviewed the latest fixes and most things are cleaned correctly. Well done <3

One tiny blocker before merge / close #257

In docs/usage/agent_loops.md, the creative/bg_remover row is missing the final Ollama column. Other rows have 7 cells, this one stops after DeepSeek. Please restore, as I think you had it before:

| creative/bg_remover|bg_remover_demo.py (local execute) | (catalog page) | (catalog page) | (catalog page) | (catalog page) | (catalog page) |

Optional but nice to have in this PR to avoid follow up PR:

  • Prefer loading skill.manifest from manifest.yaml instead of the hardcoded mini-dict, check other skills how manifest is used
  • Add a trailing newline to instructions.md or ci might fail on main

After these are in, I’m happy to merge and close #257. Good work iterating on this. <3

@AyushSrivastava1818
AyushSrivastava1818 force-pushed the feat/bg-remover-hardening-v0.2-clean branch from 1e78bf9 to 4f084df Compare July 31, 2026 17:23
Remove accidental commit-message line from examples/README.md and load manifest metadata from manifest.yaml to avoid version drift.
@rosspeili

Copy link
Copy Markdown
Contributor

@AyushSrivastava1818, thanks for iterating on this, the runtime hardening looks good and matches the scope nicely.

To save you another round-trip, I pushed a small maintainer commit onto your branch:

  • removed the accidental commit-message line after the bg_remover_demo.py row in examples/README.md
  • switched skill.py to load manifest metadata from manifest.yaml instead of a hardcoded mini-dict (avoids version drift)

Bundle + registry docs tests still pass on my side. Good to merge from here, I'll merge and close #257 once CI is green.

Nice work on this one. <3

@rosspeili
rosspeili merged commit d32260e into ARPAHLS:main Aug 2, 2026
6 checks passed
@AyushSrivastava1818

Copy link
Copy Markdown
Contributor Author

@rosspeili can we connect on slack too?

@rosspeili

Copy link
Copy Markdown
Contributor

@AyushSrivastava1818 I don't use Slack outside orgs I am in, but you we can connect on any social or via email if you have any questions outside Skillware / ARPA repos.

@AyushSrivastava1818

Copy link
Copy Markdown
Contributor Author

@rosspeili Okayy Bhai...will stay in touch loved contributing to skillware..great experience here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Skill Upgrade]: creative/bg_remover — hardening to 0.2.0

2 participants