You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
SAN-1274 PR 3 — Make fresh MDE coding sessions load the correct skills - #49
Task 63 · SAN-1274 PR 3 — Make fresh MDE coding sessions load the correct skills
Current status
PR #49 is rebased onto merged main and now provides the portable bootstrap/discovery layer for MDE coding sessions.
Base: main @ 41d376025e54622237a2c46035673f61c533b6ac
Current head: c613bf9f779a6f45fa6615719481fce3193d3d29
Scope: bootstrap/docs only; no intended application runtime behavior change
What this PR does
Makes .claude/hooks/session-start.mjs resolve the actual checkout root dynamically.
Reports the current worktree branch, HEAD, status, recent commits, and canonical-skill health.
Adds focused hook tests for clean scans, missing skills, broken skills, branch/worktree isolation, and last-three-commit output.
Modernizes AGENTS.md and CLAUDE.md around the canonical .claude/skills/ workflow.
Keeps the old using-mde-skills router and mde-task-lifecycle retired until SAN-1273 adds the lightweight router.
Current workflow
flowchart LR
A[Fresh coding session] --> B[AGENTS.md / CLAUDE.md]
B --> C[SessionStart hook]
C --> D[Resolve actual checkout]
D --> E[branch / HEAD / status / recent commits]
D --> F[canonical skill scan]
F --> G{Task type}
G -->|Simple| H[direct specialist]
G -->|Substantial| I[tasks]
G -->|Unknown failure| J[systematic-debugging]
G -->|PR review| K[code-review]
G -->|Done claim| L[task-verifier]
Loading
Verified fixes
removed hardcoded executable /home/sk/mdeai root
hook reports the checkout where it actually runs
added automated session-start tests
removed dynamic path-derived RegExp from the test
canonical required-skill scan expanded and simplified
copied-skill template warns authors to remove disable-model-invocation: true
git diff --check verification is repo-wide
CLAUDE.md uses repository-relative language
AGENTS.md rewritten as a concise portable bootstrap document
removed obsolete May-era status/model/MCP instructions from AGENTS.md
removed the 5 broken legacy links from AGENTS.md
removed stale skill aliases from active bootstrap guidance
shared detailed invariants now point to owning canonical skills
Exact-head focused verification
On the current branch after the bootstrap cleanup:
node --check .claude/hooks/session-start.mjs PASS
node --check .claude/hooks/__tests__/session-start.test.mjs PASS
node .claude/hooks/__tests__/session-start.test.mjs PASS
git diff --check PASS
machine-specific /home/sk/mdeai refs in active bootstrap 0
broken links in changed bootstrap files 0
active using-mde-skills / routing.yaml dependencies 0
live hook canonical skill scan OK
Live hook proof reports the active isolated worktree rather than another checkout.
Canonical workflow skills available from merged main
tasks, task-verifier, systematic-debugging, testing, tdd, research, code-review, writing-skills, wireframe, and mermaid-diagrams are present on the merged lower stack.
Stack/domain skills referenced by the bootstrap were also checked against .claude/skills/*/SKILL.md.
Review status
Previously reproduced blockers are fixed. High-risk dynamic-RegExp and portability/skill-scan findings were corrected and resolved. AGENTS-specific obsolete comments became outdated after the portable rewrite and were resolved.
Remaining low/medium wording comments are not runtime correctness blockers unless they become current actionable findings on the exact head.
Merge gate
Merge only when:
exact-head PR Agent/independent review has no new high/blocker finding;
exact-head Floor is green;
PR remains mergeable against current main.
After merge, retarget/rebase PR #50 onto the new main, require npm audit --audit-level=critical = 0, run final Floor, update SAN-1274 evidence, then begin SAN-1273 from the clean workflow foundation.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
While this PR successfully implements the canonical skill alignment and the session-start integrity scan, it introduces significant portability issues. The Codacy analysis identifies the PR as not up to standards due to multiple new issues, including absolute path hardcoding and documentation redundancies.
Two critical gaps exist: first, the repository root path is hardcoded in several files, which will break the environment for any user other than the current author. Second, there are no automated tests provided for the logic in .claude/hooks/session-start.mjs. This script handles file system operations and git interactions that are vital for session initialization; without tests, this logic remains fragile. Several documentation findings also point to a lack of exception paths for absolute rules and duplicated instructions across markdown files.
About this PR
The logic in '.claude/hooks/session-start.mjs' performs environment verification via file system and git commands but lacks automated unit tests to prevent regressions in the bootstrap process.
The repository root path '/home/sk/mdeai' is hardcoded in 'session-start.mjs', 'AGENTS.md', and 'CLAUDE.md'. This prevents portability to other development environments or CI runners. Please ensure all paths are derived dynamically relative to the repository root.
Test suggestions
Missing recommended test scenario: Verify session-start.mjs reports 'skill scan needs attention' when a required skill directory (e.g., 'tasks') is missing.
Missing recommended test scenario: Verify session-start.mjs correctly identifies and lists broken symlinks in the skills directory.
Missing recommended test scenario: Verify the git log output in the session preamble is restricted to the last 3 commits as specified in the updated code.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Missing recommended test scenario: Verify session-start.mjs reports 'skill scan needs attention' when a required skill directory (e.g., 'tasks') is missing.
2. Missing recommended test scenario: Verify session-start.mjs correctly identifies and lists broken symlinks in the skills directory.
3. Missing recommended test scenario: Verify the git log output in the session preamble is restricted to the last 3 commits as specified in the updated code.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Absolute rules without exception paths can cause operational blockers. Document a clear exception or escalation process for when dynamic field masks are required.
Suggested change
- Google Places requests must use intentional field masks; Maps markers require the correct map configuration.
- Google Places requests must use intentional field masks; Maps markers require the correct map configuration. Exceptions must be approved by the platform owner.
- Read the dated/numbered planning docs in the sibling planning repo (`/home/sk/mdeai/plan/`) for current direction; some `docs/` content may be superseded — cross-check the planning repo's audits.
- Use `mde-task-lifecycle` to plan/ship a task; floor before shipping: `/verify-floor`.
- Linear label taxonomy and deprecated prefixes are in `linear.md` §Labels. Do not use `SCREEN-*`, `EVP-*`, `IMP-*` as new issue prefixes.
Structural eval definitions are specifications only until they are actually executed. Never report an eval as passing merely because its JSON validates.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Define what should occur if an eval cannot be executed but the specification requires updating. Include criteria for authorization of such overrides.
Try running the following prompt in your IDE agent:
Add a documented exception path for the absolute rule regarding structural eval definitions in CLAUDE.md line 80, specifying how to handle cases where behavior is not objectively testable.
See [`/home/sk/mde/FREEZE.md`](../mde/FREEZE.md). After **2026-05-26**, `/home/sk/mde/` accepts only P0 security fixes (data exposure, auth bypass, payment failure, Sentry P0). All non-P0 work belongs in `/home/sk/mdeai/mdeapp/`. The hook `.Codex/hooks/guard-sensitive-paths.mjs` already blocks `Edit/Write/MultiEdit` into the legacy tree — that protection stays on. The 5-min onboarding for the new app lives at [`mdeapp/docs/ARCHITECTURE.md`](mdeapp/docs/ARCHITECTURE.md).
Keep tool-specific settings separate from shared skill content. Claude Code uses `CLAUDE.md` and `.claude/`; Cursor/OpenCode may add their own thin config only when needed; Codex and compatible agents use this `AGENTS.md` as the portable bootstrap.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Vague conditionals create ambiguity about when actions should occur. Specify the concrete thresholds or criteria that justify adding tool-specific config.
Suggested change
Keep tool-specific settings separate from shared skill content. Claude Code uses `CLAUDE.md` and `.claude/`; Cursor/OpenCode may add their own thin config only when needed; Codex and compatible agents use this `AGENTS.md` as the portable bootstrap.
Keep tool-specific settings separate from shared skill content. Claude Code uses `CLAUDE.md` and `.claude/`; Cursor/OpenCode may add their own thin config only when a tool-specific manifest (e.g., `.cursorrules`) is required for features not supported by AGENTS.md; Codex and compatible agents use this `AGENTS.md` as the portable bootstrap.
For simple work, use the directly relevant skill and do not add orchestration.
For substantial or ambiguous work:
1. For substantial or ambiguous work, start with `tasks`; it owns dependency-safe execution and specialist selection. SAN-1273 will add the lightweight router later.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Nitpick: The introductory phrase is redundant as it repeats the section header immediately above.
Suggested change
1.For substantial or ambiguous work, start with `tasks`; it owns dependency-safe execution and specialist selection. SAN-1273 will add the lightweight router later.
1.Start with `tasks`; it owns dependency-safe execution and specialist selection. SAN-1273 will add the lightweight router later.```
Fixed and pushed at 066c69ebf05e58340f56ccf179ddbd799b1f6a38.
Verified fixes:
removed hardcoded /home/sk/mdeai from executable hook logic; root now resolves from the hook location via git rev-parse --show-toplevel with a local fallback
added focused hook tests for current worktree/root, missing required skill, broken required-skill symlink, clean canonical scan, and exactly the last 3 commits
template copy instruction now explicitly says to remove disable-model-invocation: true unless intentionally disabled
CLAUDE.md verification now uses unrestricted git diff --check, so AGENTS.md and every changed file are covered
narrowed broken-link health checking to required canonical skills so legacy compatibility aliases do not create false failures
live hook from isolated PR worktree reports the actual /tmp/... checkout root and canonical skill scan: OK
git diff --check HEAD^..HEAD PASS
worktree clean after push
Resolved the three review threads directly addressed by this commit: hardcoded root, Kilo template disable flag, and Kilo diff-check scope. Lower-value Codacy wording/style comments were intentionally left unchanged because they are not blockers and are outside this focused portability fix.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The pull request successfully implements the logic for dynamic repository root resolution and skill scanning in the MDE bootstrap layer, meeting most functional acceptance criteria. However, the repository is currently not up to standards due to a high-severity security issue and several consistency gaps.
A significant security risk (ReDoS) was identified in the test suite's use of dynamic regular expressions. Furthermore, while the code now supports dynamic path resolution, the documentation (AGENTS.md and CLAUDE.md) still contains hardcoded absolute paths, which directly contradicts the portability goals of this change. There is also a discrepancy between the skills listed as 'required' in the session-start hook and the 'canonical' skills defined in the project documentation.
About this PR
There is a systemic inconsistency where the logic for session-start has been made portable, but the supporting documentation continues to use hardcoded absolute paths (/home/sk/mdeai). This prevents the repository from being truly environment-agnostic as intended.
Test suggestions
Verify dynamic repository root resolution correctly identifies the active worktree
Verify skill scan reports 'OK' when all required canonical skills are present
Verify skill scan reports 'missing' when a required SKILL.md file is absent
Verify skill scan identifies 'broken' symlinks for required skills
Verify the session preamble limits the git log output to the last 3 commits
Verify test suite robustness by replacing dynamic RegExp with string inclusion checks
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify test suite robustness by replacing dynamic RegExp with string inclusion checks
For simple work, use the directly relevant skill and do not add orchestration.
For substantial or ambiguous work:
1. For substantial or ambiguous work, start with `tasks`; it owns dependency-safe execution and specialist selection. SAN-1273 will add the lightweight router later.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
amoai-tech
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Task 63 · SAN-1274 PR 3 — Make fresh MDE coding sessions load the correct skills
Current status
PR #49 is rebased onto merged
mainand now provides the portable bootstrap/discovery layer for MDE coding sessions.main @ 41d376025e54622237a2c46035673f61c533b6acc613bf9f779a6f45fa6615719481fce3193d3d29What this PR does
.claude/hooks/session-start.mjsresolve the actual checkout root dynamically.AGENTS.mdandCLAUDE.mdaround the canonical.claude/skills/workflow.using-mde-skillsrouter andmde-task-lifecycleretired until SAN-1273 adds the lightweight router.Current workflow
flowchart LR A[Fresh coding session] --> B[AGENTS.md / CLAUDE.md] B --> C[SessionStart hook] C --> D[Resolve actual checkout] D --> E[branch / HEAD / status / recent commits] D --> F[canonical skill scan] F --> G{Task type} G -->|Simple| H[direct specialist] G -->|Substantial| I[tasks] G -->|Unknown failure| J[systematic-debugging] G -->|PR review| K[code-review] G -->|Done claim| L[task-verifier]Verified fixes
/home/sk/mdeairootRegExpfrom the testdisable-model-invocation: truegit diff --checkverification is repo-wideCLAUDE.mduses repository-relative languageAGENTS.mdrewritten as a concise portable bootstrap documentAGENTS.mdAGENTS.mdExact-head focused verification
On the current branch after the bootstrap cleanup:
Live hook proof reports the active isolated worktree rather than another checkout.
Canonical workflow skills available from merged main
tasks,task-verifier,systematic-debugging,testing,tdd,research,code-review,writing-skills,wireframe, andmermaid-diagramsare present on the merged lower stack.Stack/domain skills referenced by the bootstrap were also checked against
.claude/skills/*/SKILL.md.Review status
Previously reproduced blockers are fixed. High-risk dynamic-RegExp and portability/skill-scan findings were corrected and resolved. AGENTS-specific obsolete comments became outdated after the portable rewrite and were resolved.
Remaining low/medium wording comments are not runtime correctness blockers unless they become current actionable findings on the exact head.
Merge gate
Merge only when:
main.After merge, retarget/rebase PR #50 onto the new
main, requirenpm audit --audit-level=critical= 0, run final Floor, update SAN-1274 evidence, then begin SAN-1273 from the clean workflow foundation.