Repository navigation
PCA-209: Skills Repository v2 governed portable capability library - #17
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0511440cda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "revision": revision or entry["source_revision"], | ||
| "sha256": actual_hash, |
There was a problem hiding this comment.
When callers pass --revision, the resolver hashes files from the current checkout but labels that hash as the caller-supplied revision without checking out or otherwise verifying it. For example, resolving with --revision <old-commit> returns the current v2.0.0 contents attributed to that old commit, so a governed consumer cannot safely fetch or audit the claimed immutable source. Reject revisions other than the registry revision or load and hash the requested revision before emitting the binding.
Useful? React with 👍 / 👎.
| if contract["id"] != skill_id: | ||
| errors.append(f"{skill_id}: contract id mismatch") | ||
| if contract["authority"]["grants_authority"] is not False: | ||
| errors.append(f"{skill_id}: skill illegally grants authority") | ||
| if entry["source_revision"] != registry["release_revision"]: | ||
| errors.append(f"{skill_id}: revision differs from registry release") |
There was a problem hiding this comment.
When a skill.yaml changes without rerunning build_registry.py, the strict validator still passes as long as the contract ID and grants_authority remain valid; CI does not invoke the registry builder. Consequently fields consumed by resolve_skill.py, including version, runtimes, authority class, capabilities, evidence requirements, and mutation level, can remain stale or contradict the contract while the repository reports PASS. Compare every derived registry field with the loaded contract, and retain a verifiable contract hash.
Useful? React with 👍 / 👎.
| "jsCode": "// Generate briefing markdown\nconst data = $json;\nlet markdown = `# Daily Intelligence Briefing\\n\\n**Date**: ${new Date().toLocaleDateString('en-CA')}\\n**Time**: ${new Date().toLocaleTimeString()}\\n\\n`;\n\nmardown += `## Executive Summary\\n`;\nmardown += `Total items reviewed: ${data.item_count}\\n\\n`;\n\nObject.entries(data.grouped).forEach(([tier, items]) => {\n markdown += `### ${tier}\\n\\n`;\n items.slice(0, 3).forEach(item => {\n markdown += `- **${item.title}**\\n - Source: ${item.source}\\n - Score: ${item.combined_score}/100\\n - [Read more](${item.link})\\n\\n`;\n });\n});\n\nreturn { markdown };" | ||
| "method": "POST", | ||
| "url": "https://graph.microsoft.com/v1.0/me/sendMail", | ||
| "body": "={{ {message:{subject:`Daily Intelligence Briefing - ${$json.briefing_date}`,body:{contentType:'Text',content:$json.markdown}}} }}" |
There was a problem hiding this comment.
On every daily run, this Microsoft Graph sendMail request constructs a message with a subject and body but no toRecipients; the repair removed the existing EMAIL_LEADERSHIP_LIST recipient. The email branch therefore cannot deliver the promised leadership briefing and will be rejected or produce no delivery, even though the parallel Teams branch may succeed. Include the configured recipient list in the message payload.
Useful? React with 👍 / 👎.
| { | ||
| "parameters": { | ||
| "jsCode": "// Generate markdown filename and path\nconst item = $json;\nconst dateTime = new Date(item.published_date);\nconst dateStr = dateTime.toISOString().split('T')[0];\nconst timeStr = dateTime.toISOString().split('T')[1].substring(0, 5).replace(':', '-');\nconst slug = item.title\n .toLowerCase()\n .replace(/[^a-z0-9]+/g, '-')\n .substring(0, 40);\n\nconst filename = `${dateStr}-${timeStr}-${slug}.md`;\nconst filepath = `/Intelligence/Sources/${item.source_tier}/${filename}`;\n\nreturn { ...item, filename, filepath };" | ||
| "jsCode": "const item=$json; const slug=(item.title||'item').toLowerCase().replace(/[^a-z0-9]+/g,'-').slice(0,40); return {...item, filepath:`/Intelligence/Sources/${item.source_tier}/${slug}.md`};" |
There was a problem hiding this comment.
When two routed articles share the same normalized title and source tier, this path is identical because it contains only the truncated title slug. The subsequent vault write can overwrite the earlier note, losing archived intelligence and making the Airtable paths ambiguous. Preserve the documented date/time component or another stable unique item identifier in the filename.
Useful? React with 👍 / 👎.
| "method": "GET", | ||
| "url": "https://api.airtable.com/v0/{{ $env.AIRTABLE_BASE_ID }}/Feed%20Items?filterByFormula=AND(combined_score%3E%3D70,combined_score%3C85,IS_AFTER(published_date,NOW()-86400))" | ||
| "url": "https://api.airtable.com/v0/{{ $env.AIRTABLE_BASE_ID }}/Feed%20Items" |
There was a problem hiding this comment.
The daily job now requests the unfiltered Feed Items endpoint, then uses the first ten returned records, so old, low-scoring, or already alerted items can replace the intended score-70–84 items from the past 24 hours. This directly contradicts the routing contract in SKILL.md and makes the generated daily intelligence briefing inaccurate. Restore the score and publication-time filter, with pagination or ordering appropriate for the selected window.
Useful? React with 👍 / 👎.
| { | ||
| "parameters": { | ||
| "jsCode": "// Analyze feed health metrics\nconst items = $json.records.map(r => r.fields);\nconst now = new Date();\nconst sevenDaysAgo = new Date(now.getTime() - 7 * 24 * 60 * 60 * 1000);\n\nconst feedMetrics = {};\nconst sources = new Set();\n\nitems.forEach(item => {\n const source = item.source || 'Unknown';\n sources.add(source);\n \n if (!feedMetrics[source]) {\n feedMetrics[source] = {\n source,\n tier: item.source_tier || 'Unknown',\n item_count: 0,\n avg_score: 0,\n last_updated: null,\n status: 'OK'\n };\n }\n \n feedMetrics[source].item_count += 1;\n feedMetrics[source].avg_score += (item.combined_score || 0);\n \n const itemDate = new Date(item.published_date);\n if (!feedMetrics[source].last_updated || itemDate > new Date(feedMetrics[source].last_updated)) {\n feedMetrics[source].last_updated = item.published_date;\n }\n});\n\n// Calculate averages and health status\nObject.values(feedMetrics).forEach(feed => {\n feed.avg_score = Math.round(feed.avg_score / (feed.item_count || 1));\n \n const lastUpdate = new Date(feed.last_updated);\n const hoursSinceUpdate = (now - lastUpdate) / (1000 * 60 * 60);\n \n if (hoursSinceUpdate > 48) {\n feed.status = 'STALE';\n } else if (hoursSinceUpdate > 24) {\n feed.status = 'SLOW';\n } else {\n feed.status = 'OK';\n }\n});\n\nreturn {\n total_sources: sources.size,\n total_items_7d: items.length,\n feed_metrics: Object.values(feedMetrics)\n};" | ||
| "jsCode": "const items=($json.records||[]).map(r=>r.fields); const by={}; for(const i of items){const s=i.source||'Unknown'; by[s]=(by[s]||0)+1;} return {total_sources:Object.keys(by).length,total_items_7d:items.length,source_counts:by};" |
There was a problem hiding this comment.
For a stale or stopped feed, this analyzer only counts records by source and never reads published_date, so it cannot classify feeds as slow or stale or trigger the documented >48-hour alert. The workflow can therefore report nominal aggregate counts while a source has silently stopped updating. Retain each source's latest timestamp and derive the freshness status used by the alert branch.
Useful? React with 👍 / 👎.
1 participant
Implements PCA-209 Skills Repository v2.
Key outcomes:
Local verification before PR:
Jira authority: PCA-209 (parent PCA-53, governed by PCA-4).