fix: replace silent catch blocks with warn messages

[vystartasv]

What

Replaces 5 silent catch {} blocks with proper error logging. These catch blocks were swallowing real failures — skill parse errors, hook config issues, and discovery problems — with zero visibility for the user.

The CONTRIBUTING.md standard says: "Error messages should help the user fix the problem. 'Failed to load agent.yaml: file not found at /path' is great. A bare 'Error' isn't helpful." These catch blocks didn't even give a bare error — they were completely silent.

Changes

File	What	Before	After
skill-loader.ts	parseSkillMd failure	Silent skip	warn() with path + error message
skill-loader.ts	loadSkillMetadata failure	Silent skip	warn() with path + error message
skill-discovery.ts	Discovery parse failure	Silent skip	warn() with path + error message
skill-discovery.ts	Skill load failure	Silent skip	warn() with path + error message
gemini.ts	Hook config parse failure	Silent return null	console.warn() then return null
openai.ts	Tool stub TODO	Generic # TODO	Names the specific tool that needs implementation

Why

Debugging silent skill or hook failures is currently impossible without adding temporary console.log calls. Users see "0 skills loaded" with no indication WHY their skills didn't parse.

Tested

• npm run build passes (pre-existing js-yaml type issue unchanged)
• All files compile to dist/
• Warning messages follow existing warn() pattern from format.ts

[shreyas-lyzr]

The catch-block changes are genuinely good — silent catch{} → warn(...) with error message in four hot paths (skill discovery, skill loader x2, gemini hook config). Real DX improvement; debugging a malformed SKILL.md is much easier with Skill parse failed: <path> — <error> than with nothing.

But this PR also includes the identity.yaml changes from #76 (the bundled diff shows spec/SPECIFICATION.md, spec/schemas/identity.schema.json modifications). The identity work has a design conflict with the just-merged RFC #73 — I've left detailed feedback on #76 about that.

Concrete ask:

1. Rebase this PR off main so the identity-related files (spec/SPECIFICATION.md, spec/schemas/identity.schema.json) are no longer in the diff.
2. The PR then becomes ~30 lines of pure catch-block fixes across src/adapters/gemini.ts, src/adapters/openai.ts (the # TODO: Implement tool logic → # Tool stub cleanup), src/utils/skill-discovery.ts, and src/utils/skill-loader.ts.

After the rebase, this will merge same-day. Apologies for the carryover from #76.

[vystartasv]

Rebased off main — the PR is now ~30 lines of pure catch-block fixes across exactly the four files you listed:

• src/adapters/gemini.ts — hook config parse failure now logs warning
• src/adapters/openai.ts — TODO comment improved (tool stub)
• src/utils/skill-discovery.ts — two silent catch blocks now warn with skill path and error
• src/utils/skill-loader.ts — two silent catch blocks now warn with path and parse error

No identity files in the diff. Ready for same-day merge as offered. Thanks for the detailed review on both PRs — the RFC #73 direction makes sense and I'll follow up on #76 separately.

[shreyas-lyzr]

Clean, focused fix. All five silent catch blocks are now surfaced with actionable warn messages — exactly what the CONTRIBUTING.md standard asks for.

Two small observations (not blocking):

1. gemini.ts uses bare console.warn(...) while the four other sites use the project-level warn(...) helper from format.ts. Worth making consistent — import warn and use it there too so log formatting is uniform.

2. The openai.ts TODO-to-stub rename is a nice clarity win, but worth noting that it diverges slightly from the LangChain adapter (PR #81) which uses a # TODO: Implement tool logic comment in the same generated stub. Minor, but a single canonical stub message would avoid confusion for new contributors.

Overall: the change is safe, additive, and directly improves debuggability. Good to merge.

[vystartasv]

Rebased on latest main (was 1 commit behind). All 3 CI builds passing on the new SHA. The previous approval got dismissed in the rebase — could use a re-approve when you get a chance @shreyas-lyzr 🙏

[shreyas-lyzr]

Clean, targeted fix. Replacing silent catch blocks with warn() calls is the right call — silent swallowing of skill parse errors was a real operational hazard.

A few observations:

1. All five changed catch blocks now use (err as Error).message. This is safe in practice but TypeScript allows throwing non-Error values. String(err) or err instanceof Error ? err.message : String(err) is more defensive and avoids a potential [object Object] warn in edge cases.

2. The comment stub change in openai.ts (// TODO: Implement tool logic → // Tool stub — replace with actual ${tool.name} implementation) is bundled into this PR but is unrelated to the silent-catch fix. Fine to include, but worth calling out in the PR description.

3. The loader.ts change adds a financial_governance field to ComplianceConfig. This is also unrelated to the silent-catch fix and appears to have been included unintentionally. Worth confirming this was intended to be part of this PR.

Core fix looks good. The warn messages include the path and the error message, which is exactly what CONTRIBUTING.md asks for.