Worktree fix+compound value emission

.github/workflows/DeployManual.yml

@@ -66,7 +66,7 @@ jobs:
             # Auto-extract from git log since the previous tag
             if [ "${{ github.event_name }}" = "release" ]; then
               # Release event: HEAD is the new tag — find the nearest ancestor tag before it
-              PREV=$(git describe --tags --abbrev=0 HEAD^ 2>/dev/null || git tag --sort=-version:refname | tail -1)
+              PREV=$(git describe --tags --abbrev=0 HEAD^ 2>/dev/null || git tag --sort=-version:refname | head -1)
             else
               # Manual dispatch: find the nearest ancestor tag from HEAD
               # (git describe respects branch ancestry; avoids pulling in commits from sibling branches)

src/mcp/security/pathPolicy.ts

@@ -71,7 +71,9 @@ export function assertPathAllowed(filePath: string, allowedPaths: string[]): voi
     resolvedAllowed.length > 0 &&
     !resolvedAllowed.some((base) => {
       const baseKey = normalizeForCompare(base);
-      return resolvedKey === baseKey || resolvedKey.startsWith(baseKey + path.sep);
+      // Strip a trailing separator so roots like '/' or 'C:\' don't produce '//' or 'C:\\'
+      const baseKeyNorm = baseKey.endsWith(path.sep) ? baseKey.slice(0, -1) : baseKey;
+      return resolvedKey === baseKey || resolvedKey.startsWith(baseKeyNorm + path.sep);
     })
   ) {
     throw new PathPolicyError(

src/mcp/tools/testCaseGenerate.ts

@@ -278,7 +278,29 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig
 
 // ── XML builder ───────────────────────────────────────────────────────────────
 
-// Build the <value> element for a single argument (D2/D4 aware).
+// F1/F3: build class="compound" for strings that mix literal text with {VarName} tokens.
+function buildCompoundValue(val: string, indent: string): string {
+  const i = `${indent}  `;
+  const parts: string[] = [];
+  const tokenRe = /\{([\w.]+)\}/g;
+  let last = 0;
+  let m: RegExpExecArray | null;
+  while ((m = tokenRe.exec(val)) !== null) {
+    const before = val.slice(last, m.index);
+    if (before) parts.push(`${i}<value valueClass="string">${escapeXmlContent(before)}</value>`);
+    const pathElements = m[1]
+      .split('.')
+      .map((p) => `${i}  <path element="${escapeXmlAttr(p)}"/>`)
+      .join('\n');
+    parts.push(`${i}<variable>\n${pathElements}\n${i}</variable>`);
+    last = m.index + m[0].length;
+  }
+  const tail = val.slice(last);
+  if (tail) parts.push(`${i}<value valueClass="string">${escapeXmlContent(tail)}</value>`);
+  return `${indent}<value class="compound">\n${i}<parts>\n${parts.join('\n')}\n${i}</parts>\n${indent}</value>`;
+}
+
+// Build the <value> element for a single argument (D2/D4/F1 aware).
 // inNamedValues: when true (inside SetValues namedValues), skip uiTarget/uiLocator dispatch.
 // apiId: resolved API ID used to restrict key-name dispatch to the correct UI APIs.
 function buildArgumentValue(key: string, val: string, indent: string, inNamedValues = false, apiId = ''): string {
@@ -291,6 +313,10 @@ function buildArgumentValue(key: string, val: string, indent: string, inNamedVal
       .join('\n');
     return `${indent}<value class="variable">\n${pathElements}\n${indent}</value>`;
   }
+  // F1/F3: {VarName} embedded in surrounding text → class="compound" with <parts>.
+  if (/\{[\w.]+\}/.test(val)) {
+    return buildCompoundValue(val, indent);
+  }
   if (!inNamedValues) {
     // D2: 'target' argument → class="uiTarget" (only for UiWithScreen / UiWithRow).
     if (key === 'target' && (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow'))) {

src/mcp/tools/testCaseValidate.ts

@@ -329,23 +329,43 @@ export function validateTestCase(xmlContent: string, testName?: string): TestCas
     validateApiCall(call, issues);
   }
 
-  // VAR-REF-001 (gap in both local and quality-hub-agents backend):
-  // Detect {VarName} or {Obj.Field} literals stored as plain string values.
-  // Provar will pass these as-is to the API rather than resolving them as variable references.
-  const varLiteralRe = /<value[^>]+valueClass="string"[^>]*>\{([\w.]+)\}<\/value>/g;
-  let varMatch: RegExpExecArray | null;
-  while ((varMatch = varLiteralRe.exec(xmlContent)) !== null) {
-    issues.push({
-      rule_id: 'VAR-REF-001',
-      severity: 'WARNING',
-      message: `Argument value "{${varMatch[1]}}" looks like a variable reference but is stored as a plain string — Provar will not resolve it at runtime.`,
-      applies_to: 'argument',
-      suggestion: `Replace with <value class="variable"><path element="${varMatch[1]
-        .split('.')
-        .join(
-          '"/><path element="'
-        )}"/></value>. In provar_testcase_generate, use the {VarName} syntax in the attributes object — the generator converts it automatically.`,
-    });
+  // VAR-REF-001 / VAR-REF-002: detect {VarName} tokens inside valueClass="string" elements.
+  // Provar does not interpolate {…} tokens in plain string values at runtime — they must use
+  // class="variable" (pure reference) or class="compound" (embedded in surrounding text).
+  const stringValueRe = /<value[^>]+valueClass="string"[^>]*>([^<]+)<\/value>/g;
+  let stringMatch: RegExpExecArray | null;
+  while ((stringMatch = stringValueRe.exec(xmlContent)) !== null) {
+    const rawContent = stringMatch[1];
+    if (!/\{[\w.]+\}/.test(rawContent)) continue;
+    const isPure = /^\{[\w.]+\}$/.test(rawContent.trim());
+    const varNames = [...rawContent.matchAll(/\{([\w.]+)\}/g)].map((m) => m[1]);
+    if (isPure) {
+      const varName = varNames[0];
+      issues.push({
+        rule_id: 'VAR-REF-001',
+        severity: 'WARNING',
+        message: `Argument value "{${varName}}" looks like a variable reference but is stored as a plain string — Provar will not resolve it at runtime.`,
+        applies_to: 'argument',
+        suggestion: `Replace with <value class="variable"><path element="${varName
+          .split('.')
+          .join(
+            '"/><path element="'
+          )}"/></value>. In provar_testcase_generate, use the {VarName} syntax in the attributes object — the generator converts it automatically.`,
+      });
+    } else {
+      const preview = rawContent.length > 60 ? rawContent.slice(0, 57) + '…' : rawContent;
+      issues.push({
+        rule_id: 'VAR-REF-002',
+        severity: 'WARNING',
+        message: `Argument value "${preview}" contains {${varNames.join(
+          '}, {'
+        )}} embedded in a plain string — Provar does not interpolate {…} tokens in string values at runtime.`,
+        applies_to: 'argument',
+        suggestion:
+          'Use class="compound" with <parts> to split literal text and variable references at each {VarName} boundary. ' +
+          'In provar_testcase_generate, pass the value with {VarName} placeholders in the attributes object — the generator emits compound XML automatically.',
+      });
+    }
   }
 
   return finalize(issues, tcId, tcName, apiCalls.length, xmlContent, testName);

src/mcp/tools/testPlanTools.ts

@@ -259,8 +259,9 @@ export function registerTestPlanAddInstance(server: McpServer, config: ServerCon
           };
         }
 
-        // Resolve testcase absolute path
+        // Resolve testcase absolute path and enforce path policy before any fs access
         const absoluteTestCasePath = path.join(projectRoot, test_case_path);
+        assertPathAllowed(absoluteTestCasePath, config.allowedPaths);
         if (!fs.existsSync(absoluteTestCasePath)) {
           return {
             isError: true,

test/unit/mcp/testCaseGenerate.test.ts

@@ -744,4 +744,121 @@ describe('provar_testcase_generate', () => {
       assert.ok(!('validation' in body), 'validation field should be absent when validate_after_edit=false');
     });
   });
+
+  describe('F1/F3 — compound value emission for embedded {VarName} tokens', () => {
+    it('emits class="compound" with <parts> when a SOQL query embeds a variable (F1)', () => {
+      const result = server.call('provar_testcase_generate', {
+        test_case_name: 'SOQL Compound Test',
+        steps: [
+          {
+            api_id: 'ApexSoqlQuery',
+            name: 'Query account',
+            attributes: { soqlQuery: "SELECT Id, Name FROM Account WHERE Id = '{AccountId}'" },
+          },
+        ],
+        dry_run: true,
+        overwrite: false,
+        validate_after_edit: false,
+      });
+
+      assert.equal(isError(result), false);
+      const xml = parseText(result)['xml_content'] as string;
+      assert.ok(xml.includes('class="compound"'), 'Expected class="compound" for embedded variable in SOQL');
+      assert.ok(xml.includes('<parts>'), 'Expected <parts> element inside compound value');
+      assert.ok(xml.includes('<variable>'), 'Expected <variable> element for the AccountId reference');
+      assert.ok(xml.includes('<path element="AccountId"/>'), 'Expected <path element="AccountId"/>');
+      assert.ok(
+        !xml.includes('valueClass="string">{AccountId}'),
+        'Must NOT emit {AccountId} as a plain string literal'
+      );
+    });
+
+    it('emits class="compound" for Provar system variables embedded in a string (F3: {NOW})', () => {
+      const result = server.call('provar_testcase_generate', {
+        test_case_name: 'NOW Compound Test',
+        steps: [
+          {
+            api_id: 'SetValues',
+            name: 'Set account name',
+            attributes: { AccountName: 'Acme Corp CRUD Test {NOW}' },
+          },
+        ],
+        dry_run: true,
+        overwrite: false,
+        validate_after_edit: false,
+      });
+
+      assert.equal(isError(result), false);
+      const xml = parseText(result)['xml_content'] as string;
+      assert.ok(xml.includes('class="compound"'), 'Expected class="compound" inside namedValues');
+      assert.ok(xml.includes('<path element="NOW"/>'), 'Expected <path element="NOW"/> for system variable');
+      assert.ok(
+        !xml.includes('valueClass="string">Acme Corp CRUD Test {NOW}'),
+        'Must NOT emit {NOW} as a literal string'
+      );
+    });
+
+    it('emits <parts> with correct literal fragments around the variable', () => {
+      const result = server.call('provar_testcase_generate', {
+        test_case_name: 'Fragment Test',
+        steps: [
+          {
+            api_id: 'ApexSoqlQuery',
+            name: 'Query with prefix and suffix',
+            attributes: { soqlQuery: "SELECT Id FROM Contact WHERE Email = '{Email}' LIMIT 1" },
+          },
+        ],
+        dry_run: true,
+        overwrite: false,
+        validate_after_edit: false,
+      });
+
+      const xml = parseText(result)['xml_content'] as string;
+      assert.ok(xml.includes("SELECT Id FROM Contact WHERE Email = '"), 'Expected literal prefix fragment');
+      assert.ok(xml.includes("' LIMIT 1"), 'Expected literal suffix fragment');
+      assert.ok(xml.includes('<path element="Email"/>'), 'Expected variable path element');
+    });
+
+    it('handles multiple embedded variables in one string', () => {
+      const result = server.call('provar_testcase_generate', {
+        test_case_name: 'Multi Var Test',
+        steps: [
+          {
+            api_id: 'ApexSoqlQuery',
+            name: 'Query by two fields',
+            attributes: { soqlQuery: "SELECT Id FROM Case WHERE AccountId='{AccId}' AND OwnerId='{OwnerId}'" },
+          },
+        ],
+        dry_run: true,
+        overwrite: false,
+        validate_after_edit: false,
+      });
+
+      const xml = parseText(result)['xml_content'] as string;
+      assert.ok(xml.includes('<path element="AccId"/>'), 'Expected first variable path');
+      assert.ok(xml.includes('<path element="OwnerId"/>'), 'Expected second variable path');
+      const compoundCount = (xml.match(/class="compound"/g) ?? []).length;
+      assert.equal(compoundCount, 1, 'Should be exactly one compound element for the soqlQuery argument');
+    });
+
+    it('pure {VarName} value (entire argument) still uses class="variable", not compound', () => {
+      const result = server.call('provar_testcase_generate', {
+        test_case_name: 'Pure Var Test',
+        steps: [
+          {
+            api_id: 'ApexDeleteObject',
+            name: 'Delete account',
+            attributes: { recordId: '{AccountId}' },
+          },
+        ],
+        dry_run: true,
+        overwrite: false,
+        validate_after_edit: false,
+      });
+
+      const xml = parseText(result)['xml_content'] as string;
+      assert.ok(xml.includes('class="variable"'), 'Pure {VarName} should use class="variable"');
+      assert.ok(!xml.includes('class="compound"'), 'Pure {VarName} must NOT use class="compound"');
+    });
+  });
 });

test/unit/mcp/testCaseValidate.test.ts

@@ -788,6 +788,110 @@ describe('validateTestCase', () => {
       );
     });
   });
+
+  describe('VAR-REF-002', () => {
+    it('warns when {VarName} is embedded in a larger SOQL string (F1)', () => {
+      const r = validateTestCase(
+        `<?xml version="1.0" encoding="UTF-8"?>
+<testCase id="x" guid="${GUID_TC}" registryId="r" name="T">
+  <steps>
+    <apiCall guid="${GUID_S1}" apiId="ApexSoqlQuery" name="Query account" testItemId="1">
+      <arguments>
+        <argument id="soqlQuery">
+          <value class="value" valueClass="string">SELECT Id, Name FROM Account WHERE Id = '{AccountId}'</value>
+        </argument>
+      </arguments>
+    </apiCall>
+  </steps>
+</testCase>`
+      );
+      assert.ok(
+        r.issues.some((i) => i.rule_id === 'VAR-REF-002'),
+        'Expected VAR-REF-002 for embedded variable in SOQL string'
+      );
+      const issue = r.issues.find((i) => i.rule_id === 'VAR-REF-002')!;
+      assert.equal(issue.severity, 'WARNING');
+      assert.ok(issue.message.includes('AccountId'), `Message should include variable name: ${issue.message}`);
+      assert.ok(issue.suggestion?.includes('compound'), 'Suggestion should mention compound format');
+    });
+
+    it('warns for multiple embedded variables in one string (F3 / system vars)', () => {
+      const r = validateTestCase(
+        `<?xml version="1.0" encoding="UTF-8"?>
+<testCase id="x" guid="${GUID_TC}" registryId="r" name="T">
+  <steps>
+    <apiCall guid="${GUID_S1}" apiId="SetValues" name="Set name" testItemId="1">
+      <arguments>
+        <argument id="values">
+          <value class="valueList" mutable="Mutable">
+            <namedValues>
+              <namedValue name="AccountName">
+                <value class="value" valueClass="string">Acme Corp CRUD Test {NOW}</value>
+              </namedValue>
+            </namedValues>
+          </value>
+        </argument>
+      </arguments>
+    </apiCall>
+  </steps>
+</testCase>`
+      );
+      assert.ok(
+        r.issues.some((i) => i.rule_id === 'VAR-REF-002'),
+        'Expected VAR-REF-002 for {NOW} embedded in SetValues string'
+      );
+      assert.ok(r.issues.find((i) => i.rule_id === 'VAR-REF-002')!.message.includes('NOW'));
+    });
+
+    it('does NOT fire for a pure {VarName} value (VAR-REF-001 owns that case)', () => {
+      const r = validateTestCase(
+        `<?xml version="1.0" encoding="UTF-8"?>
+<testCase id="x" guid="${GUID_TC}" registryId="r" name="T">
+  <steps>
+    <apiCall guid="${GUID_S1}" apiId="SomeApi" name="Pure var" testItemId="1">
+      <arguments>
+        <argument id="id">
+          <value class="value" valueClass="string">{AccountId}</value>
+        </argument>
+      </arguments>
+    </apiCall>
+  </steps>
+</testCase>`
+      );
+      assert.ok(!r.issues.some((i) => i.rule_id === 'VAR-REF-002'), 'VAR-REF-002 must not double-fire on pure vars');
+      assert.ok(
+        r.issues.some((i) => i.rule_id === 'VAR-REF-001'),
+        'VAR-REF-001 should still fire'
+      );
+    });
+
+    it('does NOT fire for a correct class="compound" value', () => {
+      const r = validateTestCase(
+        `<?xml version="1.0" encoding="UTF-8"?>
+<testCase id="x" guid="${GUID_TC}" registryId="r" name="T">
+  <steps>
+    <apiCall guid="${GUID_S1}" apiId="ApexSoqlQuery" name="Query account" testItemId="1">
+      <arguments>
+        <argument id="soqlQuery">
+          <value class="compound">
+            <parts>
+              <value valueClass="string">SELECT Id FROM Account WHERE Id = '</value>
+              <variable><path element="AccountId"/></variable>
+              <value valueClass="string">'</value>
+            </parts>
+          </value>
+        </argument>
+      </arguments>
+    </apiCall>
+  </steps>
+</testCase>`
+      );
+      assert.ok(
+        !r.issues.some((i) => i.rule_id === 'VAR-REF-002'),
+        'VAR-REF-002 must not fire on correct compound value'
+      );
+    });
+  });
 });
 
 // ── Handler-level tests (registerTestCaseValidate) ────────────────────────────

test/unit/mcp/testPlanTools.test.ts

@@ -507,6 +507,28 @@ describe('provar_testplan_add-instance', () => {
     });
   });
 
+  describe('path policy on test_case_path', () => {
+    it('returns PATH_NOT_ALLOWED when test_case_path escapes project root via ..', () => {
+      const strictServer = new MockMcpServer();
+      // Use projectDir as the allowed root so ../outside.testcase is outside allowed paths
+      registerAllTestPlanTools(strictServer as never, { allowedPaths: [projectDir] });
+      makeProject(projectDir);
+      makePlan(projectDir, 'P');
+
+      const result = strictServer.call('provar_testplan_add-instance', {
+        project_path: projectDir,
+        test_case_path: '../outside.testcase',
+        plan_name: 'P',
+        overwrite: false,
+        dry_run: false,
+      });
+
+      assert.equal(isError(result), true);
+      const code = errorCode(result);
+      assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected error code: ${code}`);
+    });
+  });
+
   describe('testCasePath forward-slash normalization', () => {
     it('normalizes backslashes to forward slashes in written XML', () => {
       makeProject(projectDir);