Add option to merge system and user review rules

[guqihao-7]

Summary

Adds a new ocr review --merge-sys-rule flag that keeps the matched built-in system rule when a custom, project, or global user rule also matches the reviewed file.

By default, user rules continue to replace the matched system rule. When --merge-sys-rule is enabled, OCR merges both rule sources into the review checklist.

Changes

• Added --merge-sys-rule to ocr review
• Added ResolverOptions and NewResolverWithOptions to configure resolver behavior without changing the Resolver interface
• Updated rule resolution so matched user rules can be merged with matched system rules
• Kept merge behavior scoped to the rules resolver layer
• Added tests for flag parsing and merge-rule resolution behavior
• Updated README docs, CLI help, and OpenCodeReview skill docs

Below is what the rules look like after merging:

## System-Specific Rules (Mandatory)

#### Correctness
Is the logic correct? Are there missing boundary conditions?
Are exceptions handled properly?
Is it thread-safe in concurrent scenarios?

#### Security
Are there security vulnerabilities such as SQL injection or XSS?
Is sensitive information handled correctly?
Is permission validation complete?

#### Performance
Are there obvious performance issues (e.g., N+1 queries, unnecessary loops)?
Are resources properly released?

#### Maintainability
Is the code clear and easy to understand?
Do names accurately express intent?
Does it follow the project’s existing code style and architecture patterns?

#### Test Coverage
Do critical logic paths have corresponding test cases?
Do test cases cover boundary conditions?

---

## User-Specific Rules (Mandatory)

#### 所有新方法必须对必填参数进行空值校验
#### 所有变量名见名知其意

unit tests all pass:

[github-actions]

✅ OpenCodeReview: No comments generated. Looks good to me.

[CLAassistant]

All committers have signed the CLA.

[lizhengfeng101]

Thanks for the contribution! The merge behavior itself is a valid use case, but I think the approach could be improved.

Adding --merge-sys-rule as a CLI flag feels invasive to the command interface — it introduces a flag that only matters when user rules are configured, and creates an awkward edge case when --merge-sys-rule is passed but no user rules actually exist.

Since this behavior is inherently tied to user rules, I'd suggest making it a rule-level configuration instead — a merge_system_rule field at the same level as path and rule in the rule JSON:

{
  "rules": [
    {
      "path": "force-api/**/*.java",
      "rule": "所有新方法必须对必填参数进行空值校验",
      "merge_system_rule": true
    }
  ]
}

This way:

1. The merge behavior is scoped to specific rules/paths, giving users finer-grained control
2. No CLI flag needed — keeps the command interface clean
3. No need to handle the case where --merge-sys-rule is set but no user rules are configured
4. It's self-documenting: the rule file itself declares whether it wants to coexist with system rules

[guqihao-7]

Thanks for the contribution! The merge behavior itself is a valid use case, but I think the approach could be improved.

Adding --merge-sys-rule as a CLI flag feels invasive to the command interface — it introduces a flag that only matters when user rules are configured, and creates an awkward edge case when --merge-sys-rule is passed but no user rules actually exist.

Since this behavior is inherently tied to user rules, I'd suggest making it a rule-level configuration instead — a merge_system_rule field at the same level as path and rule in the rule JSON:

{
  "rules": [
    {
      "path": "force-api/**/*.java",
      "rule": "所有新方法必须对必填参数进行空值校验",
      "merge_system_rule": true
    }
  ]
}

This way:

1. The merge behavior is scoped to specific rules/paths, giving users finer-grained control
2. No CLI flag needed — keeps the command interface clean
3. No need to handle the case where --merge-sys-rule is set but no user rules are configured
4. It's self-documenting: the rule file itself declares whether it wants to coexist with system rules

Great suggestion — I agree that moving this behavior into the rule configuration makes a lot more sense than exposing it as a CLI flag. Thanks for the detailed feedback. I’ll work on this and get it sorted out as soon as fast. Thanks!

[guqihao-7]

@lizhengfeng101

Hi, I’d like to confirm the intended matching semantics for user rule entries, especially for backward compatibility.

In the previous implementation, when a rule entry matched the file path, the resolver returned that entry’s rule value immediately. This means that if the matched rule was an empty string, the resolver still stopped matching the remaining entries in the same rule file. The empty string would then be ignored by the outer priority logic, so resolution would continue to lower-priority rule layers or the system rules.

func matchProjectRule(pr *ProjectRule, path string) string {
	if pr == nil {
		return ""
	}
	lowerPath := strings.ToLower(path)
	for _, entry := range pr.Rules {
		expanded := expandBraces(entry.Path)
		for _, p := range expanded {
            // not check whether the entry's rule is empty, just return if matched 
			if matched, _ := doublestar.Match(strings.ToLower(p), lowerPath); matched {
				return entry.Rule
			}
		}
	}
	return ""
}

For example:

  {
    "rules": [
      {
        "path": "internal/**/*.go",
        "rule": ""
      },
      {
        "path": "internal/config/**/*.go",
        "rule": "Config code must validate default values."
      }
    ]
  }

For the file internal/config/rules/system_rules.go, the previous behavior was:

1. internal/**/*.go matches first.
2. Its rule is an empty string, but matching stops there.
3. The later internal/config/**/*.go rule is not considered.
4. Since the returned user rule is empty, resolution falls back to lower-priority rule layers or system rules.

Could you confirm what the intended behavior should be?

Should the resolver preserve the previous “first matched entry wins” behavior within the same rule file, even when the matched rule is empty?

More generally, when a file matches multiple rules in the same JSON rule file, should the first matching rule always take precedence, or should the resolver continue looking for a later matching rule under some conditions, such as when the earlier rule has an empty rule value?

I want to keep this change compatible with the existing rule priority semantics as much as possible.

Looking forward to your reply.